1371335 - webcompat-reporter shouldn't load anything before opening and painting the first browser window

Status: RESOLVED FIXED

(Web Compatibility :: Tooling & Investigations, enhancement)

Description

[(no longer active)]

browser/base/content/test/performance/browser_startup.js indicates that the following scripts are loaded before opening the first window:

chrome://webcompat-reporter/content/TabListener.jsm
chrome://webcompat/content/lib/ua_overrider.jsm

Comment 1

[Mike Taylor [:miketaylr]]

ooh, that seems bad.

Ehsan, are there any relevant bugs I can look at for pointers on how to defer loading?

Comment 2

[(no longer active)]

https://searchfox.org/mozilla-central/rev/1a054419976437d0778a2b89be1b00207a744e94/browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm#35 can be made a lazy getter using XPCOMUtils.defineLazyModuleGetter().  You should be able to find piles of examples for it.

https://searchfox.org/mozilla-central/rev/1a054419976437d0778a2b89be1b00207a744e94/browser/extensions/webcompat/bootstrap.js#13 is already a lazy getter, so some code is accessing the UAOverrider object too early.  Judging based on the names of the functions in question that's probably https://searchfox.org/mozilla-central/rev/1a054419976437d0778a2b89be1b00207a744e94/browser/extensions/webcompat/bootstrap.js#63.  You'll need to find a way to defer this work to later.

But before you do that you should first ask a more important question: does this code have a good raison d'etre?  As far as I can tell, it's just duplicating the existing Gecko's functionality: https://searchfox.org/mozilla-central/source/netwerk/protocol/http/UserAgentOverrides.jsm  If my understanding is correct, the desirable remedy for this code would be to delete it and use Necko's support for overriding UA strings which already works and is well tested.

Comment 3

[Mike Taylor [:miketaylr]]

Thanks for the pointers -- let me pass off the question about overrides to Dennis, he owns the ua_overrider code (the "webcompat" and "webcompat-reporter" are different,  though similarly named addons :/).

Comment 4

[Dennis Schubert [:denschub]]

> If my understanding is correct, the desirable remedy for this code would be to delete it and use Necko's support for overriding UA strings which already works and is well tested.

In bug 896114, loading the UserAgentOverrides on desktop was disabled for performance reasons. We reimplemented the logic with some added filtering so we don't iterate over functions and call them if we know the domain is not of interest for us, which keeps the performance up for sites we don't want to touch. So, as far as my knowledge goes, UserAgentOverrides.jsm is only used on Fennec at the moment. Our initial plan was to eventually remove the code for fennec as well as soon as we have our GoFaster addon running there.

Although, to be fair, I have not checked recently if UserAgentOverrides.jsm is back in desktop, which indeed would render our code redundant. Will check! Also thanks for the hint, will work on the startup as well.

Comment 5

[Mike Taylor [:miketaylr]]

Attachment: Bug 1371335. Part 1 - Lazily load TabListener module.

Review commit: https://reviewboard.mozilla.org/r/153002/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153002/

Comment 6

[Mike Taylor [:miketaylr]]

Attachment: Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

Review commit: https://reviewboard.mozilla.org/r/153004/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153004/

Comment 7

[Mike Taylor [:miketaylr]]

Attachment: Bug 1371335. Part 3 - Add TabListener.jsm to browser_startup tests.

Review commit: https://reviewboard.mozilla.org/r/153006/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/153006/

Comment 8

[:Gijs (he/him)]

Comment on attachment 8881933 [details]
Bug 1371335. Part 1 - Lazily load TabListener module.

https://reviewboard.mozilla.org/r/153002/#review158206

::: browser/extensions/webcompat-reporter/content/WebCompatReporter.jsm
(Diff revision 1)
>          .getInterface(Ci.nsIDOMWindowUtils)
>          .removeSheet(wcStyleURI, this._sheetType);
>      }
>  
>      CustomizableUI.removeListener(this);
> -    Cu.unload(TABLISTENER_JSM);

You probably still want to do this. You can use Cu.isModuleLoaded() to determine if anyone loaded the TabListener module.

Comment 9

[:Gijs (he/him)]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

https://reviewboard.mozilla.org/r/153004/#review158220

::: browser/extensions/webcompat-reporter/bootstrap.js:44
(Diff revision 1)
> -  if (enabled) {
> +    if (enabled) {
> -    WebCompatReporter.init();
> +      WebCompatReporter.init();
> -  }
> +    }
> +  };
> +
> +  mm.addMessageListener("Browser:FirstPaint", initOnFirstPaint);

