RemotePageManager.jsm is currently loaded from toolkit/content/process-content.js which is a process script loaded by MainProcessSingleton.js during app-startup. It seems that this could instead be loaded lazily the first time it's used.

RemotePageManager.jsm is needed as soon as a RemotePage is instantiated in the parent process. It's been a while since I looked at this stuff, but I do think that setting up the "ports" can be done lazily when the content process first loads a page in the RemotePageManager:urls list[1], _or_ a registration message is received [2]. For that last, kmag came up with a model for loading modules on messages that we might want to adopt: http://searchfox.org/mozilla-central/rev/00fa5dacedb925022f53d025121f1a919508e7ce/browser/components/nsBrowserGlue.js#74-205 Mossop might also have more input / ideas here, so ni?ing him. [1]: http://searchfox.org/mozilla-central/rev/00fa5dacedb925022f53d025121f1a919508e7ce/toolkit/modules/RemotePageManager.jsm#530 [2]: http://searchfox.org/mozilla-central/rev/00fa5dacedb925022f53d025121f1a919508e7ce/toolkit/modules/RemotePageManager.jsm#550-554

What are we winning when replacing a module loaded too early with another module loaded as early? Is this code needed before we initialize nsBrowserGlue.js (which already contains lazy module getters triggered by observer notifications)?

This patch seems to work. I am still waiting on Try, but I ran it through the tests that I think are relevant and it passed. However, I am frankly not super happy with how the patch looks. It seems quite weird to me that nsBrowserGlue.js [1] seems to just assume that any module in |listeners.observers| [2] has a |ModuleName.observe| function that should be connected to all events specified in |listeners.observers|. I am also not real happy moving the calls to |Services.obs.addObserver| out of RemotePageManager.jsm, and then duplicating them in nsBrowserGlue.js and process-content.js. It seems to violate the principles of encapsulation and code reuse. However, I do not have any other ideas for how to fix this without introducing another early loading module. So maybe this is good enough? What do you all think? Is this ok? [1] http://searchfox.org/mozilla-central/rev/6326724982c66aaeaf70bb7c7ee170f7a38ca226/browser/components/nsBrowserGlue.js#168-176 [2] http://searchfox.org/mozilla-central/rev/6326724982c66aaeaf70bb7c7ee170f7a38ca226/browser/components/nsBrowserGlue.js#115-118

To be clear, my issue with |ModuleName.observe| is the name. It feels like a "magic string". There doesn't seem to be any documentation, standard or even a comment indicating that this is what the name needs to be. It seems to just be a silent requirement.

Does this actually help? It looks like it moves loading of RemotePageManager from nsBrowserGlue startup to the load of the first chrome window (the main browser window). That just sounds slightly later in startup to me.

@florian Can you help me figure out the answer to :mossop's question? I am not sure how much later in startup this module would need to be to make an impact.

