1376895 - Make preloaded browser use pre-existing content process

Status: RESOLVED FIXED

(Firefox :: Tabbed Browser, defect, P1)

Description

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

In order to avoid consuming the preallocated content process, the preloaded about:newtab page (if about:newtab is running remotely), should use a pre-existing content process.

The plan is to then decorate the preloaded about:newtab with a special attribute so that nsFrameLoader/ContentParent can choose a preallocated content process (if one is available) during its next page load.

This relies on bug 1376894 for that behaviour.

Comment 1

[Eric Rahm [:erahm]]

Mike, what's the priority on this? It blocks a huge awsy regression in bug 1381804. It sounds like either this bug needs to be fixed soonish or we need to back out bug 1365643 which might be a bit involved.

Given the size of the regression we need to get a fix in soon so that we don't let any smaller regressions slip by unnoticed.

Comment 2

[Ed Lee :Mardak]

I pushed the bug 1365643 backout to try now that wcpan's fix had landed, and it seems to be clean. The backouts are awaiting review in bug 1382079.

Comment 3

[Gabor Krizsanits (INACTIVE)]

(In reply to Mike Conley (:mconley) - Digging out of needinfo / review backlog. from comment #0)
> The plan is to then decorate the preloaded about:newtab with a special
> attribute so that nsFrameLoader/ContentParent can choose a preallocated
> content process (if one is available) during its next page load.

Mike, it seems to be hard to see when did the next page load occur from the nsFrameLoader/ContentParent. Do you think it's easier to tell that from frontend code and tell the frame loader to throw away its remote browser and create a new one? From the awesome bar it should be simple to do it from frontend, maybe the other cases can be handled from frontend too?

Comment 4

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

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #3)
> Mike, it seems to be hard to see when did the next page load occur from the
> nsFrameLoader/ContentParent.

Hm. I wonder if that's worth adding. Maybe smaug or billm has an opinion on that, dunno.

> Do you think it's easier to tell that from
> frontend code and tell the frame loader to throw away its remote browser and
> create a new one? From the awesome bar it should be simple to do it from
> frontend, maybe the other cases can be handled from frontend too?

It looks like we don't keep a history entry for about:newtab when it's the first item in a tab's session lifetime, so my original instinct of putting any kind of swapping logic in with the remoteness flipping stuff is probably off-base and overpowered.

We might want to hook into the same entry point though, because we have to account not only for loads initting in the parent (from, for example, the AwesomeBar), but also from the newtab itself (from, for example, clicking on a tile).

Mossop originally added the hooks in bug 999239. I wonder if we could have the preloaded browser be special cased, and when we hit that hook, somehow handle it specially so that we don't flip to non-remote, but just remove and re-add to the DOM to trigger the remote frameloader creation and tapping of the preallocated process.

Does that make sense?

Comment 5

[Gabor Krizsanits (INACTIVE)]

Attachment: Make preloaded browser use pre-existing content process. v1

On the content side there is no easy way to tell if it's a preloaded browser so that part is a bit hacky.

So what this patch does:
- sets an attribute on the preloaded browser
- detects that during the process selecting algorithm and pretends that we reached the max process count (if we have 1 at least)
- during the first navigation:
    - from parent side: we can detect the attribute
    - from child side: we redirect it to the parent if we're on about:newtab and the history is trivial
- once we detected what's going on, we request a new frame loader on the parent side that will rerun the process selecting algorithm and also clear the attribute

I should run this on try and somehow write a test for it... but I would like to get an early feedback, so I'm setting feedback? for now.

Comment 6

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

Comment on attachment 8890302 [details] [diff] [review]
Make preloaded browser use pre-existing content process. v1

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

This looks like the right idea for me! We might be able to pass down the preload state to the TabChild though - see below.

::: browser/modules/E10SUtils.jsm
@@ +243,5 @@
>          this.getRemoteTypeForURIObject(aURI, true, remoteType, webNav.currentURI);
>      }
>  
> +    if (webNav.currentURI.spec == "about:newtab" && sessionHistory.count == 1) {
> +      // This is possibly a preloaded browser and we're about to navigate away for

I feel like this is something that could be communicated to the child when the frameloader / TabParent is constructed. Perhaps when constructing the TabParent, in the TabContext that we send down?: http://searchfox.org/mozilla-central/rev/09c065976fd4f18d4ad764d7cb4bbc684bf56714/dom/ipc/TabContext.h#30

Comment 7

[Gabor Krizsanits (INACTIVE)]

Attachment: Make preloaded browser use pre-existing content process. v2

(In reply to Mike Conley (:mconley) - Buried in needinfo / review backlog, eating my way out from comment #6)

> > +    if (webNav.currentURI.spec == "about:newtab" && sessionHistory.count == 1) {
> > +      // This is possibly a preloaded browser and we're about to navigate away for
> 
> I feel like this is something that could be communicated to the child when
> the frameloader / TabParent is constructed. Perhaps when constructing the
> TabParent, in the TabContext that we send down?:
> http://searchfox.org/mozilla-central/rev/
> 09c065976fd4f18d4ad764d7cb4bbc684bf56714/dom/ipc/TabContext.h#30

So TabContext is immutable, no? I'm not sure how that will work out tbh after the navigation... Anyway, in the end we will make the final decision on the parent side based on the attribute on the browser element either way so it would not change much... maybe this one check would look simpler a bit but I don't see any side effect of this version that worries me. So I'd prefer to land it as it is, and then maybe clean this part up a bit in followup when there is less time pressure on us. How does that sound?

Comment 8

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

Comment on attachment 8892855 [details] [diff] [review]
Make preloaded browser use pre-existing content process. v2

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

Okay, looks good - just some cosmetic stuff. Probably best to make sure ESLint likes this to avoid automation yelling at you post-landing.

Thanks!

::: dom/base/test/browser_aboutnewtab_process_selection.js
@@ +1,3 @@
> +const TEST_URL = "http://www.example.com/browser/dom/tests/browser/dummy.html";
> +
> +var ppmm = Cc["@mozilla.org/parentprocessmessagemanager;1"]

I think you can just use Services.ppmm:

http://searchfox.org/mozilla-central/rev/f0e4ae5f8c40ba742214e89aba3f554da0b89a33/toolkit/modules/Services.jsm#57-61

@@ +10,5 @@
> +    ["dom.ipc.processPrelaunch.enabled", false],
> +    ["dom.ipc.processCount", 10],
> +    ["dom.ipc.keepProcessesAlive.web", 10],
> +  ]});
> +})