I'm not sure this is the right thing... I suspect we want to use browser-delayed-startup-finished or something, though I'm not sure how that would work if you updated the add-on while Firefox was running.

Mike C., do you have suggestions here?

Comment 10

[:Gijs (he/him)]

Comment on attachment 8881935 [details]
Bug 1371335. Part 3 - Add TabListener.jsm to browser_startup tests.

https://reviewboard.mozilla.org/r/153006/#review158224

rs=me

Comment 11

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

https://reviewboard.mozilla.org/r/153004/#review158220

> I'm not sure this is the right thing... I suspect we want to use browser-delayed-startup-finished or something, though I'm not sure how that would work if you updated the add-on while Firefox was running.
> 
> Mike C., do you have suggestions here?

I agree that waiting until the first window has painted is the right choice, and the browser-delayed-startup-finished notification is probably the right thing to listen for.

If the situation that Gijs mentions arises, you can always check that the most recent window has painted first. We do something similar here:

http://searchfox.org/mozilla-central/rev/fdb811340ac4e6b93703d72ee99217f3b1d250ac/browser/base/content/browser.js#1168-1182

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

https://reviewboard.mozilla.org/r/153004/#review158256

Thanks for the patch! r-'ing though, because I agree with Gijs - we should wait for the browser-delayed-startup-finished observer notification.

Comment 13

[Mike Taylor [:miketaylr]]

Sweet, thanks for the pointers Mike and Gijs.

Comment 14

[Mike Taylor [:miketaylr]]

Comment on attachment 8881933 [details]
Bug 1371335. Part 1 - Lazily load TabListener module.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153002/diff/1-2/

Comment 15