(In reply to Dave Townsend [:mossop] from comment #8) > Does this actually help? It looks like it moves loading of RemotePageManager > from nsBrowserGlue startup to the load of the first chrome window (the main > browser window). That just sounds slightly later in startup to me. I know very little about RemotePageManager, but if I remember correctly Mike told me that it's used when loading about: pages. The first about: page we can load is likely about:home or about:sessionrestore, and we don't start these loads before first paint, so I'm hoping we may be able to delay the RemotePageManager initialization until after first paint. If we can't, we are probably indeed not going to win much.

So I do not know much about this test [1], but it looks like you have worked on it before. As you can see, my patch removes RemotePageManager.jsm from this [2] line. The test passes this way. My understanding from my brief inspection of the test is that it checks what modules have been loaded by different stages in startup. Since the test has a section called "before first paint" [3], which passes without adding RemotePageManager.jsm to the modules list, I believe that RemotePageManager.jsm is being loaded after the first paint. Do you know enough about that test to tell me if my reasoning is correct? Also, thank you for answering my question from Comment 9 but you did not address my original concerns in Comment 6. Do you think that my patch violates best coding practices, or is it acceptable? [1] http://searchfox.org/mozilla-central/rev/05c4c3bc0cfb9b0fc66bdfc8c47cac674e45f151/browser/base/content/test/performance/browser_startup.js [2] http://searchfox.org/mozilla-central/rev/05c4c3bc0cfb9b0fc66bdfc8c47cac674e45f151/browser/base/content/test/performance/browser_startup.js#44 [3] http://searchfox.org/mozilla-central/rev/05c4c3bc0cfb9b0fc66bdfc8c47cac674e45f151/browser/base/content/test/performance/browser_startup.js#61

(In reply to Kirk Steuber [:bytesized] from comment #11) > So I do not know much about this test [1], but it looks like you have worked > on it before. As you can see, my patch removes RemotePageManager.jsm from > this [2] line. The test passes this way. My understanding from my brief > inspection of the test is that it checks what modules have been loaded by > different stages in startup. Since the test has a section called "before > first paint" [3], which passes without adding RemotePageManager.jsm to the > modules list, I believe that RemotePageManager.jsm is being loaded after the > first paint. Do you know enough about that test to tell me if my reasoning > is correct? The "before first paint" is a blacklist of modules, so clearly it passes if the module is not in the list. You should try adding it there and see if the test still passes (if it passes, it means that the module is not loaded during first paint).

(In reply to Kirk Steuber [:bytesized] from comment #11) > Since the test has a section called "before > first paint" [3], which passes without adding RemotePageManager.jsm to the > modules list, I believe that RemotePageManager.jsm is being loaded after the > first paint. Do you know enough about that test to tell me if my reasoning > is correct? Yes I know enough about this test, I wrote it :). And no, unfortunately this reasoning is not correct, because the "before first paint" section contains a blacklist, not a whitelist. (In reply to Kirk Steuber [:bytesized] from comment #6) > However, I am frankly not super happy with how the patch looks. It seems > quite weird to me that nsBrowserGlue.js [1] seems to just assume that any > module in |listeners.observers| [2] has a |ModuleName.observe| function that > should be connected to all events specified in |listeners.observers|. I don't see a problem with that, observers are expected to implement an 'observe' method, it's defined in the nsIObserver interface http://searchfox.org/mozilla-central/rev/05c4c3bc0cfb9b0fc66bdfc8c47cac674e45f151/xpcom/ds/nsIObserver.idl#33 > I am also not real happy moving the calls to |Services.obs.addObserver| out > of RemotePageManager.jsm, and then duplicating them in nsBrowserGlue.js and > process-content.js. It seems to violate the principles of encapsulation and > code reuse. The duplication doesn't smell great. I don't have any great suggestion to offer though :-/.

Thank you for explaining what I was missing in browser_startup.js. That makes much more sense. I can now see that RemotePageManager.jsm is indeed loading before the first paint. It looks like the load is happening earlier than I had hoped because of this line [1] in the AddonManager. I will look into having the AddonManager load RemotePageManager.jsm later in order to move the load after the first paint. [1] http://searchfox.org/mozilla-central/rev/1c13d5cf85f904afb8976c02a80daa252b893fca/toolkit/mozapps/extensions/AddonManager.jsm#908

Comment on attachment 8907810 [details] Bug 1369466 - Load RemotePageManager.jsm lazily https://reviewboard.mozilla.org/r/179498/#review188960 Sorry for the delay. The patch looks good to me overall, but it only moves this to right after first paint. I wonder if it would be possible to make this actually lazy. I don't know much about the RemotePageManager code though, so I'll defer to mconley. ::: toolkit/content/process-content.js:24 (Diff revision 4) > +// This wrapper is needed to ensure that RemotePageManager.jsm is not loaded > +// earlier than necessary. > +function remotePageManagerObserverWrapper(subject, topic, data) { > + let url = subject.document.documentURI; > + if (url == "about:blank" || > + url == "chrome://gfxsanity/content/sanitytest.html") { Are there other pages we should filter out? Or do we have a way to know more specifically for which pages RemotePageManager is going to be useful? Is it only for about: pages? ::: toolkit/modules/RemotePageManager.jsm:564 (Diff revision 4) > > +// The public API for RemotePageManagerInternal > +this.RemotePageManager = { > + addRemotePageListener: RemotePageManagerInternal.addRemotePageListener.bind(RemotePageManagerInternal), > + removeRemotePageListener: RemotePageManagerInternal.removeRemotePageListener.bind(RemotePageManagerInternal), > + observe, I would inline the observe method here. An object that has an 'observe' method looks like an observer, but that "var observe = (window) => {" separated function is less obvious at first sight. Also, it doesn't use "this" so it doesn't need to be an arrow function. ::: toolkit/mozapps/extensions/AddonManager.jsm:1119 (Diff revision 4) > Services.prefs.removeObserver(PREF_EM_STRICT_COMPATIBILITY, this); > Services.prefs.removeObserver(PREF_EM_CHECK_UPDATE_SECURITY, this); > Services.prefs.removeObserver(PREF_EM_UPDATE_ENABLED, this); > Services.prefs.removeObserver(PREF_EM_AUTOUPDATE_DEFAULT, this); > Services.prefs.removeObserver(PREF_EM_HOTFIX_ID, this); > + if (gPluginPageListener != null) { nit: "!= null" isn't needed for a simple null check.

Comment on attachment 8907810 [details] Bug 1369466 - Load RemotePageManager.jsm lazily https://reviewboard.mozilla.org/r/179498/#review188960 Unfortunaly, making the module load in a more traditionally lazy manner would sort of require one of two things that I do not want to do. Either I would have to explicitly wait for the page loads that we know that RemotePageManager.jsm's clients are interested in. However, if we are going to do that, we should really hardcode those URLs into RemotePageManager.jsm. As written, the implication is that it will work on whatever page you give it. If we hardcoded the URLs that it listens to into process-content.js, obviously only the URLs hardcoded there would actually work. I suppose we *could* do this, but it sounds kinda messy and I would rather not do it that way. Alternately, we would need to somehow move the bit that registers which URLs we care about from RemotePageManager.jsm to process-content.js. I would really rather not do that, since I cannot think of any way to do it cleanly. > Are there other pages we should filter out? Or do we have a way to know more specifically for which pages RemotePageManager is going to be useful? Is it only for about: pages? Honestly, I am not sure if there are other pages that ought to be filtered out. I only filtered these ones out because the module would consistently be loaded too early otherwise. As I mentioned above, we could potentially filter out all but the specific addresses that we want but the methods of doing so seem problematic. :mconley - When you are reviewing this, please let me know if you have any feedback on this.

Comment on attachment 8907810 [details] Bug 1369466 - Load RemotePageManager.jsm lazily https://reviewboard.mozilla.org/r/179498/#review194914 Hey bytesized - sorry for the really long review delay here. :( This patch makes sense - I see what you're doing, and I think this is the way to do it (with some minor modificiations). See questions below. ::: toolkit/content/process-content.js:22 (Diff revision 5) > + let url = subject.document.documentURI; > + if (url == "about:blank" || > + url == "chrome://gfxsanity/content/sanitytest.html") { > + return; > + } > + RemotePageManager.observe(subject, topic, data); Seems to me like we _could_ hold a copy of the RemotePageManager:urls in process-content.js as well, and compare the URI against that list. Once we get a match, _then_ we run RemotePageManager.observe. Do we know what this buys us in practice, btw? Do we, for example, see an improvement in the cpstartup Talos test? (Which measures content process start-up time). ::: toolkit/content/process-content.js:27 (Diff revision 5) > + let url = subject.document.documentURI; > + if (url == "about:blank" || > + url == "chrome://gfxsanity/content/sanitytest.html") { > + return; > + } > + RemotePageManager.observe(subject, topic, data); Should we remove the observer wrapper observers here? ::: toolkit/content/process-content.js:40 (Diff revision 5) > +if (gInContentProcess) { > + if (Services.cpmm.initialProcessData["RemotePageManager:initialized"]) { > + RemotePageManager; > + } else { > + Services.cpmm.addMessageListener("RemotePage:Init", () => { > + RemotePageManager; Shouldn't we remove this message listener once we receive this message? ::: toolkit/modules/RemotePageManager.jsm:579 (Diff revision 5) > + const { AddonManagerPrivate } = Cu.import("resource://gre/modules/AddonManager.jsm", {}); > + AddonManagerPrivate.initRemotePageListener(); Mmm, this feels a bit strange to couple RemotePageManager.jsm with AddonManager.jsm... and to call into AddonManagerPrivate. It might be better to send a notification over the nsIObserverService instead alerting things in the parent process that the RPM has been initialized.

The commit that I just posted is simply a rebase of the previous patch. Sometimes the interdiff for a rebased revision shows odd things, so I have started posting them separately from other revisions.

Comment on attachment 8907810 [details] Bug 1369466 - Load RemotePageManager.jsm lazily https://reviewboard.mozilla.org/r/179498/#review194914 > Seems to me like we _could_ hold a copy of the RemotePageManager:urls in process-content.js as well, and compare the URI against that list. Once we get a match, _then_ we run RemotePageManager.observe. > > Do we know what this buys us in practice, btw? Do we, for example, see an improvement in the cpstartup Talos test? (Which measures content process start-up time). It looks like there *might* be a minor improvement in the cpstartup Talos test [1]. [1] https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&newProject=try&newRevision=4f256a410a41&framework=1&selectedTimeRange=172800

(In reply to Kirk Steuber [:bytesized] from comment #24) > > It looks like there *might* be a minor improvement in the cpstartup Talos > test [1]. > Hm. I'm not seeing it. Would you be able to push to try the revision of mozilla-central this is based on and compare that way? If it turns out that this isn't improving the content process start-up time, then (and I super-hate to say this because I know how hard you've worked on sorting this stuff out), I'm not certain that the complexity of loading RemotePageManager.jsm lazily is worth it. Like, if there's no measurable impact, that's good to know... It might also be worth checking to see if disabling RemotePageManager.jsm _entirely_ has any impact on cpstartup, or first paint time, or anything like that. Like, just comment it out entirely and see if any of our Talos tests notice. If there's no change, then either: 1) Making RPM lazy is not having any measurable impact on start-up performance and we should abandon this effort, or 2) Our tests aren't capturing the performance improvement, and we should try to figure out where the performance improvement actually is in order to justify the added complexity of loading RPM lazily. Again, I know this is kind of a pain - we filed this bug, it was kind of speculative, and then you did all of this work to make it happen, and now I'm throwing into question whether it's worth actually fixing. I understand that sucks. :( If I could go back in time, I'd probably argue that we should have done a small experiment to see if removing RPM entirely has any discernable impact on start-up time. But, well, here we are.

Comment on attachment 8907810 [details] Bug 1369466 - Load RemotePageManager.jsm lazily https://reviewboard.mozilla.org/r/179498/#review195988 Clearing review until we understand what this work buys us, performance-wise.

I'm happy to try to investigate whether or not making RPM lazy has a positive impact - I don't think we need to make you responsible for that; we should have done that before we started writing the patch. If you _do_ want to do that investigation, by all means, but let me know if you want me to do the comparison, and I will.

If you could do the investigation, that would be really awesome. Thanks for helping me out with this.

(In reply to Mike Conley (:mconley) (:⚙️) - Backlogged on reviews and needinfos from comment #25) > If I could go back in time, I'd probably argue that we should > have done a small experiment to see if removing RPM entirely has any > discernable impact on start-up time. But, well, here we are. Most of the files we removed from the startup path didn't have a measurable impact on startup performance by themselves. Together though, it was significant. On a startup profile on a super slow machine, RemotePageManager.jsm looks like 20ms (out of 6s) https://perfht.ml/2imcfd3 So I would expect a 2ms win on a normal machine. That's within noise level for most tests, but these small wins do add up.

So what do we want to do here? Should we merge? Forget the patch altogether? Do something else?

Hey bytesized, Sorry, was traveling, but am back now. I'll own having a decision on this this week sometime.

Comment on attachment 8907810 [details] Bug 1369466 - Load RemotePageManager.jsm lazily https://reviewboard.mozilla.org/r/179498/#review201096 Thanks bytesized. I've been looking at this today, and I think we can make this work, but I'm a bit worried about the RPM logic falling into the AddonManager. I have a suggestion below. ::: toolkit/content/process-content.js:19 (Diff revision 7) > +// Once the chrome process has loaded the RemotePageManager, trigger the load of > +// the RemotePageManager in this process immediately. > +if (gInContentProcess) { > + if (Services.cpmm.initialProcessData["RemotePageManager:initialized"]) { > + RemotePageManager; > + } else { > + Services.cpmm.addMessageListener("RemotePage:Init", function RPMInitListener() { > + Services.cpmm.removeMessageListener("RemotePage:Init", RPMInitListener); > + RemotePageManager; > + }); > + } > +} I think we need some documentation here to describe what these two conditionals are doing. Namely, (I believe) we have: 1. The first condition which sets up handlers for the content process if and when the RPM needs to be loaded based on the parent process sending RPM state. 2. The second condition, which is for either process, when a document loads, and RPM hasn't been loaded yet, but might be necessary for the page about to load. ::: toolkit/mozapps/extensions/AddonManager.jsm:908 (Diff revision 7) > - let { RemotePages } = Cu.import("resource://gre/modules/RemotePageManager.jsm", {}); > - > - gPluginPageListener = new RemotePages("about:plugins"); > - gPluginPageListener.addMessageListener("RequestPlugins", this.requestPlugins); > + if (Services.cpmm.initialProcessData["RemotePageManager:initialized"]) { > + this.initRemotePageListener(); > + } else { > + // Add about:plugins to the RemotePageManager:urls list. This ensures > + // that the remote page listener will be initialized when the > + // about:plugins page is opened (if not before). > + if (Services.ppmm.initialProcessData["RemotePageManager:urls"]) { > + Services.ppmm.initialProcessData["RemotePageManager:urls"].push("about:plugins"); > + } else { > + Services.ppmm.initialProcessData["RemotePageManager:urls"] = ["about:plugins"]; > + } > + let initRemotePageListener = this.initRemotePageListener; > + Services.cpmm.addMessageListener("RemotePage:Init", function RPMInitListener() { > + Services.cpmm.removeMessageListener("RemotePage:Init", RPMInitListener); > + initRemotePageListener(); > + }); > + } Hrm. We're leaking implementation detail about RPM into AddonManager.jsm. :/ I've been looking over this patch for a while now, and I think what I'm noticing is that a lot of effort is (understandably) going into attempting to prepare things so that RemotePageManager only needs to load at the last minute. But that means that we end up leaking a bunch of implementation detail like this into modules that really don't need to know about RPM's lazy mechanism. Could we instead add an observer here for some kind of notification that is sent from RPM when it's loaded? And at _that_ point, can we tell RPM about about:plugins and register our RemotePage?

Mossop and I chatted about this today, and he's mulling over how we want to proceed here.

Ok, how about this as a proposal. We have very few consumers of this module in-tree so we should be pretty free to make changes that support them specifically. This is sort of ugly, but I guess in the names of perf: Hardcode a list of URLs to listen to from the content process side. When an interesting URL loads have it message the parent process. Parent process loads RemotePageManager.jsm which has a url <=> module mapping. Parent process passes a new MessagePort to the module which then sets up message listeners on the port. Wavy hands for testing :( Thoughts?

I would like Florian's input on this too. Personally, I think that every idea I have come up with or heard pitched for this is just too much obfuscation for too little performance gain. If there are other modules having similar problems to this one, it would probably make sense to write a module that can lazy load all of them. But if not, I kind of feel like we should just leave it alone. As Florian mentioned in Comment 29, this optimization just won't have that big of an impact on its own; the impact will come from many of these small wins. To me that means that either: a) We can afford to lose this small optimization in the name of readability, or b) There are other modules with the same problem as this one, justifying the creation of a module that can lazy load all of them properly.

Marking as blocking bug 1336227 because this code currently gets loaded before the early blank window has a chance to be displayed.

(In reply to Kirk Steuber [:bytesized] from comment #35) > I would like Florian's input on this too. Clearing my needinfo for now as I'll be away for a few weeks and I don't want this to remain blocked on my input while away. Sorry for not replying earlier. Just 2 cents: a big refactoring for a small win may not be worth it, so if we want a cheap small win, we could just delay a bit the initial import of RemotePageManager.jsm so that it's done after the initial blank window is shown. Of course making it lazy (so that it hopefully initializes after the full browser UI is painted in most cases) is still much better, but we could do incremental improvements instead.

So the way I approached this was to first split up RemotePageManager.jsm in parts, separating the code meant for the parent and content processes, and then adjust consumers accordingly. Then I moved the observers out of the RemotePageManagerChild.jsm into process-content.js, which will remain as the only piece of code loaded related to it until a registered page is loaded. The "source of truth" for the list of registered pages remains initialProcessData (or, rather, its new replacement, `sharedData`). With this, the `new RemotePages(..)` API was kept, and it's simpler to change it now to a pre-registration list if still wanted (I'll file a separate bug about this tomorrow) Patches incoming

(Sorry for the churn, there were some path/eslint fixups after I moved the test to the new folder) The version posted now should be complete and I just sent it to tryserver: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0f3f0f6ac3f2802bedc311a539e163d65533214a

Comment on attachment 8994999 [details] Bug 1369466 - Split the RemotePageManager.jsm code into parent/child sides, and put it in a new location. https://reviewboard.mozilla.org/r/259478/#review266784

Comment on attachment 8995000 [details] Bug 1369466 - Change consumers to the new location of RemotePageManager. https://reviewboard.mozilla.org/r/259480/#review266786

Comment on attachment 8995001 [details] Bug 1369466 - Don't load the RemotePageManager on content processes before necessary. https://reviewboard.mozilla.org/r/259482/#review266788

Comment on attachment 8995002 [details] Bug 1369466 - Use Services.ppmm.sharedData instead of initialProcessData on the RemotePageManager. https://reviewboard.mozilla.org/r/259484/#review266792

Comment on attachment 8995003 [details] Bug 1369466 - Remove entries related to the RemotePageManager from the startup tests whitelists. https://reviewboard.mozilla.org/r/259486/#review266794

Comment on attachment 8995004 [details] Bug 1369466 - Move RemotePageManager test to its new location. https://reviewboard.mozilla.org/r/259488/#review266796

Comment on attachment 8995005 [details] Bug 1369466 - Remove the old RemotePageManager.jsm code. https://reviewboard.mozilla.org/r/259490/#review266798 Please also remove it from tools/lint/eslint/modules.json.

We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. hg error in cmd: hg rebase -s 7c028d26575dadb8c90cb9cbf868ee5d013be672 -d 357206a19974: rebasing 474797:7c028d26575d "Bug 1369466 - Split the RemotePageManager.jsm code into parent/child sides, and put it in a new location. r=mossop" merging toolkit/modules/RemotePageManager.jsm and toolkit/components/remotepagemanager/MessagePort.jsm to toolkit/components/remotepagemanager/MessagePort.jsm merging toolkit/modules/RemotePageManager.jsm and toolkit/components/remotepagemanager/RemotePageManagerChild.jsm to toolkit/components/remotepagemanager/RemotePageManagerChild.jsm merging toolkit/modules/RemotePageManager.jsm and toolkit/components/remotepagemanager/RemotePageManagerParent.jsm to toolkit/components/remotepagemanager/RemotePageManagerParent.jsm warning: conflicts while merging toolkit/components/remotepagemanager/MessagePort.jsm! (edit, then use 'hg resolve --mark') warning: conflicts while merging toolkit/components/remotepagemanager/RemotePageManagerChild.jsm! (edit, then use 'hg resolve --mark') unresolved conflicts (see hg resolve, then hg rebase --continue)

I need to wait until bug 1478308 gets merged to mozilla-central to rebase

Pushed by felipc@gmail.com: https://hg.mozilla.org/integration/autoland/rev/df67254b8e79 Split the RemotePageManager.jsm code into parent/child sides, and put it in a new location. r=mossop https://hg.mozilla.org/integration/autoland/rev/6aa03eee1597 Change consumers to the new location of RemotePageManager. r=mossop https://hg.mozilla.org/integration/autoland/rev/652158e8839c Don't load the RemotePageManager on content processes before necessary. r=mossop https://hg.mozilla.org/integration/autoland/rev/b403c2fa1f29 Use Services.ppmm.sharedData instead of initialProcessData on the RemotePageManager. r=mossop https://hg.mozilla.org/integration/autoland/rev/5a821925cc49 Remove entries related to the RemotePageManager from the startup tests whitelists. r=mossop https://hg.mozilla.org/integration/autoland/rev/2a8afa0f67cb Move RemotePageManager test to its new location. r=mossop https://hg.mozilla.org/integration/autoland/rev/43d426747b30 Remove the old RemotePageManager.jsm code. r=mossop

https://hg.mozilla.org/mozilla-central/rev/df67254b8e79 https://hg.mozilla.org/mozilla-central/rev/6aa03eee1597 https://hg.mozilla.org/mozilla-central/rev/652158e8839c https://hg.mozilla.org/mozilla-central/rev/b403c2fa1f29 https://hg.mozilla.org/mozilla-central/rev/5a821925cc49 https://hg.mozilla.org/mozilla-central/rev/2a8afa0f67cb https://hg.mozilla.org/mozilla-central/rev/43d426747b30

Commit pushed to master at https://github.com/mozilla/activity-stream https://github.com/mozilla/activity-stream/commit/2848d503ea9c567c3a641a3cf9c3a395c170087c chore(mc): Port Bug 1369466 - Change consumers to the new location of RemotePageManager. r=mossop (#4268) Manually add RemotePages global until eslint-plugin-mozilla is updated.

Some improvements that I found on talos that seem related to this bug: 3.5% cpstartup, at least on windows https://treeherder.mozilla.org/perf.html#/graphs?series=mozilla-central,1652483,1,1&series=mozilla-central,1649840,1,1&series=mozilla-central,1717668,1,1&zoom=1532592051898.3052,1532726932372.8813,135,150.95505560828505 And 1% Base Content JS memory on all platforms https://treeherder.mozilla.org/perf.html#/graphs?series=mozilla-central,1684866,1,4&series=mozilla-central,1684850,1,4&series=mozilla-central,1685137,1,4&zoom=1532563576708.2908,1532803511663.255,6300000,6600000&selected=mozilla-central,1685137,360951,516852443,4