Missing semi-colon.

@@ +14,5 @@
> +})
> +
> +async function ensurePreloaded(gBrowser) {
> +  gBrowser._createPreloadBrowser();
> +  await BrowserTestUtils.waitForCondition( () => {

Instead of using the contentDocument CPOW, I think you should be able to use:

await BrowserTestUtils.browserLoaded(gBrowser._preloadedBrowser);

@@ +30,5 @@
> +  // Wait for the preloaded browser to load.
> +  await ensurePreloaded(gBrowser);
> +
> +  // Store the number of processes (note: +1 for the parent process).
> +  const {childCount} = ppmm;

Can we call this originalChildCount for clarity? You might already know this, but you can keep the destructuring too:

const { childCount: originalChildCount } = ppmm;

@@ +32,5 @@
> +
> +  // Store the number of processes (note: +1 for the parent process).
> +  const {childCount} = ppmm;
> +
> +  // Use the prelaoded browser and create another one.

Nit: prelaoded -> preloaded

@@ +50,5 @@
> +  is(ppmm.childCount, childCount, "Preloaded browser should (still) not create a new content process.")
> +
> +  // Navigate to a content page from the parent side.
> +  tab2.linkedBrowser.loadURI(TEST_URL);
> +  await BrowserTestUtils.browserStopped(gBrowser);

BrowserTestUtils.browserStopped takes a <xul:browser>, not a <xul:tabbrowser>. I think it's kinda accidental that this works.

For clarity, could you please instead use:

await BrowserTestUtils.browserLoaded(tab2.linkedBrowser);

@@ +60,5 @@
> +  await ContentTask.spawn(tab1.linkedBrowser, null, async function() {
> +    const TEST_URL = "http://www.example.com/browser/dom/tests/browser/dummy.html";
> +    content.location.href = TEST_URL;
> +  });
> +  await BrowserTestUtils.browserStopped(gBrowser);

As above, probably best to use:

await BrowserTestUtils.browserLoaded(tab1.linkedBrowser);

@@ +71,5 @@
> +  // Since we kept alive all the processes, we can shut down the ones that do
> +  // not host any tabs reliably.
> +  ppmm.releaseCachedProcesses();
> +  is(ppmm.childCount, childCount, "We're back to the original process count.")
> +})

Missing semi-colon.

Comment 9

[Gabor Krizsanits (INACTIVE)]

Attachment: Make preloaded browser use pre-existing content process. v3

(In reply to Mike Conley (:mconley) - Holiday until August 8th from comment #8)
> Okay, looks good - just some cosmetic stuff. Probably best to make sure
> ESLint likes this to avoid automation yelling at you post-landing.

I probably should have put the test somewhere where eslint is set up :(

> @@ +14,5 @@
> > +})
> > +
> > +async function ensurePreloaded(gBrowser) {
> > +  gBrowser._createPreloadBrowser();
> > +  await BrowserTestUtils.waitForCondition( () => {
> 
> Instead of using the contentDocument CPOW, I think you should be able to use:
> 
> await BrowserTestUtils.browserLoaded(gBrowser._preloadedBrowser);

Not really, the BTU helper function tries to insert the browser which break things here. And I think the CPOW is under the contentDocumentAsCPOW property no? I tried to ignore the load and just made sure the browser is created but that did not seem to be enough so this is the best I could come up with.

> @@ +50,5 @@
> > +  is(ppmm.childCount, childCount, "Preloaded browser should (still) not create a new content process.")
> > +
> > +  // Navigate to a content page from the parent side.
> > +  tab2.linkedBrowser.loadURI(TEST_URL);
> > +  await BrowserTestUtils.browserStopped(gBrowser);
> 
> BrowserTestUtils.browserStopped takes a <xul:browser>, not a
> <xul:tabbrowser>. I think it's kinda accidental that this works.
> 
> For clarity, could you please instead use:
> 
> await BrowserTestUtils.browserLoaded(tab2.linkedBrowser);

Some other tests are also relying on this, I wonder if I should file a bug. I thought browserLoaded is not reliable when the navigation switches processes but it seems to work perfectly, thanks for the hint!

So I realized that the parent side part is actually not working (or maybe something has changed since then because I could swear it used to work) and it's always the child side code now that starts the new process and makes both tests pass. I had to handle the parent side case in _loadURIWithFlags. Since that's a functional change, and I'm not sure it's acceptable, could you take a look at it again?

changes:
- _loadURIWithFlags part in browser.js
- I've added a dummy file since the one I was referring to previously did not work (I haven't noticed the 404's since from the process count perspective it didn't really matter)
- I address your comments except the one I mentioned above

Comment 10

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

Comment on attachment 8894543 [details] [diff] [review]
Make preloaded browser use pre-existing content process. v3

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

Let's do it! Thanks gabor.

::: dom/base/test/browser_aboutnewtab_process_selection.js
@@ +12,5 @@
> +});
> +
> +async function ensurePreloaded(gBrowser) {
> +  gBrowser._createPreloadBrowser();
> +  await BrowserTestUtils.waitForCondition( () => {

Can you call out explicitly why we're doing it this way in a comment?

@@ +16,5 @@
> +  await BrowserTestUtils.waitForCondition( () => {
> +    return gBrowser._preloadedBrowser.contentDocument.readyState == "complete";
> +  });
> +}
> +

Can you briefly document what is being tested here in a block comment above the add_task?

@@ +69,5 @@
> +  // Since we kept alive all the processes, we can shut down the ones that do
> +  // not host any tabs reliably.
> +  ppmm.releaseCachedProcesses();
> +  is(ppmm.childCount, originalChildCount, "We're back to the original process count.");
> +})

Nit: needs semi-colon.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/93151fa47fcf
Make preloaded browser use pre-existing content process. r=mconley

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for turning test_frameNavigation.html permafail like https://treeherder.mozilla.org/logviewer.html#?job_id=122365018&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/93cf457d27d97667ff50dcf6aaeb7f0560708fae

Comment 13

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/2ef9f5e1ae1c
Backed out changeset 93151fa47fcf for permafailing test_frameNavigation.html a=backout

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0aca62e914cf
Make preloaded browser use pre-existing content process. r=mconley

Comment 15

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

Backed out

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=0aca62e914cf96bae93d8fbafd4c9ce43c153116&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=runnable
Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=122546325&repo=mozilla-inbound
11:34:40     INFO -  158 INFO None159 INFO TEST-START | dom/events/test/test_bug336682_1.html
11:34:40     INFO -  GECKO(7988) | ++DOMWINDOW == 62 (0000016AE0B8A000) [pid = 5844] [serial = 70] [outer = 0000016ADC486800]
11:34:40     INFO -  GECKO(7988) | Assertion failure: IsSafeToRun(), at z:\build\build\src\obj-firefox\dist\include\mozilla/SchedulerGroup.h:81
11:34:54     INFO -  GECKO(7988) | #01: nsDocument::GetEventTargetParent(mozilla::EventChainPreVisitor &) [dom/base/nsDocument.cpp:8035]
11:34:54     INFO -  GECKO(7988) | #02: mozilla::EventTargetChainItem::GetEventTargetParent(mozilla::EventChainPreVisitor &) [dom/events/EventDispatcher.cpp:388]
11:34:54     INFO -  GECKO(7988) | #03: mozilla::EventDispatcher::Dispatch(nsISupports *,nsPresContext *,mozilla::WidgetEvent *,nsIDOMEvent *,nsEventStatus *,mozilla::EventDispatchingCallback *,nsTArray<mozilla::dom::EventTarget *> *) [dom/events/EventDispatcher.cpp:788]
11:34:54     INFO -  GECKO(7988) | #04: mozilla::EventDispatcher::DispatchDOMEvent(nsISupports *,mozilla::WidgetEvent *,nsIDOMEvent *,nsPresContext *,nsEventStatus *) [dom/events/EventDispatcher.cpp:888]
11:34:54     INFO -  GECKO(7988) | #05: nsINode::DispatchEvent(nsIDOMEvent *,bool *) [dom/base/nsINode.cpp:1347]
11:34:54     INFO -  GECKO(7988) | #06: nsContentUtils::DispatchEvent(nsIDocument *,nsISupports *,nsAString const &,bool,bool,bool,bool *,bool) [dom/base/nsContentUtils.cpp:4489]
11:34:54     INFO -  GECKO(7988) | #07: nsContentUtils::DispatchTrustedEvent(nsIDocument *,nsISupports *,nsAString const &,bool,bool,bool *) [dom/base/nsContentUtils.cpp:4459]
11:34:54     INFO -  GECKO(7988) | #08: nsGlobalWindow::FireOfflineStatusEventIfChanged() [dom/base/nsGlobalWindow.cpp:11490]
11:34:54     INFO -  GECKO(7988) | #09: nsGlobalWindow::Observe(nsISupports *,char const *,char16_t const *) [dom/base/nsGlobalWindow.cpp:12106]
11:34:54     INFO -  GECKO(7988) | #10: nsObserverList::NotifyObservers(nsISupports *,char const *,char16_t const *) [xpcom/ds/nsObserverList.cpp:112]
11:34:54     INFO -  GECKO(7988) | #11: nsObserverService::NotifyObservers(nsISupports *,char const *,char16_t const *) [xpcom/ds/nsObserverService.cpp:299]
11:34:54     INFO -  GECKO(7988) | #12: mozilla::net::nsIOService::SetOffline(bool) [netwerk/base/nsIOService.cpp:1140]
11:34:54     INFO -  GECKO(7988) | #13: XPTC__InvokebyIndex
11:34:54     INFO -  GECKO(7988) | #14: CallMethodHelper::Call() [js/xpconnect/src/XPCWrappedNative.cpp:1317]
11:34:54     INFO -  GECKO(7988) | #15: XPCWrappedNative::CallMethod(XPCCallContext &,XPCWrappedNative::CallMode) [js/xpconnect/src/XPCWrappedNative.cpp:1282]
11:34:54     INFO -  GECKO(7988) | #16: XPC_WN_GetterSetter(JSContext *,unsigned int,JS::Value *) [js/xpconnect/src/XPCWrappedNativeJSOps.cpp:995]
11:34:54     INFO -  GECKO(7988) | #17: js::CallJSNative(JSContext *,bool (*)(JSContext *,unsigned int,JS::Value *),JS::CallArgs const &) [js/src/jscntxtinlines.h:293]
11:34:54     INFO -  GECKO(7988) | #18: js::InternalCallOrConstruct(JSContext *,JS::CallArgs const &,js::MaybeConstruct) [js/src/vm/Interpreter.cpp:469]
11:34:54     INFO -  GECKO(7988) | #19: js::CallSetter(JSContext *,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::Handle<JS::Value>) [js/src/vm/Interpreter.cpp:662]
11:34:54     INFO -  GECKO(7988) | #20: SetExistingProperty [js/src/vm/NativeObject.cpp:2765]
11:34:54     INFO -  GECKO(7988) | #21: js::NativeSetProperty(JSContext *,JS::Handle<js::NativeObject *>,JS::Handle<jsid>,JS::Handle<JS::Value>,JS::Handle<JS::Value>,js::QualifiedBool,JS::ObjectOpResult &) [js/src/vm/NativeObject.cpp:2799]
11:34:54     INFO -  GECKO(7988) | #22: Reflect_set [js/src/builtin/Reflect.cpp:214]
11:34:54     INFO -  GECKO(7988) | #23: js::CallJSNative(JSContext *,bool (*)(JSContext *,unsigned int,JS::Value *),JS::CallArgs const &) [js/src/jscntxtinlines.h:293]
11:34:54     INFO -  GECKO(7988) | #24: js::InternalCallOrConstruct(JSContext *,JS::CallArgs const &,js::MaybeConstruct) [js/src/vm/Interpreter.cpp:469]
11:34:54     INFO -  GECKO(7988) | #25: Interpret [js/src/vm/Interpreter.cpp:3065]
11:34:54     INFO -  GECKO(7988) | #26: js::RunScript(JSContext *,js::RunState &) [js/src/vm/Interpreter.cpp:409]
11:34:54     INFO -  GECKO(7988) | #27: js::InternalCallOrConstruct(JSContext *,JS::CallArgs const &,js::MaybeConstruct) [js/src/vm/Interpreter.cpp:487]
11:34:54     INFO -  GECKO(7988) | #28: js::Call(JSContext *,JS::Handle<JS::Value>,JS::Handle<JS::Value>,js::AnyInvokeArgs const &,JS::MutableHandle<JS::Value>) [js/src/vm/Interpreter.cpp:533]
11:34:54     INFO -  GECKO(7988) | #29: js::ScriptedProxyHandler::set(JSContext *,JS::Handle<JSObject *>,JS::Handle<jsid>,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::ObjectOpResult &) [js/src/proxy/ScriptedProxyHandler.cpp:1080]
11:34:54     INFO -  GECKO(7988) | #30: js::Proxy::set(JSContext *,JS::Handle<JSObject *>,JS::Handle<jsid>,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::ObjectOpResult &) [js/src/proxy/Proxy.cpp:384]
11:34:54     INFO -  GECKO(7988) | #31: JSObject::nonNativeSetProperty(JSContext *,JS::Handle<JSObject *>,JS::Handle<jsid>,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::ObjectOpResult &) [js/src/jsobj.cpp:1047]
11:34:54     INFO -  GECKO(7988) | #32: js::SetProperty(JSContext *,JS::Handle<JSObject *>,JS::Handle<jsid>,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::ObjectOpResult &) [js/src/vm/NativeObject.h:1554]
11:34:54     INFO -  GECKO(7988) | #33: js::Wrapper::set(JSContext *,JS::Handle<JSObject *>,JS::Handle<jsid>,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::ObjectOpResult &) [js/src/proxy/Wrapper.cpp:156]
11:34:54     INFO -  GECKO(7988) | #34: js::CrossCompartmentWrapper::set(JSContext *,JS::Handle<JSObject *>,JS::Handle<jsid>,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::ObjectOpResult &) [js/src/proxy/CrossCompartmentWrapper.cpp:238]
11:34:54     INFO -  GECKO(7988) | #35: js::Proxy::set(JSContext *,JS::Handle<JSObject *>,JS::Handle<jsid>,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::ObjectOpResult &) [js/src/proxy/Proxy.cpp:384]
11:34:54     INFO -  GECKO(7988) | #36: JSObject::nonNativeSetProperty(JSContext *,JS::Handle<JSObject *>,JS::Handle<jsid>,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::ObjectOpResult &) [js/src/jsobj.cpp:1047]
11:34:54     INFO -  GECKO(7988) | #37: SetPropertyOperation [js/src/vm/Interpreter.cpp:243]
11:34:54     INFO -  GECKO(7988) | #38: Interpret [js/src/vm/Interpreter.cpp:2862]
11:34:54     INFO -  GECKO(7988) | #39: js::RunScript(JSContext *,js::RunState &) [js/src/vm/Interpreter.cpp:409]
11:34:54     INFO -  GECKO(7988) | #40: js::InternalCallOrConstruct(JSContext *,JS::CallArgs const &,js::MaybeConstruct) [js/src/vm/Interpreter.cpp:487]
11:34:54     INFO -  GECKO(7988) | #41: js::Call(JSContext *,JS::Handle<JS::Value>,JS::Handle<JS::Value>,js::AnyInvokeArgs const &,JS::MutableHandle<JS::Value>) [js/src/vm/Interpreter.cpp:533]
11:34:54     INFO -  GECKO(7988) | #42: js::fun_apply(JSContext *,unsigned int,JS::Value *) [js/src/jsfun.cpp:1309]
11:34:54     INFO -  GECKO(7988) | #43: js::CallJSNative(JSContext *,bool (*)(JSContext *,unsigned int,JS::Value *),JS::CallArgs const &) [js/src/jscntxtinlines.h:293]
11:34:54     INFO -  GECKO(7988) | #44: js::InternalCallOrConstruct(JSContext *,JS::CallArgs const &,js::MaybeConstruct) [js/src/vm/Interpreter.cpp:469]
11:34:54     INFO -  GECKO(7988) | #45: Interpret [js/src/vm/Interpreter.cpp:3065]
11:34:54     INFO -  GECKO(7988) | #46: js::RunScript(JSContext *,js::RunState &) [js/src/vm/Interpreter.cpp:409]
11:34:54     INFO -  GECKO(7988) | #47: js::InternalCallOrConstruct(JSContext *,JS::CallArgs const &,js::MaybeConstruct) [js/src/vm/Interpreter.cpp:487]
11:34:54     INFO -  GECKO(7988) | #48: js::Call(JSContext *,JS::Handle<JS::Value>,JS::Handle<JS::Value>,js::AnyInvokeArgs const &,JS::MutableHandle<JS::Value>) [js/src/vm/Interpreter.cpp:533]
11:34:54     INFO -  GECKO(7988) | #49: JS::Call(JSContext *,JS::Handle<JS::Value>,JS::Handle<JS::Value>,JS::HandleValueArray const &,JS::MutableHandle<JS::Value>) [js/src/jsapi.cpp:2943]
11:34:54     INFO -  GECKO(7988) | #50: mozilla::dom::EventHandlerNonNull::Call(JSContext *,JS::Handle<JS::Value>,mozilla::dom::Event &,JS::MutableHandle<JS::Value>,mozilla::ErrorResult &) [obj-firefox/dom/bindings/EventHandlerBinding.cpp:260]
11:34:54     INFO -  GECKO(7988) | #51: mozilla::dom::EventHandlerNonNull::Call<nsISupports *>(nsISupports * const &,mozilla::dom::Event &,JS::MutableHandle<JS::Value>,mozilla::ErrorResult &,char const *,mozilla::dom::CallbackObject::ExceptionHandling,JSCompartment *) [obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h:362]
11:34:54     INFO -  GECKO(7988) | #52: mozilla::JSEventHandler::HandleEvent(nsIDOMEvent *) [dom/events/JSEventHandler.cpp:216]
11:34:54     INFO -  GECKO(7988) | #53: mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener *,nsIDOMEvent *,mozilla::dom::EventTarget *) [dom/events/EventListenerManager.cpp:1112]
11:34:54     INFO -  GECKO(7988) | #54: mozilla::EventListenerManager::HandleEventInternal(nsPresContext *,mozilla::WidgetEvent *,nsIDOMEvent * *,mozilla::dom::EventTarget *,nsEventStatus *) [dom/events/EventListenerManager.cpp:1283]
11:34:54     INFO -  GECKO(7988) | #55: mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor &,mozilla::ELMCreationDetector &) [dom/events/EventDispatcher.cpp:317]
11:34:54     INFO -  GECKO(7988) | #56: mozilla::EventTargetChainItem::HandleEventTargetChain(nsTArray<mozilla::EventTargetChainItem> &,mozilla::EventChainPostVisitor &,mozilla::EventDispatchingCallback *,mozilla::ELMCreationDetector &) [dom/events/EventDispatcher.cpp:464]
11:34:54     INFO -  GECKO(7988) | #57: mozilla::EventDispatcher::Dispatch(nsISupports *,nsPresContext *,mozilla::WidgetEvent *,nsIDOMEvent *,nsEventStatus *,mozilla::EventDispatchingCallback *,nsTArray<mozilla::dom::EventTarget *> *) [dom/events/EventDispatcher.cpp:827]
11:34:54     INFO -  GECKO(7988) | #58: nsDocumentViewer::LoadComplete(nsresult) [layout/base/nsDocumentViewer.cpp:1083]
11:34:54     INFO -  GECKO(7988) | #59: nsDocShell::EndPageLoad(nsIWebProgress *,nsIChannel *,nsresult) [docshell/base/nsDocShell.cpp:7719]
11:34:54     INFO -  GECKO(7988) | #60: nsDocShell::OnStateChange(nsIWebProgress *,nsIRequest *,unsigned int,nsresult) [docshell/base/nsDocShell.cpp:7517]
11:34:54     INFO -  GECKO(7988) | #61: nsDocLoader::DoFireOnStateChange(nsIWebProgress * const,nsIRequest * const,int &,nsresult) [uriloader/base/nsDocLoader.cpp:1320]
11:34:54     INFO -  GECKO(7988) | #62: nsDocLoader::doStopDocumentLoad(nsIRequest *,nsresult) [uriloader/base/nsDocLoader.cpp:861]
11:34:54     INFO -  GECKO(7988) | #63: nsDocLoader::DocLoaderIsEmpty(bool) [uriloader/base/nsDocLoader.cpp:752]
11:34:54     INFO -  GECKO(7988) | #64: nsDocLoader::OnStopRequest(nsIRequest *,nsISupports *,nsresult) [uriloader/base/nsDocLoader.cpp:633]
11:34:54     INFO -  GECKO(7988) | #65: mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest *,nsISupports *,nsresult) [netwerk/base/nsLoadGroup.cpp:629]
11:34:54     INFO -  GECKO(7988) | #66: nsDocument::DoUnblockOnload() [dom/base/nsDocument.cpp:8971]
11:34:54     INFO -  GECKO(7988) | #67: nsDocument::UnblockOnload(bool) [dom/base/nsDocument.cpp:8892]
11:34:54     INFO -  GECKO(7988) | #68: nsDocument::DispatchContentLoadedEvents() [dom/base/nsDocument.cpp:5417]
11:34:54     INFO -  GECKO(7988) | #69: mozilla::detail::RunnableMethodImpl<nsDocument * const,void ( nsDocument::*)(void),1,0>::Run() [obj-firefox/dist/include/nsThreadUtils.h:1175]
11:34:54     INFO -  GECKO(7988) | #70: mozilla::SchedulerGroup::Runnable::Run() [xpcom/threads/SchedulerGroup.cpp:387]
11:34:54     INFO -  GECKO(7988) | #71: nsThread::ProcessNextEvent(bool,bool *) [xpcom/threads/nsThread.cpp:1447]
11:34:54     INFO -  GECKO(7988) | #72: NS_ProcessNextEvent(nsIThread *,bool) [xpcom/threads/nsThreadUtils.cpp:480]
11:34:54     INFO -  GECKO(7988) | #73: mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:97]
11:34:54     INFO -  GECKO(7988) | #74: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:302]
11:34:54     INFO -  GECKO(7988) | #75: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:320]
11:34:54     INFO -  GECKO(7988) | #76: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:300]
11:34:54     INFO -  GECKO(7988) | #77: nsBaseAppShell::Run() [widget/nsBaseAppShell.cpp:160]
11:34:54     INFO -  GECKO(7988) | #78: nsAppShell::Run() [widget/windows/nsAppShell.cpp:212]
11:34:54     INFO -  GECKO(7988) | #79: XRE_RunAppShell() [toolkit/xre/nsEmbedFunctions.cpp:882]
11:34:54     INFO -  GECKO(7988) | #80: mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate *) [ipc/glue/MessagePump.cpp:270]
11:34:54     INFO -  GECKO(7988) | #81: MessageLoop::RunHandler() [ipc/chromium/src/base/message_loop.cc:320]
11:34:54     INFO -  GECKO(7988) | #82: MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:300]
11:34:54     INFO -  GECKO(7988) | #83: XRE_InitChildProcess(int,char * * const,XREChildData const *) [toolkit/xre/nsEmbedFunctions.cpp:703]
11:34:54     INFO -  GECKO(7988) | #84: content_process_main(mozilla::Bootstrap *,int,char * * const) [ipc/contentproc/plugin-container.cpp:65]
11:34:54     INFO -  GECKO(7988) | #85: NS_internal_main(int,char * *,char * *) [browser/app/nsBrowserApp.cpp:288]
11:34:54     INFO -  GECKO(7988) | #86: wmain [toolkit/xre/nsWindowsWMain.cpp:118]
11:34:54     INFO -  GECKO(7988) | #87: __scrt_common_main_seh [f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:253]
11:34:54     INFO -  GECKO(7988) | #88: KERNEL32.DLL + 0x12774
11:34:54     INFO -  GECKO(7988) | #89: ntdll.dll + 0x70d61

Comment 16

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ebd299dd19d6
Backed out changeset 0aca62e914cf for asserting IsSafeToRun(), at dist\include\mozilla/SchedulerGroup.h:81 e.g. in mochitest dom/events/test/test_bug336682_1.html on Windows 10 x64. r=backout

Comment 17

[Joel Maher ( :jmaher ) (UTC -8)]

when this landed, I see a performance improvement:
== Change summary for alert #8721 (as of August 10 2017 16:42 UTC) ==

Improvements:

 21%  Images summary osx-10-10 opt      7,791,664.51 -> 6,150,685.99
  7%  Heap Unclassified summary osx-10-10 opt 77,126,073.35 -> 71,847,216.20
  5%  Explicit Memory summary osx-10-10 opt 328,954,372.62 -> 311,953,231.77

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8721

when it was backed out it was reverted, looking forward to this landing again.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3a06ab7fda1a
Disable preloaded browser in two mochitests. r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/620d01ac103a
Make preloaded browser use pre-existing content process. r=mconley

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back this out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=123041538&repo=mozilla-inbound

https://hg.mozilla.org/integration/mozilla-inbound/rev/fbdd07bde1cf3cbabba01fadf8b788affe5f32a3

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/9758617c1833
Disabling a perm failing part of a test. r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/6949fdc7f97c
Disable preloaded browser in two mochitests. r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/adf5ed713e0d
Make preloaded browser use pre-existing content process. r=mconley

Comment 21

[Gabor Krizsanits (INACTIVE)]

(In reply to Wes Kocher (:KWierso) from comment #19)
> I had to back this out for failures like
> https://treeherder.mozilla.org/logviewer.html#?job_id=123041538&repo=mozilla-
> inbound
> 
> https://hg.mozilla.org/integration/mozilla-inbound/rev/
> fbdd07bde1cf3cbabba01fadf8b788affe5f32a3

For the record:
This test was already perma failing without my patches on OSX locally. I disabled the failing part for more details: Bug 1315092 Comment 29.

Comment 22

[Joel Maher ( :jmaher ) (UTC -8)]

on the backout, I also see win10 heap unclassified regressions:
== Change summary for alert #8767 (as of August 10 2017 21:31 UTC) ==

Regressions:

  2%  Heap Unclassified summary windows10-64 opt      48,327,727.54 -> 49,331,165.22
  2%  Heap Unclassified summary windows10-64 pgo      48,299,561.99 -> 49,283,845.33

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8767

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/9758617c1833
https://hg.mozilla.org/mozilla-central/rev/6949fdc7f97c
https://hg.mozilla.org/mozilla-central/rev/adf5ed713e0d

Comment 24

[Carsten Book [:Tomcat]]

reopen and backout for test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=123423688&repo=mozilla-central

Comment 25

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/mozilla-central/rev/312b9022c181
Backed out changeset adf5ed713e0d 
https://hg.mozilla.org/mozilla-central/rev/78840b9dc759
Backed out changeset 6949fdc7f97c 
https://hg.mozilla.org/mozilla-central/rev/c498777e8f39
Backed out changeset 9758617c1833 for test failures in own test

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8791ca306138
Disabling a perm failing part of a test. r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/987042361c06
Disable preloaded browser in two mochitests. r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/1bc06a4d1473
Make preloaded browser use pre-existing content process. r=mconley

Comment 27

[Joel Maher ( :jmaher ) (UTC -8)]

many improvements from when this landed:
== Change summary for alert #8803 (as of August 15 2017 12:20 UTC) ==

Improvements:

 29%  Images summary windows7-32 pgo      6,566,655.81 -> 4,686,413.49
 27%  Images summary windows7-32 opt      6,360,747.97 -> 4,668,869.42
 22%  Images summary osx-10-10 opt        7,206,239.33 -> 5,653,849.13
 20%  Images summary linux64 opt          6,888,030.51 -> 5,492,390.03
  7%  Resident Memory summary windows7-32 opt 312,222,186.36 -> 290,739,176.72
  7%  Heap Unclassified summary osx-10-10 opt 75,308,655.83 -> 70,229,550.51
  7%  Explicit Memory summary windows7-32 opt 239,832,748.53 -> 223,901,471.93
  6%  Explicit Memory summary windows7-32 pgo 239,747,212.40 -> 224,448,458.05
  6%  Explicit Memory summary osx-10-10 opt 324,689,646.00 -> 306,291,014.72
  5%  Explicit Memory summary linux64 opt 302,219,293.27 -> 286,917,840.06
  5%  Explicit Memory summary windows10-64 opt 306,051,152.69 -> 290,823,292.87
  5%  Resident Memory summary windows10-64 opt 465,934,965.56 -> 443,688,187.36
  5%  Resident Memory summary windows10-64 pgo 455,883,535.04 -> 434,339,139.39
  4%  Explicit Memory summary windows10-64 pgo 305,651,448.47 -> 292,687,047.94
  4%  Heap Unclassified summary linux64 opt 56,725,292.60 -> 54,358,031.18
  4%  Heap Unclassified summary windows7-32 opt 41,888,388.16 -> 40,245,279.99
  4%  JS summary windows7-32 opt          93,617,337.09 -> 90,331,922.59
  3%  JS summary windows7-32 pgo          93,089,018.46 -> 89,877,096.45
  3%  Heap Unclassified summary windows10-64 pgo 49,440,206.88 -> 47,908,935.96
  3%  JS summary windows10-64 opt         125,061,692.48 -> 121,203,477.58
  3%  Heap Unclassified summary windows10-64 opt 49,424,883.07 -> 47,898,048.11
  3%  JS summary windows10-64 pgo         124,990,889.32 -> 121,386,405.31

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8803

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/8791ca306138
https://hg.mozilla.org/mozilla-central/rev/987042361c06
https://hg.mozilla.org/mozilla-central/rev/1bc06a4d1473

Comment 29

[Gabor Krizsanits (INACTIVE)]

(In reply to Carsten Book [:Tomcat] from comment #24)
> reopen and backout for test failures like
> https://treeherder.mozilla.org/logviewer.html#?job_id=123423688&repo=mozilla-
> central

For the record, for tier-2 failures backout is not needed. See:  bug 1305242 comment 21.

Comment 30

[Ionuț Goldan [:igoldan]]

This improvement notification finally popped up:

== Change summary for alert #8809 (as of August 15 2017 12:06 UTC) ==

Improvements:

  5%  Resident Memory summary windows7-32 pgo      301,608,414.91 -> 286,576,517.37

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=8809