1039866 - Remove the metro browser from mozilla-central

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Build Config, defect)

Description

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

It messes with my MXR search results to see what APIs/etc are used.  And it's always in version control if we want it back.

Comment 1

[Matt Brubeck (:mbrubeck)]

No objections here.  We have a few people working on unofficial projects based on metrofox, but we encourage them to use https://hg.mozilla.org/projects/metro as a base, rather than mozilla-central (since we have no protection against regressions on m-c).  We can keep the code alive there, and merge core changes from m-c to projects/metro periodically.

Comment 2

[Jim Mathies [:jimm]]

(In reply to Kyle Huey [:khuey] ([e-mail]) from comment #0)
> It messes with my MXR search results to see what APIs/etc are used.  And
> it's always in version control if we want it back.

Are you suggesting removing browser/metro or everything idfef'd MOZ_METRO? If we can keep platform for another year or so to see, that would make merges much simpler.

Comment 3

[Maksim Lebedev]

In my humble opinion, if metro will be only live in project/metro branch, in case of big changing m-c, supporting of work MetroFF will be harder and more increase.

Comment 4

[Maksim Lebedev]

Objections: There are several issues in m-c at current time. This issues blocking merging from m-c to project/metro branch. As result we have old version of MetroFF in project/metro branch and we can not get new updates from m-c. That's why I think that MetroFF should live in m-c and should be compiled possibly always (for example, with flag --enable-metro). Only in this case project/metro branch can have stable version of MetroFF.

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Having Metro in mozilla-central is not the right cost/benefit tradeoff.

I'd like to at least remove browser/metro, removing all of the #ifdef MOZ_METRO code is probably less important.

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

I can see Gavin's point. We have to consider the larger project group too. Having Metro in m-c means there is more code to worry about during refactors, more code to consider when adding features, and more code that just continues to fall out of date.

I don't object to removing it.

Comment 7

[Matt Brubeck (:mbrubeck)]

The work to create a reference implementation of Pointer Events in Firefox for Metro was basically completed a few weeks ago.  If any followups are necessary in the future we could use the "metro" project branch.  I have no more objections to removing the code from m-c.

Comment 8

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

What is the scope of this bug? Should dependent bugs be created?

To remove?
/browser/metro (removes 58k LOC)
metro searchplugins
widget/windows/winrt, and cleanup more widget stuff
code ifdef-ed with MOZ_METRO
code which checks for Services.metro
code which checks for (XRE_GetWindowsEnvironment() == WindowsEnvironmentType_Metro)
toolkit code related to metro (e.g. updater)
metro tests
etc.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

browser/metro is my primary concern. Less concerned about the other things you list, though eventually those should go too.

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch removing /browser/metro, v1

Successful Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5764d1fc551b

Comment 11

[Jim Mathies [:jimm]]

Comment on attachment 8562000 [details] [diff] [review]
patch removing /browser/metro, v1

Seems rather sloppy ripping out the easy part and ignoring the rest? Sounds like Gavin wants this part gone though, so transferring the review request over to him.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8562000 [details] [diff] [review]
patch removing /browser/metro, v1

I think we should be a bit more thorough than just a straight hg rm of browser/metro, assuming that's what this is. I think cleaning up all of browser/ at once is probably a good first step.

I think that means that beyond this, we should also audit all of the results at https://mxr.mozilla.org/mozilla-central/search?string=metro&find=browser%2F outside of browser/metro and see what else should be removed.

Comment 13

[Richard Newman [:rnewman]]

There's some benefit in splitting this bug into the easy "delete the files" part and the harder "wipe up all the blood" part -- doing the former unblocks other untangling where the metro code has some dependency, such as Bug 1148979.

Unless someone wants to green-light landing changes in the mean time that outright break code in browser/metro, of course…

Comment 14

[Tim Taubert [:ttaubert] (inactive)]

(In reply to Richard Newman [:rnewman] from comment #13)
> Unless someone wants to green-light landing changes in the mean time that
> outright break code in browser/metro, of course…

Code in browser/metro is pretty much broken already. I've reviewed and wrote patches that did that and I'm sure other people did the same.

Comment 15

[:Gavin Sharp [email: [e-mail]]]

(In reply to Richard Newman [:rnewman] from comment #13)
> Unless someone wants to green-light landing changes in the mean time that
> outright break code in browser/metro, of course…

Such changes have been green lighted for a while now.

Comment 16

[Richard Newman [:rnewman]]

\o/

Comment 17

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 1 - Remove widget/windows/winrt

Comment 18

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 2 - Remove browser/metro

Comment 19

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 3 - Remove MOZ_METRO and friends

I think there might still be a bunch of JS and related assets floating around, but this gets rid of the bulk of it. Haven't tried compiling anything yet.

Comment 20

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 3 - Remove MOZ_METRO and friends

Updated to remove more metro stuff. There's still a bit left in browser/installer/windows/nsis/shared.nsh which I'm not sure what to do with, and it looks scary to touch.

Comment 21

[Kartikaya Gupta (email:[e-mail])]

Attachment: References to metro still left

Here's a case-insensitive grep of stuff still left in the tree after applying the three patches above. Apparently still a lot left :(

Comment 22

[Kartikaya Gupta (email:[e-mail])]

Attachment: References to metro still left

Whoops, wrong version

Comment 23

[Jim Mathies [:jimm]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #20)
> Created attachment 8592887 [details] [diff] [review]
> Part 3 - Remove MOZ_METRO and friends
> 
> Updated to remove more metro stuff. There's still a bit left in
> browser/installer/windows/nsis/shared.nsh which I'm not sure what to do
> with, and it looks scary to touch.

I think you can safely kick this out to a followup if you don't want to mess with it.

Thanks for working on this btw!

Comment 24

[Kartikaya Gupta (email:[e-mail])]

I updated part 3 a bit more to remove stuff I thought was safe, and did a try push. *fingers crossed*

https://treeherder.mozilla.org/#/jobs?repo=try&revision=794a002a5a87

Comment 25

[:Gavin Sharp [email: [e-mail]]]

Thanks for digging into this, kats!

Bug 1151598 fixed some of the toolkit/modules hits from attachment 8592896 [details].

Comment 26

[Maksim Lebedev]

I have become sad. MetroFireFox, I will miss You!

Comment 27

[Kartikaya Gupta (email:[e-mail])]

Better try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=626480d08d1b. Once that's done I'll upload the latest patches.

Comment 28

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 1 - Remove widget/windows/winrt

Comment 29

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 2 - Remove browser/metro

Comment 30

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 3 - Remove MOZ_METRO and friends

I don't know who all needs to review this. Here's the list of files I touched. If you want me to split it up into multiple patches let me know.

CLOBBER
accessible/windows/msaa/AccessibleWrap.cpp
browser/app-rules.mk
browser/app/moz.build
browser/app/nsBrowserApp.cpp
browser/app/profile/firefox.js
browser/base/content/browser-menubar.inc
browser/base/content/browser.js
browser/base/content/sync/setup.js
browser/base/content/sync/setup.xul
browser/branding/aurora/Makefile.in
browser/branding/aurora/content/Makefile.in
browser/branding/aurora/content/jar.mn
browser/branding/aurora/content/metro-about-footer.png
browser/branding/aurora/content/metro-about-wordmark.png
browser/branding/aurora/content/metro-about.css
browser/branding/aurora/content/metro_firstrun_logo.png
browser/branding/aurora/content/[e-mail]
browser/branding/aurora/content/[e-mail]
browser/branding/nightly/Makefile.in
browser/branding/nightly/content/Makefile.in
browser/branding/nightly/content/jar.mn
browser/branding/nightly/content/metro-about-footer.png
browser/branding/nightly/content/metro-about-wordmark.png
browser/branding/nightly/content/metro-about.css
browser/branding/nightly/content/metro_firstrun_logo.png
browser/branding/nightly/content/[e-mail]
browser/branding/nightly/content/[e-mail]
browser/branding/official/Makefile.in
browser/branding/official/content/Makefile.in
browser/branding/official/content/jar.mn
browser/branding/official/content/metro-about-footer.png
browser/branding/official/content/metro-about-wordmark.png
browser/branding/official/content/metro-about.css
browser/branding/official/content/metro_firstrun_logo.png
browser/branding/official/content/[e-mail]
browser/branding/official/content/[e-mail]
browser/branding/unofficial/Makefile.in
browser/branding/unofficial/content/Makefile.in
browser/branding/unofficial/content/jar.mn
browser/branding/unofficial/content/metro-about-footer.png
browser/branding/unofficial/content/metro-about-wordmark.png
browser/branding/unofficial/content/metro-about.css
browser/branding/unofficial/content/metro_firstrun_logo.png
browser/branding/unofficial/content/[e-mail]
browser/branding/unofficial/content/[e-mail]
browser/build.mk
browser/components/customizableui/CustomizableUI.jsm
browser/components/customizableui/CustomizableWidgets.jsm
browser/components/customizableui/test/browser_876944_customize_mode_create_destroy.js
browser/components/customizableui/test/browser_880382_drag_wide_widgets_in_panel.js
browser/components/customizableui/test/browser_890140_orphaned_placeholders.js
browser/components/customizableui/test/head.js
browser/components/preferences/advanced.js
browser/components/preferences/advanced.xul
browser/components/preferences/in-content/advanced.js
browser/components/preferences/in-content/advanced.xul
browser/components/preferences/in-content/main.js
browser/components/preferences/main.js
browser/extensions/Makefile.in
browser/installer/package-manifest.in
browser/installer/windows/nsis/defines.nsi.in
browser/installer/windows/nsis/installer.nsi
browser/installer/windows/nsis/shared.nsh
browser/installer/windows/nsis/uninstaller.nsi
browser/locales/Makefile.in
browser/locales/en-US/chrome/browser/browser.dtd
browser/locales/en-US/chrome/browser/customizableui/customizableWidgets.properties
browser/locales/en-US/chrome/browser/preferences/advanced.dtd
browser/locales/en-US/chrome/browser/syncSetup.dtd
browser/locales/en-US/profile/bookmarks.inc
browser/locales/en-US/searchplugins/bingmetrofx.xml
browser/locales/en-US/searchplugins/googlemetrofx.xml
browser/locales/en-US/searchplugins/metrolist.txt
browser/locales/en-US/searchplugins/wikipediametrofx.xml
browser/locales/en-US/searchplugins/yahoometrofx.xml
browser/locales/filter.py
browser/modules/BrowserUITelemetry.jsm
browser/moz.build
browser/themes/shared/browser.inc
browser/themes/windows/Metro_Glyph-inverted.png
browser/themes/windows/Metro_Glyph-menuPanel.png
browser/themes/windows/Metro_Glyph.png
browser/themes/windows/browser.css
browser/themes/windows/jar.mn
build/automation.py.in
build/win32/mozconfig.vs2010-win64
build/win32/mozconfig.vs2013-win64
build/win64/mozconfig.vs2010
build/win64/mozconfig.vs2013
config/recurse.mk
configure.in
dom/base/nsContentUtils.h
dom/svg/SVGDocument.cpp
gfx/doc/AsyncPanZoom.md
gfx/layers/d3d11/CompositorD3D11.cpp
gfx/thebes/gfxWindowsPlatform.cpp
ipc/glue/WindowsMessageLoop.cpp
js/src/configure.in
layout/reftests/forms/input/color/reftest.list
modules/libpref/Preferences.cpp
profile/dirserviceprovider/nsProfileDirServiceProvider.cpp
startupcache/StartupCache.cpp
testing/mochitest/Makefile.in
testing/mozbase/mozrunner/mozrunner/runners.py
testing/profiles/prefs_general.js
toolkit/components/crashmonitor/CrashMonitor.jsm
toolkit/components/downloads/nsDownloadManager.cpp
toolkit/components/jsdownloads/src/DownloadIntegration.jsm
toolkit/components/startup/nsAppStartup.cpp
toolkit/components/startup/nsAppStartup.h
toolkit/components/startup/public/nsIAppStartup.idl
toolkit/content/widgets/browser.xml
toolkit/crashreporter/nsExceptionHandler.cpp
toolkit/library/moz.build
toolkit/locales/en-US/chrome/places/places.properties
toolkit/mozapps/installer/windows/nsis/common.nsh
toolkit/mozapps/update/common/updatehelper.cpp
toolkit/mozapps/update/common/updatehelper.h
toolkit/mozapps/update/nsIUpdateService.idl
toolkit/mozapps/update/tests/chrome/utils.js
toolkit/mozapps/update/updater/updater.cpp
toolkit/webapps/WebappOSUtils.jsm
toolkit/xre/nsAppRunner.cpp
toolkit/xre/nsEmbedFunctions.cpp
toolkit/xre/nsUpdateDriver.cpp
toolkit/xre/nsWindowsWMain.cpp
toolkit/xre/nsXREDirProvider.cpp
tools/profiler/platform.cpp
uriloader/exthandler/nsExternalHelperAppService.cpp
widget/MetroUIUtils.idl
widget/NativeKeyToDOMKeyName.h
widget/moz.build
widget/nsAppShellSingleton.h
widget/nsIWinMetroUtils.idl
widget/nsWidgetsCID.h
widget/windows/KeyboardLayout.cpp
widget/windows/WinMouseScrollHandler.cpp
widget/windows/WinTaskbar.cpp
widget/windows/WinUtils.cpp
widget/windows/moz.build
widget/windows/nsClipboard.cpp
widget/windows/nsLookAndFeel.cpp
widget/windows/nsTextStore.cpp
widget/windows/nsTextStore.h
widget/windows/nsWidgetFactory.cpp
widget/windows/nsWindow.cpp
xpcom/base/ErrorList.h
xpcom/base/nsDebugImpl.cpp
xpcom/base/nsSystemInfo.cpp
xpcom/base/nsWindowsHelpers.h
xpcom/build/IOInterposer.cpp
xpcom/build/nsXULAppAPI.h
xpcom/io/nsAppDirectoryServiceDefs.h
xpcom/io/nsLocalFileWin.cpp

Comment 31

[Kartikaya Gupta (email:[e-mail])]

Attachment: References to metro still left

And here's an updated list of references still left.

Comment 32

[Jim Mathies [:jimm]]

Comment on attachment 8593404 [details] [diff] [review]
Part 3 - Remove MOZ_METRO and friends

Review of attachment 8593404 [details] [diff] [review]:
-----------------------------------------------------------------

cc'ing rstrong on the updater and installer changes.

::: browser/branding/aurora/Makefile.in
@@ -48,5 @@
>  BRANDING_TARGET := export
>  INSTALL_TARGETS += BRANDING
> -
> -ifeq ($(MOZ_WIDGET_TOOLKIT) $(DIST_SUBDIR),windows metro)
> -VISUALMANIFEST := VisualElementsManifest.xml

did these files get deleted somewhere, I'm not seeing that.

::: browser/branding/aurora/content/Makefile.in
@@ -6,5 @@
>  #  - jars chrome artwork
> -
> -# resources needed for the metro tile interface
> -ifeq ($(MOZ_WIDGET_TOOLKIT) $(DIST_SUBDIR),windows metro)
> -TILE_FILES := $(wildcard $(srcdir)/VisualElements*)

ditto?

Comment 33

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8593402 [details] [diff] [review]
Part 2 - Remove browser/metro

r=me assuming this is a straight "hg rm browser/metro".

Comment 34

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8593409 [details] [diff] [review]
References to metro still left

>./browser/app/profile/firefox.js:// incompatibilities are ignored by updates in Metro

Should just remove this line.

>./browser/base/content/browser.js:      // Pulls in Metro controlled prefs and pushes out Desktop controlled prefs

We should remove all references to WindowsPrefSync (which covers this comment).

>./services/datareporting/DataReporting.manifest:#   metro browser:  {99bceaaa-e3c6-48c1-b981-ef9b46b67d60}
>./services/sync/SyncComponents.manifest:#   metro browser:  {99bceaaa-e3c6-48c1-b981-ef9b46b67d60}
>./services/sync/tps/extensions/mozmill/resource/driver/controller.js:    case "MetroFirefox":
>./services/sync/tps/extensions/mozmill/resource/driver/controller.js:    case "MetroFirefox":
>./services/sync/tps/extensions/mozmill/resource/driver/controller.js:    case "MetroFirefox":
>./services/sync/tps/extensions/mozmill/resource/stdlib/utils.js:  '{99bceaaa-e3c6-48c1-b981-ef9b46b67d60}': 'MetroFirefox'
>./services/sync/tps/extensions/mozmill/resource/stdlib/utils.js:    case "MetroFirefox":
>./testing/mochitest/browser-harness.xul:                    gConfig.testRoot == "metro" ? "navigator:browser" :
>./testing/mochitest/browser-test.js:      gConfig.testRoot == "metro" ||
>./toolkit/components/crashmonitor/CrashMonitor.jsm:    if (Services.metro && Services.metro.immersive) {
>./toolkit/components/crashmonitor/CrashMonitor.jsm:      OS.File.makeDir(OS.Path.join(OS.Constants.Path.profileDir, "metro"));
>./toolkit/mozapps/update/tests/data/shared.js:const PREF_APP_UPDATE_METRO_ENABLED       = "app.update.metro.enabled";
>./toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js:  Services.prefs.setBoolPref(PREF_APP_UPDATE_METRO_ENABLED, true);
>./toolkit/mozapps/update/updater/updater.cpp:      bool updateFromMetro = false;
>./toolkit/mozapps/update/updater/updater.cpp:      DWORD waitTime = updateFromMetro ?
>./toolkit/mozapps/update/updater/updater.cpp:      if (result != WAIT_OBJECT_0 && !updateFromMetro)

At least all of these should just be ripped out too. Maybe best in another bug.

Comment 35

[Robert Strong (they/them - no direct email)]

Comment on attachment 8593404 [details] [diff] [review]
Part 3 - Remove MOZ_METRO and friends

You should be able to remove these as well
./toolkit/mozapps/update/tests/data/shared.js:const PREF_APP_UPDATE_METRO_ENABLED       = "app.update.metro.enabled";
./toolkit/mozapps/update/tests/data/xpcshellUtilsAUS.js:  Services.prefs.setBoolPref(PREF_APP_UPDATE_METRO_ENABLED, true);
Please remove these if you don't mind though I am ok with doing them in a new bug.

The remaining changes needed to the installer and updater I'm fine with doing in new bugs.

Comment 36

[Kartikaya Gupta (email:[e-mail])]

Attachment: Part 3b - More removal

(In reply to Jim Mathies [:jimm] from comment #32)
> 
> did these files get deleted somewhere, I'm not seeing that.
> 

Whoops, I missed those. I'll add a new patch with the results of:
  find . -name "VisualElements*" | xargs git rm
which removed those xml files as well as the png files they referenced.

(In reply to :Gavin Sharp [email: [e-mail]] from comment #33)
> r=me assuming this is a straight "hg rm browser/metro".

It is, yes.

(In reply to :Gavin Sharp [email: [e-mail]] from comment #34)
> >./browser/app/profile/firefox.js:// incompatibilities are ignored by updates in Metro
> 
> Should just remove this line.

Done

> >./browser/base/content/browser.js:      // Pulls in Metro controlled prefs and pushes out Desktop controlled prefs
> 
> We should remove all references to WindowsPrefSync (which covers this
> comment).

Done

> At least all of these should just be ripped out too. Maybe best in another
> bug.

Done

All the above comments are addressed in this part 3b, which I'll squash into part 3 on landing. Kept it separate for easier review. I'll do another try push as well.

Comment 37

[Kartikaya Gupta (email:[e-mail])]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=edb620097b99

Comment 38

[Kartikaya Gupta (email:[e-mail])]

Attachment: References to metro still left

Comment 39

[Robert Strong (they/them - no direct email)]

Comment on attachment 8593718 [details] [diff] [review]
Part 3b - More removal

Looks good to me!

Comment 40

[Kartikaya Gupta (email:[e-mail])]

Thanks! Am I good to land this now, or is there somebody else who should still review these changes?

Comment 41

[Kartikaya Gupta (email:[e-mail])]

(gavin said on IRC he'd find somebody, but not sure if that was :rstrong or somebody else...)

Comment 42

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8593404 [details] [diff] [review]
Part 3 - Remove MOZ_METRO and friends

>diff --git a/browser/branding/aurora/Makefile.in b/browser/branding/aurora/Makefile.in

>-# On Windows only do this step for browser, skip for metro.
> ifeq ($(MOZ_WIDGET_TOOLKIT) $(DIST_SUBDIR),windows browser)

You can remove the DIST_SUBDIR check and make this just:

ifeq ($(MOZ_WIDGET_TOOLKIT),windows)

Same comment applies to the other branding package Makefile.ins (browser/branding/nightly/Makefile.in, browser/branding/official/Makefile.in, browser/branding/unofficial/Makefile.in, etc.)

I haven't reviewed this fully, I will try to get back to it soon.

Comment 43

[Jim Mathies [:jimm]]

Should someone like Mark Mayo have the opportunity to sign off on this as well?

Comment 44

[:Gavin Sharp [email: [e-mail]]]

Looking at patch part 3, I'm a bit surprised that we seem to have some examples of code in (XRE_GetWindowsEnvironment() == WindowsEnvironmentType_Metro) conditionals that is further #ifdef'ed MOZ_METRO.

Was it really possible to somehow run a non-MOZ_METRO build such that (XRE_GetWindowsEnvironment() == WindowsEnvironmentType_Metro), or was that just guaranteed dead code?

Comment 45

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 8593404 [details] [diff] [review]
Part 3 - Remove MOZ_METRO and friends

>diff --git a/browser/locales/en-US/chrome/browser/preferences/advanced.dtd b/browser/locales/en-US/chrome/browser/preferences/advanced.dtd

>-<!-- Note either updateAuto1 is used or (updateAutoMetro and updateAutoDesktop),
>+<!-- Note either updateAuto1 is used or updateAutoDesktop,

The updateAutoDesktop strings are no longer used after this patch, and so they can be removed also. You can then just remove this L10N note entirely.

>diff --git a/config/recurse.mk b/config/recurse.mk

> # The rules here are doing directory traversal, so we don't want further
> # recursion to happen when running make -C subdir $tier. But some make files
> # further call make -C something else, and sometimes expect recursion to
>-# happen in that case (see browser/metro/locales/Makefile.in for example).
>+# happen in that case.
> # Conveniently, every invocation of make increases MAKELEVEL, so only stop
> # recursion from happening at current MAKELEVEL + 1.

Was that Metro Makefile the only Makefile that did this? Perhaps we can rip out more of this stuff, or find another example? Separate bug.

>diff --git a/dom/base/nsContentUtils.h b/dom/base/nsContentUtils.h

>    * Making the fullscreen API content only is useful on platforms where we
>    * still want chrome to be visible or accessible while content is
>-   * fullscreen, like on Windows 8 in Metro mode.
>+   * fullscreen.

Looking at this code, Metro seems to be the only thing that sets full-screen-api.content-only=true. So we can probably rip out a bunch more related code. This can be a new bug too.

>diff --git a/testing/profiles/prefs_general.js b/testing/profiles/prefs_general.js

>-// prefs for firefox metro.
> // Disable first-tun tab
> user_pref("browser.firstrun.count", 0);

At least this pref (and maybe some of the ones after it?) is Metro-only and can be removed.

>diff --git a/toolkit/mozapps/update/nsIUpdateService.idl b/toolkit/mozapps/update/nsIUpdateService.idl

>   readonly attribute boolean isOtherInstanceHandlingUpdates;

This stuff could probably all be ripped out too, though maybe we want to leave it. Separate bug.

>diff --git a/toolkit/xre/nsAppRunner.cpp b/toolkit/xre/nsAppRunner.cpp

>+ * @return true in all environments
> */
> static bool
> CanShowProfileManager()

This is probably not useful anymore?

>diff --git a/tools/profiler/platform.cpp b/tools/profiler/platform.cpp

> static const char * gGeckoThreadName = "GeckoMain";

This may no longer be useful either, maybe also worth a separate bug.

r=me with those addressed / bugs filed.

Comment 46

[Robert Strong (they/them - no direct email)]

(In reply to :Gavin Sharp [email: [e-mail]] from comment #45)
>...
> >diff --git a/toolkit/mozapps/update/nsIUpdateService.idl b/toolkit/mozapps/update/nsIUpdateService.idl
> 
> >   readonly attribute boolean isOtherInstanceHandlingUpdates;
> 
> This stuff could probably all be ripped out too, though maybe we want to
> leave it. Separate bug.
That is definitely not for metro so no need.

Comment 47

[Jim Mathies [:jimm]]

(In reply to :Gavin Sharp [email: [e-mail]] from comment #44)
> Looking at patch part 3, I'm a bit surprised that we seem to have some
> examples of code in (XRE_GetWindowsEnvironment() ==
> WindowsEnvironmentType_Metro) conditionals that is further #ifdef'ed
> MOZ_METRO.
> 
> Was it really possible to somehow run a non-MOZ_METRO build such that
> (XRE_GetWindowsEnvironment() == WindowsEnvironmentType_Metro), or was that
> just guaranteed dead code?

The reverse was true, a MOZ_METRO build where XRE_GetWindowsEnvironment() != WindowsEnvironmentType_Metro. Looking at one example here, the modules/libpref/Preferences.cpp code.

Comment 48

[Kartikaya Gupta (email:[e-mail])]

(In reply to :Gavin Sharp [email: [e-mail]] from comment #45)
> The updateAutoDesktop strings are no longer used after this patch, and so
> they can be removed also. You can then just remove this L10N note entirely.

I removed this in my local patches.

> > # Conveniently, every invocation of make increases MAKELEVEL, so only stop
> > # recursion from happening at current MAKELEVEL + 1.
> 
> Was that Metro Makefile the only Makefile that did this? Perhaps we can rip
> out more of this stuff, or find another example? Separate bug.

Filed bug 1157684 for this.

> Looking at this code, Metro seems to be the only thing that sets
> full-screen-api.content-only=true. So we can probably rip out a bunch more
> related code. This can be a new bug too.

Filed bug 1157685 for this.

> > user_pref("browser.firstrun.count", 0);
> 
> At least this pref (and maybe some of the ones after it?) is Metro-only and
> can be removed.

Removed this locally. The next few prefs seem to be not Metro-only so I left that as-is. If you have specific prefs in mind let me know.

> >diff --git a/toolkit/mozapps/update/nsIUpdateService.idl b/toolkit/mozapps/update/nsIUpdateService.idl
> 
> >   readonly attribute boolean isOtherInstanceHandlingUpdates;
> 
> This stuff could probably all be ripped out too, though maybe we want to
> leave it. Separate bug.

Didn't file a bug for this, as per comment 46.

> > CanShowProfileManager()
> 
> This is probably not useful anymore?

Filed bug 1157686 for this.

> > static const char * gGeckoThreadName = "GeckoMain";
> 
> This may no longer be useful either, maybe also worth a separate bug.

This looks like it is used in the profiler for other platforms, so I left it in. (I've definitely seen threads labelled GeckoMain in non-metro profiles, so I'm pretty sure this is used).

Did a new try-push with the above changes, parts 3+4 squashed, and rebased to latest m-c:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=8e17f6dc0ec8

I'll land it if the try run is green.

Comment 49

[Kartikaya Gupta (email:[e-mail])]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #48)
> Removed this locally. The next few prefs seem to be not Metro-only so I left
> that as-is. If you have specific prefs in mind let me know.

Some grepping showed that loop.throttled and browser.uitour.pinnedTabUrl are two prefs in that file that aren't referenced anywhere else in m-c. Not sure if either of those are metro-specific though.

Comment 50

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/03b0f9b58e7e
https://hg.mozilla.org/integration/mozilla-inbound/rev/f545b6dfa3da
https://hg.mozilla.org/integration/mozilla-inbound/rev/bf1d0c9242d9

Comment 51

[Kartikaya Gupta (email:[e-mail])]

Part 3 also included a UUID bump to nsIAppStartup which the mercurial push hook caught for me :)

Comment 52

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/03b0f9b58e7e
https://hg.mozilla.org/mozilla-central/rev/f545b6dfa3da
https://hg.mozilla.org/mozilla-central/rev/bf1d0c9242d9

Comment 53

[Maksim Lebedev]

MetroFireFox, I will miss You!

Comment 54

[Francesco Lodolo [:flod]]

Still trying to figure out what happened to filter.py, but now all localized searchplugins are reported as obsolete, plus keys in region.properties. To be honest I'm tempted to ask a back-out

Example: https://l10n.mozilla.org/dashboard/compare?run=489973

Comment 55

[Kartikaya Gupta (email:[e-mail])]

Where do the localized searchplugins come from? They're not in my mozilla-central tree and I don't know very much about how the l10n process works, sorry.

Comment 56

[Kartikaya Gupta (email:[e-mail])]

Doh this is a stupid mistake on my part. Missing ')' in filter.py. I'll land a follow-up.

Comment 57

[Francesco Lodolo [:flod]]

Yep, confirming the issue (it took me way too long to find it)

if (re.match(r"searchplugins\/.+\.xml", path):

The first parenthesis should not be there at all

if re.match(r"searchplugins\/.+\.xml", path):

Comment 58

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/026bec3cff57

Comment 59

[Kartikaya Gupta (email:[e-mail])]

(In reply to Francesco Lodolo [:flod] (UTC+2) from comment #57)
> Yep, confirming the issue (it took me way too long to find it)
> 
> if (re.match(r"searchplugins\/.+\.xml", path):
> 
> The first parenthesis should not be there at all
> 
> if re.match(r"searchplugins\/.+\.xml", path):

Oh, I did it the C++ way and left the extra parentheses in. It should work either way, next time this file is touched somebody can take it out. Or if you want me to do it anyway let me know.

Comment 60

[Francesco Lodolo [:flod]]

(In reply to Kartikaya Gupta (email:[e-mail]) from comment #59)
> Oh, I did it the C++ way and left the extra parentheses in. It should work
> either way, next time this file is touched somebody can take it out. Or if
> you want me to do it anyway let me know.

No need for me.

Comment 61

[aleth [:aleth]]

https://hg.mozilla.org/comm-central/rev/dc79be9b874e

Comment 62

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/026bec3cff57