[Mike Taylor [:miketaylr]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153004/diff/1-2/

Comment 16

[Mike Taylor [:miketaylr]]

Comment on attachment 8881935 [details]
Bug 1371335. Part 3 - Add TabListener.jsm to browser_startup tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153006/diff/1-2/

Comment 17

[Mike Taylor [:miketaylr]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

https://reviewboard.mozilla.org/r/153004/#review160386

::: browser/extensions/webcompat-reporter/bootstrap.js:42
(Diff revision 2)
> +
>  function startup(aData, aReason) {
>    // Observe pref changes and enable/disable as necessary.
>    Services.prefs.addObserver(PREF_WC_REPORTER_ENABLED, prefObserver);
>  
> -  // Only initialize if pref is enabled.
> +  // Only initialize if pref is enabled, after first paint.

Oops. Will update comment.

Comment 18

[Mike Taylor [:miketaylr]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153004/diff/2-3/

Comment 19

[Mike Taylor [:miketaylr]]

Comment on attachment 8881935 [details]
Bug 1371335. Part 3 - Add TabListener.jsm to browser_startup tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153006/diff/2-3/

Comment 20

[Florian Quèze [:florian]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

https://reviewboard.mozilla.org/r/153004/#review160388

::: browser/extensions/webcompat-reporter/bootstrap.js:32
(Diff revision 3)
>  };
>  
> +let notificationObserver = {
> +  observe(aSubject, aTopic, aData) {
> +    if (aTopic === DELAYED_STARTUP_FINISHED) {
> +      WebCompatReporter.init();

How quickly after startup do we need this to happen? Would it be ok to schedule this using Services.tm.idleDispatchToMainThread?

::: browser/extensions/webcompat-reporter/bootstrap.js:45
(Diff revision 3)
>    Services.prefs.addObserver(PREF_WC_REPORTER_ENABLED, prefObserver);
>  
> -  // Only initialize if pref is enabled.
> +  // Only initialize if pref is enabled, after the delayed startup notification.
>    let enabled = Services.prefs.getBoolPref(PREF_WC_REPORTER_ENABLED);
>    if (enabled) {
> +    if (window.gBrowserInit.delayedStartupFinished) {

I think 'window' is undefined here. Have you tested this code?

Comment 21

[Mike Taylor [:miketaylr]]

(In reply to Florian Quèze [:florian] [:flo] from comment #20)
> Comment on attachment 8881934 [details]
> Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after
> browser-delayed-startup-finished.
> 
> https://reviewboard.mozilla.org/r/153004/#review160388
> 
> ::: browser/extensions/webcompat-reporter/bootstrap.js:32
> (Diff revision 3)
> >  };
> >  
> > +let notificationObserver = {
> > +  observe(aSubject, aTopic, aData) {
> > +    if (aTopic === DELAYED_STARTUP_FINISHED) {
> > +      WebCompatReporter.init();
> 
> How quickly after startup do we need this to happen? Would it be ok to
> schedule this using Services.tm.idleDispatchToMainThread?

Not that quickly, I think. Most people are interacting with this addon only when they've encountered a busted page... 

Will look into idleDispatchToMainThread.
 
> ::: browser/extensions/webcompat-reporter/bootstrap.js:45
> (Diff revision 3)
> >    Services.prefs.addObserver(PREF_WC_REPORTER_ENABLED, prefObserver);
> >  
> > -  // Only initialize if pref is enabled.
> > +  // Only initialize if pref is enabled, after the delayed startup notification.
> >    let enabled = Services.prefs.getBoolPref(PREF_WC_REPORTER_ENABLED);
> >    if (enabled) {
> > +    if (window.gBrowserInit.delayedStartupFinished) {
> 
> I think 'window' is undefined here. Have you tested this code?

Uhh, you're right. >_>

Comment 22

[Mike Taylor [:miketaylr]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153004/diff/3-4/

Comment 23

[Mike Taylor [:miketaylr]]

Comment on attachment 8881935 [details]
Bug 1371335. Part 3 - Add TabListener.jsm to browser_startup tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153006/diff/3-4/

Comment 24

[Florian Quèze [:florian]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

https://reviewboard.mozilla.org/r/153004/#review160784

Almost there :-)

::: browser/extensions/webcompat-reporter/bootstrap.js:38
(Diff revision 4)
>      WebCompatReporter.uninit();
>    }
>  };
>  
> +let notificationObserver = {
> +  observe(aSubject, aTopic, aData) {

Please make this a simple function, and remove the switch/case stuff, we will only need it if we start observing multiple different notification topics.

::: browser/extensions/webcompat-reporter/bootstrap.js:55
(Diff revision 4)
>    Services.prefs.addObserver(PREF_WC_REPORTER_ENABLED, prefObserver);
>  
> -  // Only initialize if pref is enabled.
> +  // Only initialize if pref is enabled, after the delayed startup notification.
>    let enabled = Services.prefs.getBoolPref(PREF_WC_REPORTER_ENABLED);
>    if (enabled) {
> -    WebCompatReporter.init();
> +    let win = RecentWindow.getMostRecentBrowserWindow();

I would just use Services.wm.getMostRecentWindow("navigator:browser") here. RecentWindow.jsm isn't loaded yet at this point of startup, and it's probably not needed at all before first paint.

::: browser/extensions/webcompat-reporter/bootstrap.js:70
(Diff revision 4)
>  function shutdown(aData, aReason) {
>    if (aReason === APP_SHUTDOWN) {
>      return;
>    }
>  
>    Cu.unload(WEBCOMPATREPORTER_JSM);

not related to what this patch is trying to fix, but I think there's a missing Services.prefs.removeObserver(PREF_WC_REPORTER_ENABLED, prefObserver); here, which may cause the old version of this add-on to be leaked when updating.

Comment 25

[Mike Taylor [:miketaylr]]

Attachment: Bug 1371335. Part 4 - Remove pref observer during bootstrap shutdown().

Review commit: https://reviewboard.mozilla.org/r/155784/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/155784/

Comment 26

[Mike Taylor [:miketaylr]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153004/diff/4-5/

Comment 27

[Mike Taylor [:miketaylr]]

Comment on attachment 8881935 [details]
Bug 1371335. Part 3 - Add TabListener.jsm to browser_startup tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153006/diff/4-5/

Comment 28

[Mike Taylor [:miketaylr]]

Thanks for the review feedback!

Comment 29

[Florian Quèze [:florian]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

https://reviewboard.mozilla.org/r/153004/#review160884

::: browser/extensions/webcompat-reporter/bootstrap.js:20
(Diff revision 5)
>  XPCOMUtils.defineLazyModuleGetter(this, "WebCompatReporter",
>    WEBCOMPATREPORTER_JSM);
>  
>  const PREF_WC_REPORTER_ENABLED = "extensions.webcompat-reporter.enabled";
>  
> +let requestReporterInit = () => {

This should be a simple function:
function requestReporterInit() {

(don't forget to remove the no longer needed ';')

::: browser/extensions/webcompat-reporter/bootstrap.js:26
(Diff revision 5)
> +  Services.tm.idleDispatchToMainThread(() => {
> +    WebCompatReporter.init();
> +  });
> +};
> +
>  let prefObserver = function(aSubject, aTopic, aData) {

nit: This could also become a simple function, but I'm not requiring it as it's not new code.

::: browser/extensions/webcompat-reporter/bootstrap.js:35
(Diff revision 5)
>    } else {
>      WebCompatReporter.uninit();
>    }
>  };
>  
> +let onDelayedStartupFinished = (subject, topic, data) => {

You don't need to access 'this' from within this function, so it should be a simple function:

function onDelayedStartupFinished(subject, topic, data) {

::: browser/extensions/webcompat-reporter/bootstrap.js:37
(Diff revision 5)
>    }
>  };
>  
> +let onDelayedStartupFinished = (subject, topic, data) => {
> +  requestReporterInit();
> +  Services.obs.removeObserver(this, DELAYED_STARTUP_FINISHED);

this doesn't point to the observer here, but likely to the global scope.

You want to removeObserver(onDelayedStartupFinished, ...
ie. call removeObserver with the same first parameter as you gave to addObserver.

::: browser/extensions/webcompat-reporter/bootstrap.js:38
(Diff revision 5)
>  };
>  
> +let onDelayedStartupFinished = (subject, topic, data) => {
> +  requestReporterInit();
> +  Services.obs.removeObserver(this, DELAYED_STARTUP_FINISHED);
> +};

when converting to a normal function, don't forget to remove the ';' here.

Comment 30

[Florian Quèze [:florian]]

Comment on attachment 8884958 [details]
Bug 1371335. Part 4 - Remove pref observer during bootstrap shutdown().

https://reviewboard.mozilla.org/r/155784/#review160888

Comment 31

[Mike Taylor [:miketaylr]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

https://reviewboard.mozilla.org/r/153004/#review160926

::: browser/extensions/webcompat-reporter/bootstrap.js:20
(Diff revision 5)
>  XPCOMUtils.defineLazyModuleGetter(this, "WebCompatReporter",
>    WEBCOMPATREPORTER_JSM);
>  
>  const PREF_WC_REPORTER_ENABLED = "extensions.webcompat-reporter.enabled";
>  
> +let requestReporterInit = () => {

Roger that.

::: browser/extensions/webcompat-reporter/bootstrap.js:37
(Diff revision 5)
>    }
>  };
>  
> +let onDelayedStartupFinished = (subject, topic, data) => {
> +  requestReporterInit();
> +  Services.obs.removeObserver(this, DELAYED_STARTUP_FINISHED);

Ah yeah. I'll just change this to a named function so this points to the right thing.

Comment 32

[Mike Taylor [:miketaylr]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153004/diff/5-6/

Comment 33

[Mike Taylor [:miketaylr]]

Comment on attachment 8881935 [details]
Bug 1371335. Part 3 - Add TabListener.jsm to browser_startup tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153006/diff/5-6/

Comment 34

[Mike Taylor [:miketaylr]]

Comment on attachment 8884958 [details]
Bug 1371335. Part 4 - Remove pref observer during bootstrap shutdown().

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/155784/diff/1-2/

Comment 35

[Florian Quèze [:florian]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

https://reviewboard.mozilla.org/r/153004/#review160934

::: browser/extensions/webcompat-reporter/bootstrap.js:37
(Diff revision 6)
>    }
> -};
> +}
> +
> +function onDelayedStartupFinished(subject, topic, data) {
> +  requestReporterInit();
> +  Services.obs.removeObserver(this, DELAYED_STARTUP_FINISHED);

'this' still points to the global scope here. As I said before, you should removeObserver(onDelayedStartupFinished, ...

Comment 36

[Mike Taylor [:miketaylr]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153004/diff/6-7/

Comment 37

[Mike Taylor [:miketaylr]]

Comment on attachment 8881935 [details]
Bug 1371335. Part 3 - Add TabListener.jsm to browser_startup tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153006/diff/6-7/

Comment 38

[Mike Taylor [:miketaylr]]

Comment on attachment 8884958 [details]
Bug 1371335. Part 4 - Remove pref observer during bootstrap shutdown().

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/155784/diff/2-3/

Comment 39

[Florian Quèze [:florian]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

https://reviewboard.mozilla.org/r/153004/#review160948

Comment 40

[Mike Taylor [:miketaylr]]

Comment on attachment 8881933 [details]
Bug 1371335. Part 1 - Lazily load TabListener module.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153002/diff/2-3/

Comment 41

[Mike Taylor [:miketaylr]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153004/diff/7-8/

Comment 42

[Mike Taylor [:miketaylr]]

Comment on attachment 8881935 [details]
Bug 1371335. Part 3 - Add TabListener.jsm to browser_startup tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153006/diff/7-8/

Comment 43

[Mike Taylor [:miketaylr]]

Comment on attachment 8884958 [details]
Bug 1371335. Part 4 - Remove pref observer during bootstrap shutdown().

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/155784/diff/3-4/

Comment 44

[Mike Taylor [:miketaylr]]

Had some timeouts in browser_report_site_issue.js that was due to not re-importing the TabListener module in init, after it had been explicitly unloaded in uninit (which will happen if the pref is flipped to false, which browser_disabled_cleanup.js does just before this test).

So, just added the following in init, and pushed a new try run:

if (!Cu.isModuleLoaded(TABLISTENER_JSM)) {
  Cu.import(TABLISTENER_JSM);
}

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

Comment 45

[Mike Taylor [:miketaylr]]

Comment on attachment 8881933 [details]
Bug 1371335. Part 1 - Lazily load TabListener module.

Try is green for relevant tests. 

Gijs can you sanity check the changes described in Comment #44 before I land this, please?

Comment 46

[:Gijs (he/him)]

Comment on attachment 8881933 [details]
Bug 1371335. Part 1 - Lazily load TabListener module.

Discussed on IRC. This will result in importing the jsm on startup again, though patch 2 gets rid of that. We could either not take this part, or try to define the lazy getter in init() instead of in the global scope, so if init reruns it gets defined a second time.

Comment 47

[Mike Taylor [:miketaylr]]

Comment on attachment 8881933 [details]
Bug 1371335. Part 1 - Lazily load TabListener module.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153002/diff/3-4/

Comment 48

[Mike Taylor [:miketaylr]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153004/diff/8-9/

Comment 49

[Mike Taylor [:miketaylr]]

Comment on attachment 8881935 [details]
Bug 1371335. Part 3 - Add TabListener.jsm to browser_startup tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153006/diff/8-9/

Comment 50

[Mike Taylor [:miketaylr]]

Comment on attachment 8884958 [details]
Bug 1371335. Part 4 - Remove pref observer during bootstrap shutdown().

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/155784/diff/4-5/

Comment 51

[Mike Taylor [:miketaylr]]

Comment on attachment 8881933 [details]
Bug 1371335. Part 1 - Lazily load TabListener module.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153002/diff/4-5/

Comment 52

[Mike Taylor [:miketaylr]]

Comment on attachment 8881934 [details]
Bug 1371335. Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153004/diff/9-10/

Comment 53

[Mike Taylor [:miketaylr]]

Comment on attachment 8881935 [details]
Bug 1371335. Part 3 - Add TabListener.jsm to browser_startup tests.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/153006/diff/9-10/

Comment 54

[Mike Taylor [:miketaylr]]

Comment on attachment 8884958 [details]
Bug 1371335. Part 4 - Remove pref observer during bootstrap shutdown().

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/155784/diff/5-6/

Comment 55

[Mike Taylor [:miketaylr]]

Comment on attachment 8881933 [details]
Bug 1371335. Part 1 - Lazily load TabListener module.

(sorry, i don't know how to clear a review on reviewboard, or re-request review on a previously r+'d patch...)

OK. I've defined the lazyModuleGetter inside init so the TabListener sits on the WebCompatReporter object, rather than the module global.

Browser Chrome Test Summary
	Passed: 11
	Failed: 0
	Todo: 0
	Mode: e10s

Comment 56

[:Gijs (he/him)]

Comment on attachment 8881933 [details]
Bug 1371335. Part 1 - Lazily load TabListener module.

Nice, thanks!

Comment 57

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0bb8e7c81cb6
Part 1 - Lazily load TabListener module. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/d38dd453257f
Part 2 - Don't init WebCompatReporter add-on until after browser-delayed-startup-finished. r=florian
https://hg.mozilla.org/integration/autoland/rev/7059c615165a
Part 3 - Add TabListener.jsm to browser_startup tests. r=Gijs
https://hg.mozilla.org/integration/autoland/rev/4f321d1ee0fd
Part 4 - Remove pref observer during bootstrap shutdown(). r=florian

Comment 58

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0bb8e7c81cb6
https://hg.mozilla.org/mozilla-central/rev/d38dd453257f
https://hg.mozilla.org/mozilla-central/rev/7059c615165a
https://hg.mozilla.org/mozilla-central/rev/4f321d1ee0fd