STR: 1. Visit https://docs.google.com/document/d/1tUv4GrFfKt72gX1lQyItHCgF7Xoa5RtIX6Ih_TgJBX4/edit 2. Click on the link If you profile this, we basically spend about 2 seconds in JS execution off an XHR handler. I'm not sure what we can do here. Maybe it's just tons of JS. The main problem here is that you'll get a spinner if you try to switch tabs while this is happening. I think our options are: 1. Contact Google and hope that they can break things up better. 2. Wait for multiple content processes/threads. 3. Some crazy scheme to interrupt JS so that we can draw in the middle of the JS. I'd appreciate it if someone from JS could take a closer look at the profile. Maybe there's more we can do?

Matt, can you think of any reason why we couldn't pause JS execution and do painting? Is there any way that painting can do something that's observable by JS (like run other JS or do layout or modify the DOM)? It's okay if it happens on a runnable--just not right away.

Painting usually flushes all pending style and layout changes, but those shouldn't be visible to JS I think. Anything in JS that would query them would have forced the same flush to have happened.

I can't really reproduce this on OS X with the latest Nightly and a clean profile. After I click the link and then immediately switch to the mozilla.com tab I opened first, I see the spinner for a fraction of a second but it's pretty fast. I can try to profile though..

Here's what I've done so far. The tabbrowser code still needs some work.

Comment on attachment 8764081 [details] [diff] [review] work in progress Matt, can you take a look at the code in TabChild.cpp and let me know what you think? It basically runs any time JS code can run.

Comment on attachment 8764081 [details] [diff] [review] work in progress Review of attachment 8764081 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for taking so long to get to this. When we paint via nsRefreshDriver (by calling FlushPendingUpdates on the view manager), we take advantage of the paint throttling code (IsWaitingForPaint) that stops us from overproducing. Is there any that stops callers of this function from calling it too often, such that we paint at higher than the refresh rate of the screen, or faster than the compositor can keep up with?

This patch is a refactoring to allow us to have multiple interrupt callbacks. This will allow the hang monitor to have its own interrupt callback that does painting.

There isn't much going on here. The main changes are: 1. Display port suppression is now handled in the platform (at least for tab switching). 2. I removed the requestNotifyLayerTreeReady/Cleared methods on the frameloader. The platform now delivers MozLayerTreeReady/Cleared events for remote frameloaders. This is less racy and simpler. For non-remote browsers, I had to fire the events manually. 3. I removed the instrumentation for bug 1166351 since it's fixed now. 4. We now allow a tab that's in the UNLOADING state to be loaded before it moves to UNLOADED. To make this possible, I had to add a bunch of sequence numbering in the platform. Basically, we only get a MozLayerTreeReady event if the layer tree we received has a sequence number that matches the docShellIsActive=true message we sent to the child. That ensures we don't get confused.

If we want to ask a tab to paint while the content process is busy, we use the hang monitor channel/thread. This patch implements that logic. It's mostly just the usual boilerplate to add a new hang monitor message. The epoch is used to give a consistent numbering when paint messages arrive from both the hang monitor channel and from PContent.

This patch covers the compositor side of things. A big part of the patch aims to provide consistent ordering of requests to paint or clear layers for a tab. We're now able to get requests to paint either from the hang monitor channel or from PContent. The requests going to the hang monitor end up on the main thread through the JS operation callback. The first request to hit the main thread should "win". Every time the parent sends a request to paint or clear layers (via TabParent::SetDocShellIsActive) it increments an "epoch" (nonce/sequence number). The child processes these requests in order of this number. If it gets a request with an older number than the previous one it processed, it ignores it. This ensures that if the parent sends requests in this order: 1:paint 2:clear 3:paint (which could correspond to switching away from a tab and then switching back) and if request 3 is processed before request 2 (because clear requests are only sent on PContent) then 2 will never run even if it's received later. It's important that 2 shouldn't run later because it would cancel the effect of request 3, which would cause the tab to turn black when we had switched to it. I also changed how layer observers work to make them less racy. We always install an observer on the compositor and listen for update messages. The epoch value is used to ensure that we only fire events once per SetDocShellIsActive call. Previously we tried to null out the observer after it was fired, but that raced with setting a new observer on the main thread. I removed the RequestNotifyLayerTreeReady/Cleared methods on the frameloader. They just delegate to the TabParent, so they're not necessary. Finally, I moved display port suppression to the platform. It had been done from tabbrowser code, which was kind of gross. Since we know exactly when we're doing a paint for a tab switch, we can suppress the display port at that time.

Comment on attachment 8773986 [details] [diff] [review] tabbrowser changes Review of attachment 8773986 [details] [diff] [review]: ----------------------------------------------------------------- I've read through the other patches and I think I know what's going on. This patch makes sense in context. Pretty happy to see some of the logic go down further into platform! Thanks for this, billm. ::: browser/base/content/tabbrowser.xml @@ -3703,5 @@ > if (tab === this.requestedTab) { > return; > } > > - // Instrumentation to figure out bug 1166351 - if the binding Thanks for removing this stuff! @@ -3850,5 @@ > - } > - > - let b = tab.linkedBrowser; > - > - if (!b._alive) { I think you can remove http://searchfox.org/mozilla-central/rev/336105a0de49f41a2cc203db7b7ee7266d0d558b/toolkit/content/widgets/browser.xml#1370 while you're at it.

Comment on attachment 8773987 [details] [diff] [review] Hang monitor changes Review of attachment 8773987 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/ipc/ProcessHangMonitor.cpp @@ +595,5 @@ > + > +void > +HangMonitorParent::ForcePaintOnThread(TabId aTabId, uint64_t aLayerObserverEpoch) > +{ > + if (mIPCOpen) { Assert that MessageLoop::current() == MonitorLoop() here?

Comment on attachment 8773992 [details] [diff] [review] main patch Review of attachment 8773992 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/ipc/CompositorBridgeParent.cpp @@ +2502,5 @@ > mNotifyAfterRemotePaint = false; > } > > + if (state->mLayerTreeObserver) { > + state->mLayerTreeObserver->ObserveUpdate(id, aLayerTree->GetLayerObserverEpoch(), true); Still reading through the whole patch, but a quick comment - we write to this field inside the lock, but read it without taking the lock. That doesn't work for RefPtr, and it looks like we can in fact update it multiple times on the main thread via SwapLayerTreeObservers. If we're not 100% sure it's safe as-is, this should probably be: RefPtr<CompositorUpdateObserver> observer; { MonitorAutoLock lock(whatever); observer = state->mLayerTreeObserver; } That's a pre-existing problem, anyway. LayerTreeState is not the most inspiring data structure.

Comment on attachment 8773992 [details] [diff] [review] main patch Review of attachment 8773992 [details] [diff] [review]: ----------------------------------------------------------------- Looks good! Only comment is the locking thing. ::: dom/ipc/TabParent.cpp @@ +3020,3 @@ > { > + // Ignore updates from old epochs. They might tell us that layers are > + // available when we've already sent a message ot clear them. We can't trust nit: "to clear them"

These are some additional changes to make sure that docshells are active when we expect them to be. Most of them are related to print preview as we discussed.

This patch reworks the LayerTreeObserver stuff to be simpler. We just do a static method call rather than keeping an observer object. Also removed the swapping, which isn't needed. As part of these changes, I needed a way to track which epoch the parent has seen already so that we don't fire the observer constantly. I added a field to LayerTransactionParent for that. There are also some fixes here. Calling the widget's paint routing can trigger JS, so we need to call nsPresShell::Paint directly. I also found that using the nsIPresShell::PAINT_COMPOSITE triggers an event firing, which can run JS. So I left that out.

Comment on attachment 8779928 [details] [diff] [review] additional C++ changes Review of attachment 8779928 [details] [diff] [review]: ----------------------------------------------------------------- ::: gfx/layers/ipc/LayerTransactionParent.cpp @@ +717,5 @@ > + return true; > +} > + > +bool > +LayerTransactionParent::HasParentObservedEpoch() This name tripped me up. Does "ShouldParentObserveEpoch" make more sense?

Comment on attachment 8779927 [details] [diff] [review] more JS changes Review of attachment 8779927 [details] [diff] [review]: ----------------------------------------------------------------- This looks mostly okay - just a few questions, and one suggestion on how to avoid breakage in other XUL apps. Thanks billm! ::: browser/base/content/tabbrowser.xml @@ +3550,2 @@ > this.loadTimer = this.setTimer(() => this.onLoadTimeout(), this.TAB_SWITCH_TIMEOUT); > + this.setTabState(this.requestedTab, this.STATE_LOADING); Out of curiosity, why was this moved? @@ +4259,5 @@ > break; > case "sizemodechange": > if (aEvent.target == window) { > this.mCurrentBrowser.setDocShellIsActiveAndForeground( > + this.shouldActivateDocShell(this.mCurrentBrowser)); If we see a sizemodechange event while the switcher is around, don't we lose out on the original check here to see if the window is minimized? ::: toolkit/components/printing/content/printUtils.js @@ +571,5 @@ > } > > // Set the original window as an active window so any mozPrintCallbacks can > // run without delayed setTimeouts. > + this._listener.activateBrowser(this._sourceBrowser); This is probably going to break SeaMonkey and Thunderbird's print preview. I think we should either: 1) Fallback to docshell activation is the listener doesn't have activateBrowser defined, or 2) File some new bugs for the SM / TB community so that they know about this incoming change, and what they need to do.

(In reply to Mike Conley (:mconley) - (Needinfo me!) from comment #18) > ::: browser/base/content/tabbrowser.xml > @@ +3550,2 @@ > > this.loadTimer = this.setTimer(() => this.onLoadTimeout(), this.TAB_SWITCH_TIMEOUT); > > + this.setTabState(this.requestedTab, this.STATE_LOADING); > > Out of curiosity, why was this moved? To simplify things, I made the MozLayerTreeReady event trigger synchronously for non-remote tabs (there were issues where the async dispatch was happening much too late). Sync dispatch caused an issue here because the event handler asserts that loadTimer will be non-null. So I just moved the initialization so the assertion passes. > @@ +4259,5 @@ > > break; > > case "sizemodechange": > > if (aEvent.target == window) { > > this.mCurrentBrowser.setDocShellIsActiveAndForeground( > > + this.shouldActivateDocShell(this.mCurrentBrowser)); > > If we see a sizemodechange event while the switcher is around, don't we lose > out on the original check here to see if the window is minimized? Yeah, I guess that's true. I'll post a new patch. > ::: toolkit/components/printing/content/printUtils.js > @@ +571,5 @@ > > } > > > > // Set the original window as an active window so any mozPrintCallbacks can > > // run without delayed setTimeouts. > > + this._listener.activateBrowser(this._sourceBrowser); > > This is probably going to break SeaMonkey and Thunderbird's print preview. > > I think we should either: > > 1) Fallback to docshell activation is the listener doesn't have > activateBrowser defined, or > 2) File some new bugs for the SM / TB community so that they know about this > incoming change, and what they need to do. I think option (1) makes sense.

Comment on attachment 8780758 [details] [diff] [review] minimize fix Review of attachment 8780758 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/base/content/tabbrowser.xml @@ +3637,5 @@ > return; > } > > if (numPending == 0) { > + //this.finish(); Oops. I put this in for testing and forgot to take it out.

Comment on attachment 8780758 [details] [diff] [review] minimize fix Review of attachment 8780758 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, minus one thing (and removing that line you spotted) - see below. ::: toolkit/components/printing/content/printUtils.js @@ +571,5 @@ > } > > // Set the original window as an active window so any mozPrintCallbacks can > // run without delayed setTimeouts. > + if (this._listener.activateBrowser) { We need to handle the "else" here and activate the docShell of the source browser directly for SM / TB.

This is the last patch, I promise. I turns out that the way docshell swapping worked was broken. We used to swap things in the platform so that the MozLayerTreeReady notifications went to opposite <browsers>s after the swap. But that doesn't really solve the fundamental issue, which is that the switcher tracks the layer tree state per tab, rather than per docshell where the state is actually kept. So this patch reorganizes things so that the state that the switcher tracks is swapped when a docshell swap happens. I already took the platform swapping stuff out in a previous patch because it didn't make any sense to me at the time.

Comment on attachment 8781630 [details] [diff] [review] swap fix Review of attachment 8781630 [details] [diff] [review]: ----------------------------------------------------------------- This looks okay I guess, but I have a question. ::: browser/base/content/tabbrowser.xml @@ +3787,5 @@ > // postActions. > } > }, > > + onSwapDocShells(browser1, browser2) { To be clearer, maybe this should be ourBrowser and otherBrowser? I think I've seen that pattern elsewhere when we've dealt with browsers (and gBrowsers) from other windows. @@ +3815,5 @@ > + // case it has been swapped. We also set browser1's state > + // to whatever browser2's state was before the swap. > + > + if (this.loadTimer) { > + this.clearTimer(this.loadTimer); Does this mean after a frame swap, if the otherBrowser was in STATE_LOADING, we don't have the ability to time out anymore?

Comment on attachment 8781630 [details] [diff] [review] swap fix Clearing review request until I hear more regarding comment 24.

(In reply to Mike Conley (:mconley) - (Needinfo me!) from comment #24) > Comment on attachment 8781630 [details] [diff] [review] > swap fix > > Review of attachment 8781630 [details] [diff] [review]: > ----------------------------------------------------------------- > > This looks okay I guess, but I have a question. > > ::: browser/base/content/tabbrowser.xml > @@ +3787,5 @@ > > // postActions. > > } > > }, > > > > + onSwapDocShells(browser1, browser2) { > > To be clearer, maybe this should be ourBrowser and otherBrowser? I think > I've seen that pattern elsewhere when we've dealt with browsers (and > gBrowsers) from other windows. OK, makes sense. > @@ +3815,5 @@ > > + // case it has been swapped. We also set browser1's state > > + // to whatever browser2's state was before the swap. > > + > > + if (this.loadTimer) { > > + this.clearTimer(this.loadTimer); > > Does this mean after a frame swap, if the otherBrowser was in STATE_LOADING, > we don't have the ability to time out anymore? No, it means that we're effectively already timed out. Notice that all that onLoadTimeout does is clear loadTimer/loadingTab. That's because loadTimer is what protects us from showing the requestedTab (instead of the currently visible tab): http://searchfox.org/mozilla-central/rev/ae78ab94fadabc89fc6258d03c4a1a70f763f43a/browser/base/content/tabbrowser.xml#3479 So all that clearing does is cause us to show the spinner right away. I guess we could create a new timer, but this seems pretty esoteric. The only way I know of where we invoke this code when the tab isn't already in the foreground is via "Move to new window" in the tab context menu. And in that case there won't be any other tabs in the window, so we have no choice but to show the spinner anyway.

Comment on attachment 8781630 [details] [diff] [review] swap fix Review of attachment 8781630 [details] [diff] [review]: ----------------------------------------------------------------- Ah, okay. Would you mind adding a comment to that effect where we clear out the load timer?

Pushed by wmccloskey@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/cff59fe2b933 Allow multiple interrupt callbacks (r=dvander) https://hg.mozilla.org/integration/mozilla-inbound/rev/00bb53b58e96 Allow painting for tab switch when JS is running (r=dvander,mconley,mrbkap)

I had to back this out for failures like https://treeherder.mozilla.org/logviewer.html#?job_id=34267185&repo=mozilla-inbound https://hg.mozilla.org/integration/mozilla-inbound/rev/083f1c75c0a4

Pushed by wmccloskey@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/277c54118c8a Allow multiple interrupt callbacks (r=dvander) https://hg.mozilla.org/integration/mozilla-inbound/rev/b1c893387fdd Allow painting for tab switch when JS is running (r=dvander,mconley,mrbkap)

https://hg.mozilla.org/mozilla-central/rev/277c54118c8a https://hg.mozilla.org/mozilla-central/rev/b1c893387fdd

(In reply to Bill McCloskey (:billm) from comment #1) > Matt, can you think of any reason why we couldn't pause JS execution and do > painting? We might be painting the page in an inconsistent state that JS didn't expect. Usually we guarantee that everything that reaches the screen reflects the state from before the current JS started running, or, once the JS is done, the state after. (Except for JS APIs that run nested event loops, like alert().) This change breaks that guarantee. It's unclear to me whether that's a problem. But it's definitely scary. If the JS that's currently running is from a different tab than the one that we're switching to (and doesn't have sync access to it), then this is not a problem. If you only did the forced paint if this is the case, it would greatly reduce the risk of this change.

This patch simplifies some of the logic around setting a docshell as active. It moves the layer stuff out into TabChild where I think it belongs. Part of this change involves removing the InvalidateAllLayersForFrame thing. I asked Matt about this and he said it shouldn't be necessary. David, can you review this part since you looked at the previous stuff? I also got rid of the hidden flag and the setDocShellIsActiveAndForeground thing. None of that made any sense to me. The purpose of it was to ensure that, when we minimize a window, we don't throw away the layers for the foreground tab. That way there's no flashing on MacOS when we unminimize. This patch replaces that with a preserveLayers flag that's set when a window is minimized. I think it's a lot easier to understand and it avoids the bug the previous patches had. Blake, can you review this part?

This patch works around a perf regression. When we open a new tab and immediately switch to it, we send both the normal SetDocShellIsActive message and the ForcePaint message. Normally, SetDocShellIsActive is processed first because no JS is running. But in the new tab case, we're usually loading frame scripts into the new tab, so we are running JS. So ForcePaint runs first. The problem with ForcePaint is that it doesn't trigger a MozAfterPaint event (since we can't run JS from ForcePaint). The tabpaint benchmark therefore never sees this paint. Instead it sees the next paint, which comes from the refresh driver. It happens a little later because the ForcePaint paint took some time. I worked around this by not sending ForcePaint the first time we activate a tab. We will still have tests for the performance of normal (non new-tab) tab switches via the tps benchmark, so we have benchmarks for both kinds of tab switches.

Comment on attachment 8790555 [details] [diff] [review] Simplify SetDocShellIsActive Review of attachment 8790555 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/ipc/TabChild.cpp @@ +2660,5 @@ > // We don't use GetPresShell() here because that would create a content viewer > // if one doesn't exist yet. Creating a content viewer can cause JS to run, > // which we want to avoid. nsIDocShell::GetPresShell returns null if no > // content viewer exists yet. > if (nsCOMPtr<nsIPresShell> presShell = docShell->GetPresShell()) { I missed this in the last review, but this comment seems off: it says "We don't use GetPresShell" and then calls GetPresShell. @@ +2662,5 @@ > // which we want to avoid. nsIDocShell::GetPresShell returns null if no > // content viewer exists yet. > if (nsCOMPtr<nsIPresShell> presShell = docShell->GetPresShell()) { > + if (nsIFrame* root = presShell->FrameConstructor()->GetRootFrame()) { > + root->SchedulePaint(); Why do we need to schedule a paint before painting? (Would be good to comment this, but I'm interested in the answer in the bug too.)

Comment on attachment 8790558 [details] [diff] [review] performance optimization Review of attachment 8790558 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/ipc/TabChild.cpp @@ +2618,5 @@ > // Since SetDocShellIsActive requests come in from both the hang monitor > // channel and the PContent channel, we have an ordering problem. This code > // ensures that we respect the order in which the requests were made and > // ignore stale requests. > + if (mLayerObserverEpoch >= aLayerObserverEpoch) { Was this supposed to be folded into the previous patch?

(In reply to David Anderson [:dvander] from comment #36) > Comment on attachment 8790555 [details] [diff] [review] > Simplify SetDocShellIsActive > > Review of attachment 8790555 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/ipc/TabChild.cpp > @@ +2660,5 @@ > > // We don't use GetPresShell() here because that would create a content viewer > > // if one doesn't exist yet. Creating a content viewer can cause JS to run, > > // which we want to avoid. nsIDocShell::GetPresShell returns null if no > > // content viewer exists yet. > > if (nsCOMPtr<nsIPresShell> presShell = docShell->GetPresShell()) { > > I missed this in the last review, but this comment seems off: it says "We > don't use GetPresShell" and then calls GetPresShell. Yeah, it's confusing. It should say "We don't call TabChildBase::GetPresShell()" since that's the one that creates a presshell if one doesn't exist. The docshell variant doesn't. > @@ +2662,5 @@ > > // which we want to avoid. nsIDocShell::GetPresShell returns null if no > > // content viewer exists yet. > > if (nsCOMPtr<nsIPresShell> presShell = docShell->GetPresShell()) { > > + if (nsIFrame* root = presShell->FrameConstructor()->GetRootFrame()) { > > + root->SchedulePaint(); > > Why do we need to schedule a paint before painting? (Would be good to > comment this, but I'm interested in the answer in the bug too.) I'm not entirely sure. However, we don't always call PresShell::Paint right after. Usually we call PuppetWidget::Paint since that also does layout and some other important stuff. I think that routine might not paint unless we schedule. I could be wrong though.

Comment on attachment 8790555 [details] [diff] [review] Simplify SetDocShellIsActive Review of attachment 8790555 [details] [diff] [review]: ----------------------------------------------------------------- This is so much clearer. Thank you for disentangling this. ::: dom/ipc/PBrowser.ipdl @@ +729,5 @@ > > /** > * Update the child side docShell active (resource use) state. > */ > + async SetDocShellIsActive(bool aIsActive, bool aPreserveLayers, uint64_t aLayerObserverEpoch); Please add some javadoc-style comments here explaining what the two boolean parameters control. Even though it's much clearer than before, I don't think it's immediately obvious.

Pushed by wmccloskey@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0a47c0c2ba34 Allow multiple interrupt callbacks (r=dvander) https://hg.mozilla.org/integration/mozilla-inbound/rev/cfcd8c4f3a36 Allow painting for tab switch when JS is running (r=dvander,mconley,mrbkap)

https://hg.mozilla.org/mozilla-central/rev/0a47c0c2ba34 https://hg.mozilla.org/mozilla-central/rev/cfcd8c4f3a36

(In reply to Matt Woodrow (:mattwoodrow) from comment #2) > Painting usually flushes all pending style and layout changes, but those > shouldn't be visible to JS I think. Anything in JS that would query them > would have forced the same flush to have happened. reflow certainly can trigger JS. Resize event is one example but there are also others. JS is called right after reflow in those cases.

(In reply to Markus Stange [:mstange] from comment #33) > (In reply to Bill McCloskey (:billm) from comment #1) > > Matt, can you think of any reason why we couldn't pause JS execution and do > > painting? > > We might be painting the page in an inconsistent state that JS didn't > expect. Usually we guarantee that everything that reaches the screen > reflects the state from before the current JS started running, or, once the > JS is done, the state after. (Except for JS APIs that run nested event > loops, like alert().) This change breaks that guarantee. > > It's unclear to me whether that's a problem. But it's definitely scary. > > If the JS that's currently running is from a different tab than the one that > we're switching to (and doesn't have sync access to it), then this is not a > problem. If you only did the forced paint if this is the case, it would > greatly reduce the risk of this change. Yeah, there's been a lot of debate about this. I would rather see instant tab switches with no spinner, even if we're drawing an intermediate state. Other people would rather not do this since we've never done it before. I spoke to jst yesterday to make a final decision. He thinks we should paint as soon as we can even if we're painting an intermediate state.

(In reply to Bill McCloskey (:billm) from comment #43) > (In reply to Markus Stange [:mstange] from comment #33) > Yeah, there's been a lot of debate about this. I would rather see instant > tab switches with no spinner, even if we're drawing an intermediate state. > Other people would rather not do this since we've never done it before. I > spoke to jst yesterday to make a final decision. He thinks we should paint > as soon as we can even if we're painting an intermediate state. Sounds good to me. Thanks for addressing this question. It would be nice to call this out in a comment in the code as well.

Here's the telemetry data for this change: The 95th percentile for FX_TAB_SWITCH_TOTAL_E10S_MS dropped from ~400ms to ~260ms. The median dropped from ~52ms to ~47ms. The median value of FX_TAB_SWITCH_SPINNER_VISIBLE_MS dropped from ~250ms to ~125ms. So this had a pretty big impact. Hopefully preempting more stuff will improve the numbers even further.

Awesome! Nice to see some numbers to back up what subjectively felt like an improvement. Do we have the ability to perform a t-test with telemetry data? It would be useful for confirming that changes are statistically significant.

Is this stack possible, and not a duplicate? void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void nsPluginFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) void nsPluginInstanceOwner::NotifyPaintWaiter(nsDisplayListBuilder*) void nsContentUtils::AddScriptRunner(nsIRunnable*) void nsContentUtils::AddScriptRunner(struct already_AddRefed<nsIRunnable>) uint32 nsAsyncScriptLoad::Run() void nsInProcessTabChildGlobal::LoadFrameScript(nsAString_internal*, uint8) void nsMessageManagerScriptExecutor::LoadScriptInternal(nsAString_internal*, uint8) uint8 js::ExecuteInGlobalAndReturnScope(JSContext*, class JS::Handle<JSObject*>, class JS::Handle<JSScript*>, class JS::MutableHandle<JSObject*>) uint8 js::ExecuteKernel(JSContext*, class JS::Handle<JSScript*>, JSObject*, JS::Value*, js::AbstractFramePtr, JS::Value*) uint8 js::RunScript(JSContext*, js::RunState*) Interpreter.cpp:uint8 Interpret(JSContext*, js::RunState*) or this: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void nsPluginFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) void nsPluginInstanceOwner::NotifyPaintWaiter(nsDisplayListBuilder*) void nsContentUtils::AddScriptRunner(nsIRunnable*) void nsContentUtils::AddScriptRunner(struct already_AddRefed<nsIRunnable>) uint32 mozilla::dom::PromiseResolveThenableJob::Run() or avoiding NotifyPaintWaiter and gfxPlatform::Init, void PresShell::Paint(nsView*, nsRegion*, uint32) void nsIFrame::UpdatePaintCountForPaintedPresShells() nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsIPresShell] nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsIPresShell] void nsCOMPtr<T>::assign_from_helper(const nsCOMPtr_helper&, const nsIID&) [with T = nsIPresShell; nsIID = nsID] nsresult nsQueryObject<T>::operator()(const nsIID&, void**) const [with T = Bar; nsIID = nsID] uint32 mozilla::dom::Element::QueryInterface(nsID*, void**) uint32 nsBindingManager::GetBindingImplementation(nsIContent*, nsID*, void**) JSObject* xpc::GetXBLScopeOrGlobal(JSContext*, JSObject*) JSObject* xpc::GetXBLScope(JSContext*, JSObject*) JSObject* XPCWrappedNativeScope::EnsureContentXBLScope(JSContext*) uint32 xpc::CreateSandboxObject(JSContext*, class JS::MutableHandle<JS::Value>, nsISupports*, xpc::SandboxOptions*) JSObject* js::NewProxyObject(JSContext*, js::BaseProxyHandler*, class JS::Handle<JS::Value>, JSObject*, js::ProxyOptions*) js::ProxyObject* js::ProxyObject::New(JSContext*, js::BaseProxyHandler*, class JS::Handle<JS::Value>, js::TaggedProto, js::ProxyOptions*) JSObject* js::NewObjectWithGivenTaggedProto(js::ExclusiveContext*, js::Class*, class JS::Handle<js::TaggedProto>, int32, uint32, uint32) jsobj.cpp:JSObject* NewObject(js::ExclusiveContext*, class JS::Handle<js::ObjectGroup*>, int32, uint32, uint32) but I'm suspicious of that QueryInterface layer, so avoiding that too: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsRootPresContext::ComputePluginGeometryUpdates(nsIFrame*, nsDisplayListBuilder*, nsDisplayList*) void nsRootPresContext::InitApplyPluginGeometryTimer() already_AddRefed<nsITimer> nsPresContext::CreateTimer(nsTimerCallbackFunc, uint32_t) uint32 nsTimerImpl::InitWithFuncCallback((void)(nsITimer*,void*)*, void*, uint32, uint32) uint32 nsTimerImpl::InitWithFuncCallbackCommon((void)(nsITimer*,void*)*, void*, uint32, uint32, class mozilla::Variant<const int, const char*, void (*)(nsITimer*, void*, char*, long unsigned int)>) uint32 nsTimerImpl::InitCommon(uint32, uint32) uint32 TimerThread::Init() uint32 mozilla::dom::PromiseResolveThenableJob::Run() but that has some unlikely-looking init stuff, so skipping that, there's another way to land in promises: void PresShell::Paint(nsView*, nsRegion*, uint32) virtual already_AddRefed<mozilla::layers::ColorLayer> mozilla::layers::ClientLayerManager::CreateColorLayer() void mozilla::layers::CreateShadowFor(mozilla::layers::ClientLayer*, mozilla::layers::ClientLayerManager*, __ptr_member) [with CreatedMethod = void (mozilla::layers::ShadowLayerForwarder::*)(mozilla::layers::ShadowableLayer*)] mozilla::layers::PLayerChild* mozilla::layers::ShadowLayerForwarder::ConstructShadowFor(mozilla::layers::ShadowableLayer*) mozilla::layers::PLayerChild* mozilla::layers::PLayerTransactionChild::SendPLayerConstructor(mozilla::layers::PLayerChild*) uint8 mozilla::ipc::MessageChannel::Send(IPC::Message*) void mozilla::ipc::ProcessLink::SendMessage(IPC::Message*) void MessageLoop::PostTask(struct already_AddRefed<mozilla::Runnable>) void MessageLoop::PostTask_Helper(struct already_AddRefed<mozilla::Runnable>, int32) uint32 mozilla::dom::workers::TimerThreadEventTarget::Dispatch(struct already_AddRefed<nsIRunnable>, uint32) uint32 mozilla::dom::PromiseResolveThenableJob::Run() JSObject* mozilla::dom::Promise::CreateThenableFunction(JSContext*, mozilla::dom::Promise*, uint32) but before I continue further down this road, am I even looking for the right thing? I'm just looking at ways that PresShell::Paint can reach JS. Is that enough to cause an issue here?

ni? billm for comment 47

(In reply to Steve Fink [:sfink] [:s:] from comment #47) > Is this stack possible, and not a duplicate? > > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, > uint32, uint8, uint32) > void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, > nsRect*, nsDisplayList*) > void nsPluginFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, > nsDisplayListSet*) > void nsPluginInstanceOwner::NotifyPaintWaiter(nsDisplayListBuilder*) > void nsContentUtils::AddScriptRunner(nsIRunnable*) > void nsContentUtils::AddScriptRunner(struct already_AddRefed<nsIRunnable>) > uint32 nsAsyncScriptLoad::Run() > void nsInProcessTabChildGlobal::LoadFrameScript(nsAString_internal*, uint8) > void nsMessageManagerScriptExecutor::LoadScriptInternal(nsAString_internal*, > uint8) > uint8 js::ExecuteInGlobalAndReturnScope(JSContext*, class > JS::Handle<JSObject*>, class JS::Handle<JSScript*>, class > JS::MutableHandle<JSObject*>) > uint8 js::ExecuteKernel(JSContext*, class JS::Handle<JSScript*>, JSObject*, > JS::Value*, js::AbstractFramePtr, JS::Value*) > uint8 js::RunScript(JSContext*, js::RunState*) > Interpreter.cpp:uint8 Interpret(JSContext*, js::RunState*) This is sadly possible. Perhaps we need to always hold a script blocker during painting, thanks to NotifyPaintWaiter(). > or this: > > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, > uint32, uint8, uint32) > void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, > nsRect*, nsDisplayList*) > void nsPluginFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, > nsDisplayListSet*) > void nsPluginInstanceOwner::NotifyPaintWaiter(nsDisplayListBuilder*) > void nsContentUtils::AddScriptRunner(nsIRunnable*) > void nsContentUtils::AddScriptRunner(struct already_AddRefed<nsIRunnable>) > uint32 mozilla::dom::PromiseResolveThenableJob::Run() Same as above. > or avoiding NotifyPaintWaiter and gfxPlatform::Init, > > void PresShell::Paint(nsView*, nsRegion*, uint32) > void nsIFrame::UpdatePaintCountForPaintedPresShells() > nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsIPresShell] > nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsIPresShell] > void nsCOMPtr<T>::assign_from_helper(const nsCOMPtr_helper&, const nsIID&) > [with T = nsIPresShell; nsIID = nsID] > nsresult nsQueryObject<T>::operator()(const nsIID&, void**) const [with T = > Bar; nsIID = nsID] > uint32 mozilla::dom::Element::QueryInterface(nsID*, void**) > uint32 nsBindingManager::GetBindingImplementation(nsIContent*, nsID*, void**) > JSObject* xpc::GetXBLScopeOrGlobal(JSContext*, JSObject*) > JSObject* xpc::GetXBLScope(JSContext*, JSObject*) > JSObject* XPCWrappedNativeScope::EnsureContentXBLScope(JSContext*) > uint32 xpc::CreateSandboxObject(JSContext*, class > JS::MutableHandle<JS::Value>, nsISupports*, xpc::SandboxOptions*) > JSObject* js::NewProxyObject(JSContext*, js::BaseProxyHandler*, class > JS::Handle<JS::Value>, JSObject*, js::ProxyOptions*) > js::ProxyObject* js::ProxyObject::New(JSContext*, js::BaseProxyHandler*, > class JS::Handle<JS::Value>, js::TaggedProto, js::ProxyOptions*) > JSObject* js::NewObjectWithGivenTaggedProto(js::ExclusiveContext*, > js::Class*, class JS::Handle<js::TaggedProto>, int32, uint32, uint32) > jsobj.cpp:JSObject* NewObject(js::ExclusiveContext*, class > JS::Handle<js::ObjectGroup*>, int32, uint32, uint32) This one unfortunately seems legit too. :( I can't think of an obvious way to fix it. > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, > uint32, uint8, uint32) > void nsRootPresContext::ComputePluginGeometryUpdates(nsIFrame*, > nsDisplayListBuilder*, nsDisplayList*) > void nsRootPresContext::InitApplyPluginGeometryTimer() > already_AddRefed<nsITimer> nsPresContext::CreateTimer(nsTimerCallbackFunc, > uint32_t) > uint32 nsTimerImpl::InitWithFuncCallback((void)(nsITimer*,void*)*, void*, > uint32, uint32) > uint32 nsTimerImpl::InitWithFuncCallbackCommon((void)(nsITimer*,void*)*, > void*, uint32, uint32, class mozilla::Variant<const int, const char*, void > (*)(nsITimer*, void*, char*, long unsigned int)>) > uint32 nsTimerImpl::InitCommon(uint32, uint32) > uint32 TimerThread::Init() > uint32 mozilla::dom::PromiseResolveThenableJob::Run() > > but that has some unlikely-looking init stuff, so skipping that, there's > another way to land in promises: > > void PresShell::Paint(nsView*, nsRegion*, uint32) > virtual already_AddRefed<mozilla::layers::ColorLayer> > mozilla::layers::ClientLayerManager::CreateColorLayer() > void mozilla::layers::CreateShadowFor(mozilla::layers::ClientLayer*, > mozilla::layers::ClientLayerManager*, __ptr_member) [with CreatedMethod = > void > (mozilla::layers::ShadowLayerForwarder::*)(mozilla::layers:: > ShadowableLayer*)] > mozilla::layers::PLayerChild* > mozilla::layers::ShadowLayerForwarder::ConstructShadowFor(mozilla::layers:: > ShadowableLayer*) > mozilla::layers::PLayerChild* > mozilla::layers::PLayerTransactionChild::SendPLayerConstructor(mozilla:: > layers::PLayerChild*) > uint8 mozilla::ipc::MessageChannel::Send(IPC::Message*) > void mozilla::ipc::ProcessLink::SendMessage(IPC::Message*) > void MessageLoop::PostTask(struct already_AddRefed<mozilla::Runnable>) > void MessageLoop::PostTask_Helper(struct > already_AddRefed<mozilla::Runnable>, int32) > uint32 mozilla::dom::workers::TimerThreadEventTarget::Dispatch(struct > already_AddRefed<nsIRunnable>, uint32) > uint32 mozilla::dom::PromiseResolveThenableJob::Run() > JSObject* mozilla::dom::Promise::CreateThenableFunction(JSContext*, > mozilla::dom::Promise*, uint32) These last two stacks seem bogus to me. It seems like PromiseResolveThenableJob is considered a candidate since it overloads nsIRunnable::Run(), but such a runnable shouldn't be passed in upstack.

Ok, then how about docshell observers: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void nsSubDocumentFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) nsIPresShell* nsSubDocumentFrame::GetSubdocumentPresShellForPainting(uint32) uint32 nsFrameLoader::GetDocShell(nsIDocShell**) uint32 nsFrameLoader::MaybeCreateDocShell() uint32 nsObserverService::NotifyObservers(nsISupports*, int8*, uint16*) void nsObserverList::NotifyObservers(nsISupports*, int8*, uint16*) uint32 FullscreenTransitionTask::Observer::Observe(nsISupports*, int8*, uint16*) uint32 mozilla::dom::PromiseResolveThenableJob::Run() and CSS Zoom factor observers: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void nsPluginFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) void nsPluginInstanceOwner::ResolutionMayHaveChanged() uint32 nsNPAPIPluginInstance::CSSZoomFactorChanged(float32) void NS_NotifyPluginCall(uint32, uint32) uint32 nsObserverService::NotifyObservers(nsISupports*, int8*, uint16*) void nsObserverList::NotifyObservers(nsISupports*, int8*, uint16*) uint32 FullscreenTransitionTask::Observer::Observe(nsISupports*, int8*, uint16*) uint32 mozilla::dom::PromiseResolveThenableJob::Run() and more eventing stuff: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) already_AddRefed<mozilla::layers::LayerManager> nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, uint32_t) uint8 nsLayoutUtils::HasDocumentLevelListenersForApzAwareEvents(nsIPresShell*) uint32 mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, uint32*, mozilla::EventDispatchingCallback*, class nsTArray<mozilla::dom::EventTarget*>*) void mozilla::EventTargetChainItem::HandleEventTargetChain(class nsTArray<mozilla::EventTargetChainItem>*, mozilla::EventChainPostVisitor*, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector*) void mozilla::ESMEventCB::HandleEvent(mozilla::EventChainPostVisitor*) uint32 nsSliderFrame::HandleEvent(nsPresContext*, mozilla::WidgetGUIEvent*, uint32*) void nsSliderFrame::DragThumb(uint8) void nsContentUtils::AddScriptRunner(nsIRunnable*) void nsContentUtils::AddScriptRunner(struct already_AddRefed<nsIRunnable>) uint32 mozilla::dom::PromiseResolveThenableJob::Run() and "apz aware events": void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) already_AddRefed<mozilla::layers::LayerManager> nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, uint32_t) uint8 nsLayoutUtils::HasDocumentLevelListenersForApzAwareEvents(nsIPresShell*) uint32 mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent*, uint32*, mozilla::EventDispatchingCallback*, class nsTArray<mozilla::dom::EventTarget*>*) void mozilla::EventTargetChainItem::HandleEventTargetChain(class nsTArray<mozilla::EventTargetChainItem>*, mozilla::EventChainPostVisitor*, mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector*) void mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor*, mozilla::ELMCreationDetector*) void mozilla::EventListenerManager::HandleEvent(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, uint32*) void mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, uint32*) uint32 mozilla::EventListenerManager::HandleEventSubType(mozilla::EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) uint32 nsXULPopupShownEvent::HandleEvent(nsIDOMEvent*) uint32 mozilla::dom::PromiseResolveThenableJob::Run() or heck, why not spin the event loop? void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsDisplayListBuilder::EnterPresShell(nsIFrame*, uint8) nsIFrame* nsCaret::GetPaintGeometry(nsRect*) void nsCaret::CheckSelectionLanguageChange() nsIBidiKeyboard* nsContentUtils::GetBidiKeyboard() uint32 CallGetService(int8*, nsIBidiKeyboard**) [with DestinationType = nsIBidiKeyboard] uint32 CallGetService(int8*, nsID*, void**) uint32 nsComponentManagerImpl::GetServiceByContractID(int8*, nsID*, void**) uint8 NS_ProcessNextEvent(nsIThread*, uint8) (please tell me that one's not real). I don't even know what's going on here: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsRootPresContext::ApplyPluginGeometryUpdates() uint32 nsWindow::ConfigureChildren(const class nsTArray<nsIWidget::Configuration>*) uint32 mozilla::widget::PuppetWidget::Resize(float64, float64, uint8) uint8 nsView::WindowResized(nsIWidget*, int32, int32) void nsXULPopupManager::PopupResized(nsIFrame*, struct mozilla::gfx::IntSizeTyped<mozilla::LayoutDevicePixel>) uint32 nsIContent::SetAttr(int32, nsIAtom*, nsAString_internal*, uint8) uint32 mozilla::dom::Element::SetAttr(int32, nsIAtom*, nsIAtom*, nsAString_internal*, uint8) uint32 mozilla::dom::HTMLDetailsElement::BeforeSetAttr(int32, nsIAtom*, nsAttrValueOrString*, uint8) WorkerPrivate.cpp:uint32 {anonymous}::KillCloseEventRunnable::Cancel() uint32 mozilla::dom::PromiseResolveThenableJob::Run() I can give more if they're helpful.

(In reply to Steve Fink [:sfink] [:s:] from comment #50) > Ok, then how about docshell observers: > > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, > uint32, uint8, uint32) > void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, > nsRect*, nsDisplayList*) > void nsSubDocumentFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, > nsDisplayListSet*) > nsIPresShell* nsSubDocumentFrame::GetSubdocumentPresShellForPainting(uint32) > uint32 nsFrameLoader::GetDocShell(nsIDocShell**) > uint32 nsFrameLoader::MaybeCreateDocShell() > uint32 nsObserverService::NotifyObservers(nsISupports*, int8*, uint16*) > void nsObserverList::NotifyObservers(nsISupports*, int8*, uint16*) > uint32 FullscreenTransitionTask::Observer::Observe(nsISupports*, int8*, > uint16*) > uint32 mozilla::dom::PromiseResolveThenableJob::Run() Hahaha! So what we want to do here is not call a lazy getter such as nsFrameLoader::GetDocShell() which would go ahead and try to create a docshell for you if one doesn't exist. > and CSS Zoom factor observers: > > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, > uint32, uint8, uint32) > void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, > nsRect*, nsDisplayList*) > void nsPluginFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, > nsDisplayListSet*) > void nsPluginInstanceOwner::ResolutionMayHaveChanged() > uint32 nsNPAPIPluginInstance::CSSZoomFactorChanged(float32) > void NS_NotifyPluginCall(uint32, uint32) > uint32 nsObserverService::NotifyObservers(nsISupports*, int8*, uint16*) > void nsObserverList::NotifyObservers(nsISupports*, int8*, uint16*) > uint32 FullscreenTransitionTask::Observer::Observe(nsISupports*, int8*, > uint16*) > uint32 mozilla::dom::PromiseResolveThenableJob::Run() This is a false positive. The observer notification is experimental-notify-plugin-call which isn't used in-tree. But this is super dangerous in case an add-on registers an observer for that, implemented in JS. > and more eventing stuff: > > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, > uint32, uint8, uint32) > already_AddRefed<mozilla::layers::LayerManager> > nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, > uint32_t) > uint8 > nsLayoutUtils::HasDocumentLevelListenersForApzAwareEvents(nsIPresShell*) > uint32 mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, > mozilla::WidgetEvent*, nsIDOMEvent*, uint32*, > mozilla::EventDispatchingCallback*, class > nsTArray<mozilla::dom::EventTarget*>*) > void mozilla::EventTargetChainItem::HandleEventTargetChain(class > nsTArray<mozilla::EventTargetChainItem>*, mozilla::EventChainPostVisitor*, > mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector*) > void mozilla::ESMEventCB::HandleEvent(mozilla::EventChainPostVisitor*) > uint32 nsSliderFrame::HandleEvent(nsPresContext*, mozilla::WidgetGUIEvent*, > uint32*) > void nsSliderFrame::DragThumb(uint8) > void nsContentUtils::AddScriptRunner(nsIRunnable*) > void nsContentUtils::AddScriptRunner(struct already_AddRefed<nsIRunnable>) > uint32 mozilla::dom::PromiseResolveThenableJob::Run() HasDocumentLevelListenersForApzAwareEvents() passes a bunch of nullptrs into Dispatch() which cause it to not actually dispatch an event and bail out early, so this is a false positive. > and "apz aware events": > > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, > uint32, uint8, uint32) > already_AddRefed<mozilla::layers::LayerManager> > nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, > uint32_t) > uint8 > nsLayoutUtils::HasDocumentLevelListenersForApzAwareEvents(nsIPresShell*) > uint32 mozilla::EventDispatcher::Dispatch(nsISupports*, nsPresContext*, > mozilla::WidgetEvent*, nsIDOMEvent*, uint32*, > mozilla::EventDispatchingCallback*, class > nsTArray<mozilla::dom::EventTarget*>*) > void mozilla::EventTargetChainItem::HandleEventTargetChain(class > nsTArray<mozilla::EventTargetChainItem>*, mozilla::EventChainPostVisitor*, > mozilla::EventDispatchingCallback*, mozilla::ELMCreationDetector*) > void > mozilla::EventTargetChainItem::HandleEvent(mozilla::EventChainPostVisitor*, > mozilla::ELMCreationDetector*) > void mozilla::EventListenerManager::HandleEvent(nsPresContext*, > mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, uint32*) > void mozilla::EventListenerManager::HandleEventInternal(nsPresContext*, > mozilla::WidgetEvent*, nsIDOMEvent**, mozilla::dom::EventTarget*, uint32*) > uint32 > mozilla::EventListenerManager::HandleEventSubType(mozilla:: > EventListenerManager::Listener*, nsIDOMEvent*, mozilla::dom::EventTarget*) > uint32 nsXULPopupShownEvent::HandleEvent(nsIDOMEvent*) > uint32 mozilla::dom::PromiseResolveThenableJob::Run() This is exactly like the one before. > or heck, why not spin the event loop? > > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, > uint32, uint8, uint32) > void nsDisplayListBuilder::EnterPresShell(nsIFrame*, uint8) > nsIFrame* nsCaret::GetPaintGeometry(nsRect*) > void nsCaret::CheckSelectionLanguageChange() > nsIBidiKeyboard* nsContentUtils::GetBidiKeyboard() > uint32 CallGetService(int8*, nsIBidiKeyboard**) [with DestinationType = > nsIBidiKeyboard] > uint32 CallGetService(int8*, nsID*, void**) > uint32 nsComponentManagerImpl::GetServiceByContractID(int8*, nsID*, void**) > uint8 NS_ProcessNextEvent(nsIThread*, uint8) Holy $#%@$. AFAIK the event loop spinning business only happens the first time a service is accessed, but still. > (please tell me that one's not real). I don't even know what's going on here: > > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, > uint32, uint8, uint32) > void nsRootPresContext::ApplyPluginGeometryUpdates() > uint32 nsWindow::ConfigureChildren(const class > nsTArray<nsIWidget::Configuration>*) > uint32 mozilla::widget::PuppetWidget::Resize(float64, float64, uint8) > uint8 nsView::WindowResized(nsIWidget*, int32, int32) > void nsXULPopupManager::PopupResized(nsIFrame*, struct > mozilla::gfx::IntSizeTyped<mozilla::LayoutDevicePixel>) > uint32 nsIContent::SetAttr(int32, nsIAtom*, nsAString_internal*, uint8) > uint32 mozilla::dom::Element::SetAttr(int32, nsIAtom*, nsIAtom*, > nsAString_internal*, uint8) > uint32 mozilla::dom::HTMLDetailsElement::BeforeSetAttr(int32, nsIAtom*, > nsAttrValueOrString*, uint8) > WorkerPrivate.cpp:uint32 {anonymous}::KillCloseEventRunnable::Cancel() > uint32 mozilla::dom::PromiseResolveThenableJob::Run() False positive. There's a call to nsIRunnable::Cancel() from HTMLDetailsElement::BeforeSetAttr(), and while KillCloseEventRunnable::Cancel() is a suitable candidate and is picked by the static analysis, it's not really a possible runnable type here, so this call stack isn't possible to get to at runtime.

Ok, then, this is kinda fun. We could spin the event loop again, I think in the same first-call scenario: void PresShell::Paint(nsView*, nsRegion*, uint32) uint8 mozilla::layers::ClientLayerManager::EndEmptyTransaction(uint32) void mozilla::layers::ClientLayerManager::ForwardTransaction(uint8) void mozilla::dom::TabChild::DidRequestComposite(mozilla::TimeStamp*, mozilla::TimeStamp*) nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsIDocShell] nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsIDocShell] void nsCOMPtr<T>::assign_from_helper(const nsCOMPtr_helper&, const nsIID&) [with T = nsIDocShell; nsIID = nsID] uint32 nsGetServiceFromCategory::operator(nsID*, void**)(const nsIID&, void**) const uint32 nsComponentManagerImpl::GetService(nsID*, nsID*, void**) uint8 NS_ProcessNextEvent(nsIThread*, uint8) or for this shadow layer thing, but maybe this PostTask stuff isn't real?: void PresShell::Paint(nsView*, nsRegion*, uint32) virtual already_AddRefed<mozilla::layers::ColorLayer> mozilla::layers::ClientLayerManager::CreateColorLayer() void mozilla::layers::CreateShadowFor(mozilla::layers::ClientLayer*, mozilla::layers::ClientLayerManager*, __ptr_member) [with CreatedMethod = void (mozilla::layers::ShadowLayerForwarder::*)(mozilla::layers::ShadowableLayer*)] mozilla::layers::PLayerChild* mozilla::layers::ShadowLayerForwarder::ConstructShadowFor(mozilla::layers::ShadowableLayer*) mozilla::layers::PLayerChild* mozilla::layers::PLayerTransactionChild::SendPLayerConstructor(mozilla::layers::PLayerChild*) uint8 mozilla::ipc::MessageChannel::Send(IPC::Message*) void mozilla::ipc::ProcessLink::SendMessage(IPC::Message*) void MessageLoop::PostTask(struct already_AddRefed<mozilla::Runnable>) void MessageLoop::PostTask_Helper(struct already_AddRefed<mozilla::Runnable>, int32) uint32 nsThreadPool::Dispatch(struct already_AddRefed<nsIRunnable>, uint32) uint8 NS_ProcessNextEvent(nsIThread*, uint8) Ignoring that gives me some more questionable-looking QI stuff: void PresShell::Paint(nsView*, nsRegion*, uint32) uint8 nsPresContext::MayHavePaintEventListenerInSubDocument() uint8 nsPresContext::MayHavePaintEventListener() nsPresContext.cpp:uint8 MayHavePaintEventListener(nsPIDOMWindowInner*) nsCOMPtr<T>& nsCOMPtr<T>::operator=(nsQueryInterface) [with T = nsINode] void nsCOMPtr<T>::assign_from_qi(nsQueryInterface, const nsIID&) [with T = nsINode; nsIID = nsID] uint32 nsQueryInterface::operator(nsID*, void**)(const nsIID&, void**) const uint32 nsXPCWrappedJS::QueryInterface(nsID*, void**) uint32 nsXPCWrappedJSClass::DelegatedQueryInterface(nsXPCWrappedJS*, nsID*, void**) JSObject* nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject(JSContext*, JSObject*, nsID*) uint8 JS_ValueToObject(JSContext*, class JS::Handle<JS::Value>, class JS::MutableHandle<JSObject*>) JSObject* JS::ToObject(JSContext*, class JS::Handle<JS::Value>) so I can avoid CallQueryInterfaceOnJSObject, and we go back to my very first stack, but now showing that we don't need NotifyPaintWaiter: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void nsHTMLScrollFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) void mozilla::ScrollFrameHelper::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) uint8 nsLayoutUtils::SetDisplayPortMargins(nsIContent*, nsIPresShell*, struct mozilla::gfx::MarginTyped<mozilla::ScreenPixel>*, uint32, uint8) void PresShell::ScheduleApproximateFrameVisibilityUpdateNow() uint32 NS_DispatchToCurrentThread(nsIRunnable*) uint32 NS_DispatchToCurrentThread(struct already_AddRefed<nsIRunnable>*) uint32 nsThreadPool::Dispatch(struct already_AddRefed<nsIRunnable>, uint32) uint8 NS_ProcessNextEvent(nsIThread*, uint8) which makes it fall back on its GetServiceByContractID trick that we've seen before: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void nsComboboxControlFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) nsITheme* nsPresContext::GetTheme() nsCOMPtr<T>& nsCOMPtr<T>::operator=(nsGetServiceByContractID) [with T = nsITheme] void nsCOMPtr<T>::assign_from_gs_contractid(nsGetServiceByContractID, const nsIID&) [with T = nsITheme; nsIID = nsID] uint32 nsGetServiceByContractID::operator(nsID*, void**)(const nsIID&, void**) const uint32 CallGetService(int8*, nsID*, void**) uint32 nsComponentManagerImpl::GetServiceByContractID(int8*, nsID*, void**) uint8 NS_ProcessNextEvent(nsIThread*, uint8) so I'll just take that out. That gives me something that looks to by untrained eye to be rather clever: void PresShell::Paint(nsView*, nsRegion*, uint32) uint8 mozilla::layers::ClientLayerManager::EndEmptyTransaction(uint32) void mozilla::layers::ClientLayerManager::ForwardTransaction(uint8) void nsRefreshDriver::RevokeTransactionId(uint64) void nsRefreshDriver::FinishedWaitingForTransaction() void nsRefreshDriver::DoRefresh() void nsRefreshDriver::DoTick() void nsRefreshDriver::Tick(int64, mozilla::TimeStamp) void nsViewManager::ProcessPendingUpdates() void nsViewManager::CallWillPaintOnObservers() void PresShell::WillPaint() void nsRootPresContext::FlushWillPaintObservers() uint32 mozilla::dom::PromiseResolveThenableJob::Run() (note that it'll find a patch through MakeSnapshotIfRequired if you take away RevokeTransactionId). After that, I think perhaps I should blacklist nsComponentManagerImpl::GetService because it uses that again: void PresShell::Paint(nsView*, nsRegion*, uint32) void nsPresContext::NotifyInvalidation(struct mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits>*, uint32) void nsPresContext::NotifyInvalidation(nsRect*, uint32) void nsRootPresContext::EnsureEventualDidPaintEvent() already_AddRefed<nsITimer> nsPresContext::CreateTimer(nsTimerCallbackFunc, uint32_t) nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsITimer] nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsITimer] void nsCOMPtr<T>::assign_from_helper(const nsCOMPtr_helper&, const nsIID&) [with T = nsITimer; nsIID = nsID] uint32 nsGetServiceFromCategory::operator(nsID*, void**)(const nsIID&, void**) const uint32 nsComponentManagerImpl::GetService(nsID*, nsID*, void**) uint8 NS_ProcessNextEvent(nsIThread*, uint8) Taking that toy away (nsComponentManagerImpl::GetService) gives: void PresShell::Paint(nsView*, nsRegion*, uint32) virtual already_AddRefed<mozilla::layers::ColorLayer> mozilla::layers::ClientLayerManager::CreateColorLayer() void mozilla::layers::CreateShadowFor(mozilla::layers::ClientLayer*, mozilla::layers::ClientLayerManager*, __ptr_member) [with CreatedMethod = void (mozilla::layers::ShadowLayerForwarder::*)(mozilla::layers::ShadowableLayer*)] mozilla::layers::PLayerChild* mozilla::layers::ShadowLayerForwarder::ConstructShadowFor(mozilla::layers::ShadowableLayer*) mozilla::layers::PLayerChild* mozilla::layers::PLayerTransactionChild::SendPLayerConstructor(mozilla::layers::PLayerChild*) uint8 mozilla::ipc::MessageChannel::Send(IPC::Message*) void mozilla::ipc::ProcessLink::SendMessage(IPC::Message*) uint32 CrashReporter::AnnotateCrashReport(nsACString_internal*, nsACString_internal*) uint32 NS_DispatchToMainThread(nsIRunnable*, uint32) uint32 NS_DispatchToMainThread(struct already_AddRefed<nsIRunnable>*, uint32) uint32 nsThreadPool::Dispatch(struct already_AddRefed<nsIRunnable>, uint32) uint8 NS_ProcessNextEvent(nsIThread*, uint8) which I almost threw out because it has CrashReporter in it, but that looks like an unconditional call. With it out we do some more QI fun: void PresShell::Paint(nsView*, nsRegion*, uint32) uint8 nsPresContext::MayHavePaintEventListenerInSubDocument() uint8 nsPresContext::MayHavePaintEventListener() nsPresContext.cpp:uint8 MayHavePaintEventListener(nsPIDOMWindowInner*) nsCOMPtr<T>& nsCOMPtr<T>::operator=(nsQueryInterface) [with T = nsINode] void nsCOMPtr<T>::assign_from_qi(nsQueryInterface, const nsIID&) [with T = nsINode; nsIID = nsID] uint32 nsQueryInterface::operator(nsID*, void**)(const nsIID&, void**) const uint32 nsXPCWrappedJS::QueryInterface(nsID*, void**) uint32 nsXPCWrappedJSClass::DelegatedQueryInterface(nsXPCWrappedJS*, nsID*, void**) void XPCCallContext::XPCCallContext(uint32, JSContext*, class JS::Handle<JSObject*>, class JS::Handle<JSObject*>, class JS::Handle<jsid>, uint32, JS::Value*, JS::Value*) void XPCCallContext::XPCCallContext(uint32, JSContext*, class JS::Handle<JSObject*>, class JS::Handle<JSObject*>, class JS::Handle<jsid>, uint32, JS::Value*, JS::Value*) void JS_ReportError(JSContext*, int8*) I'll post this much for now.

I went through all the stuff we've seen so far, excepting Steve's most recent comment: Fonts: We have fixes for this. Sync messages (SendScreenForBrowser and SendGetWidgetNativeData): I think we can make these nested(inside_cpow) messages, in which case we won't run any CPOW messages here. DocShell timeline markers: Ehsan fixed this one. NotifyPaintWaiter: This is supposed to be a test-only issue, although in theory I think web content could intercept MozPaintWait events. I'm not sure how to fix it. The comment says "Run this event as soon as it's safe to do so". I think maybe we should avoid firing this event at all in this situation. The "real" paint that happens later can run it. UpdatePaintCountForPaintedPresShells: I think this one is actually a false positive. The only items in this list are nsIPresShells, and the only implementor of that is PresShell. So we should never call Element::QI. NS_NotifyPluginCall: Let's remove the observer. Two add-ons use this. One is some AV thing and the other is a plugin performance monitor, which isn't really relevant when Flash is the only plugin. Bidi keyboard service: The nested event loop is only if another thread is currently instantiating the service. The Bidi keyboard service seems to be main thread only. We should assert that, but I don't think we need to do anything else. Finally, we have the OnPageHide thing when the SVG document is destroyed. I assume we can put off the destruction of these objects for a little while if we're in one of these special paints.

(In reply to Steve Fink [:sfink] [:s:] from comment #52) > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint8 mozilla::layers::ClientLayerManager::EndEmptyTransaction(uint32) > void mozilla::layers::ClientLayerManager::ForwardTransaction(uint8) > void mozilla::dom::TabChild::DidRequestComposite(mozilla::TimeStamp*, > mozilla::TimeStamp*) > nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsIDocShell] > nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsIDocShell] > void nsCOMPtr<T>::assign_from_helper(const nsCOMPtr_helper&, const nsIID&) > [with T = nsIDocShell; nsIID = nsID] > uint32 nsGetServiceFromCategory::operator(nsID*, void**)(const nsIID&, > void**) const > uint32 nsComponentManagerImpl::GetService(nsID*, nsID*, void**) > uint8 NS_ProcessNextEvent(nsIThread*, uint8) We'll always have an nsDocShell for WebNavigation(), so this should never happen. > void PresShell::Paint(nsView*, nsRegion*, uint32) > virtual already_AddRefed<mozilla::layers::ColorLayer> > mozilla::layers::ClientLayerManager::CreateColorLayer() > void mozilla::layers::CreateShadowFor(mozilla::layers::ClientLayer*, > mozilla::layers::ClientLayerManager*, __ptr_member) [with CreatedMethod = > void > (mozilla::layers::ShadowLayerForwarder::*)(mozilla::layers:: > ShadowableLayer*)] > mozilla::layers::PLayerChild* > mozilla::layers::ShadowLayerForwarder::ConstructShadowFor(mozilla::layers:: > ShadowableLayer*) > mozilla::layers::PLayerChild* > mozilla::layers::PLayerTransactionChild::SendPLayerConstructor(mozilla:: > layers::PLayerChild*) > uint8 mozilla::ipc::MessageChannel::Send(IPC::Message*) > void mozilla::ipc::ProcessLink::SendMessage(IPC::Message*) > void MessageLoop::PostTask(struct already_AddRefed<mozilla::Runnable>) > void MessageLoop::PostTask_Helper(struct > already_AddRefed<mozilla::Runnable>, int32) > uint32 nsThreadPool::Dispatch(struct already_AddRefed<nsIRunnable>, uint32) > uint8 NS_ProcessNextEvent(nsIThread*, uint8) We're not doing sync dispatch here and we're not dispatching to a thread pool, so this is a false positive. > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint8 nsPresContext::MayHavePaintEventListenerInSubDocument() > uint8 nsPresContext::MayHavePaintEventListener() > nsPresContext.cpp:uint8 MayHavePaintEventListener(nsPIDOMWindowInner*) > nsCOMPtr<T>& nsCOMPtr<T>::operator=(nsQueryInterface) [with T = nsINode] > void nsCOMPtr<T>::assign_from_qi(nsQueryInterface, const nsIID&) [with T = > nsINode; nsIID = nsID] > uint32 nsQueryInterface::operator(nsID*, void**)(const nsIID&, void**) const > uint32 nsXPCWrappedJS::QueryInterface(nsID*, void**) > uint32 nsXPCWrappedJSClass::DelegatedQueryInterface(nsXPCWrappedJS*, nsID*, > void**) > JSObject* nsXPCWrappedJSClass::CallQueryInterfaceOnJSObject(JSContext*, > JSObject*, nsID*) > uint8 JS_ValueToObject(JSContext*, class JS::Handle<JS::Value>, class > JS::MutableHandle<JSObject*>) > JSObject* JS::ToObject(JSContext*, class JS::Handle<JS::Value>) nsINode can't be implemented in JS, so false positive. > so I can avoid CallQueryInterfaceOnJSObject, and we go back to my very first > stack, but now showing that we don't need NotifyPaintWaiter: > > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, > uint32, uint8, uint32) > void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, > nsRect*, nsDisplayList*) > void nsHTMLScrollFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, > nsDisplayListSet*) > void mozilla::ScrollFrameHelper::BuildDisplayList(nsDisplayListBuilder*, > nsRect*, nsDisplayListSet*) > uint8 nsLayoutUtils::SetDisplayPortMargins(nsIContent*, nsIPresShell*, > struct mozilla::gfx::MarginTyped<mozilla::ScreenPixel>*, uint32, uint8) > void PresShell::ScheduleApproximateFrameVisibilityUpdateNow() > uint32 NS_DispatchToCurrentThread(nsIRunnable*) > uint32 NS_DispatchToCurrentThread(struct already_AddRefed<nsIRunnable>*) > uint32 nsThreadPool::Dispatch(struct already_AddRefed<nsIRunnable>, uint32) > uint8 NS_ProcessNextEvent(nsIThread*, uint8) Again, no sync dispatch and no thread pool. > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, > uint32, uint8, uint32) > void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, > nsRect*, nsDisplayList*) > void nsComboboxControlFrame::BuildDisplayList(nsDisplayListBuilder*, > nsRect*, nsDisplayListSet*) > nsITheme* nsPresContext::GetTheme() > nsCOMPtr<T>& nsCOMPtr<T>::operator=(nsGetServiceByContractID) [with T = > nsITheme] > void nsCOMPtr<T>::assign_from_gs_contractid(nsGetServiceByContractID, const > nsIID&) [with T = nsITheme; nsIID = nsID] > uint32 nsGetServiceByContractID::operator(nsID*, void**)(const nsIID&, > void**) const > uint32 CallGetService(int8*, nsID*, void**) > uint32 nsComponentManagerImpl::GetServiceByContractID(int8*, nsID*, void**) > uint8 NS_ProcessNextEvent(nsIThread*, uint8) Same as the bidi keyboard thing. I don't think this service can be used off the main thread. > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint8 mozilla::layers::ClientLayerManager::EndEmptyTransaction(uint32) > void mozilla::layers::ClientLayerManager::ForwardTransaction(uint8) > void nsRefreshDriver::RevokeTransactionId(uint64) > void nsRefreshDriver::FinishedWaitingForTransaction() > void nsRefreshDriver::DoRefresh() > void nsRefreshDriver::DoTick() > void nsRefreshDriver::Tick(int64, mozilla::TimeStamp) > void nsViewManager::ProcessPendingUpdates() > void nsViewManager::CallWillPaintOnObservers() > void PresShell::WillPaint() > void nsRootPresContext::FlushWillPaintObservers() > uint32 mozilla::dom::PromiseResolveThenableJob::Run() This one looks kind of scary. Any way of getting to DoRefresh or DoTick is bad. I don't really understand what's going on here though. > void PresShell::Paint(nsView*, nsRegion*, uint32) > void nsPresContext::NotifyInvalidation(struct > mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits>*, uint32) > void nsPresContext::NotifyInvalidation(nsRect*, uint32) > void nsRootPresContext::EnsureEventualDidPaintEvent() > already_AddRefed<nsITimer> nsPresContext::CreateTimer(nsTimerCallbackFunc, > uint32_t) > nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsITimer] > nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = nsITimer] > void nsCOMPtr<T>::assign_from_helper(const nsCOMPtr_helper&, const nsIID&) > [with T = nsITimer; nsIID = nsID] > uint32 nsGetServiceFromCategory::operator(nsID*, void**)(const nsIID&, > void**) const > uint32 nsComponentManagerImpl::GetService(nsID*, nsID*, void**) > uint8 NS_ProcessNextEvent(nsIThread*, uint8) I think it's saying we could spin the event loop while waiting for another thread to start the timer service. This one might be possible I guess, but it seems incredibly unlikely. > void PresShell::Paint(nsView*, nsRegion*, uint32) > virtual already_AddRefed<mozilla::layers::ColorLayer> > mozilla::layers::ClientLayerManager::CreateColorLayer() > void mozilla::layers::CreateShadowFor(mozilla::layers::ClientLayer*, > mozilla::layers::ClientLayerManager*, __ptr_member) [with CreatedMethod = > void > (mozilla::layers::ShadowLayerForwarder::*)(mozilla::layers:: > ShadowableLayer*)] > mozilla::layers::PLayerChild* > mozilla::layers::ShadowLayerForwarder::ConstructShadowFor(mozilla::layers:: > ShadowableLayer*) > mozilla::layers::PLayerChild* > mozilla::layers::PLayerTransactionChild::SendPLayerConstructor(mozilla:: > layers::PLayerChild*) > uint8 mozilla::ipc::MessageChannel::Send(IPC::Message*) > void mozilla::ipc::ProcessLink::SendMessage(IPC::Message*) > uint32 CrashReporter::AnnotateCrashReport(nsACString_internal*, > nsACString_internal*) > uint32 NS_DispatchToMainThread(nsIRunnable*, uint32) > uint32 NS_DispatchToMainThread(struct already_AddRefed<nsIRunnable>*, uint32) > uint32 nsThreadPool::Dispatch(struct already_AddRefed<nsIRunnable>, uint32) > uint8 NS_ProcessNextEvent(nsIThread*, uint8) Another synchronous dispatch false positive. > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint8 nsPresContext::MayHavePaintEventListenerInSubDocument() > uint8 nsPresContext::MayHavePaintEventListener() > nsPresContext.cpp:uint8 MayHavePaintEventListener(nsPIDOMWindowInner*) > nsCOMPtr<T>& nsCOMPtr<T>::operator=(nsQueryInterface) [with T = nsINode] > void nsCOMPtr<T>::assign_from_qi(nsQueryInterface, const nsIID&) [with T = > nsINode; nsIID = nsID] > uint32 nsQueryInterface::operator(nsID*, void**)(const nsIID&, void**) const > uint32 nsXPCWrappedJS::QueryInterface(nsID*, void**) > uint32 nsXPCWrappedJSClass::DelegatedQueryInterface(nsXPCWrappedJS*, nsID*, > void**) > void XPCCallContext::XPCCallContext(uint32, JSContext*, class > JS::Handle<JSObject*>, class JS::Handle<JSObject*>, class JS::Handle<jsid>, > uint32, JS::Value*, JS::Value*) > void XPCCallContext::XPCCallContext(uint32, JSContext*, class > JS::Handle<JSObject*>, class JS::Handle<JSObject*>, class JS::Handle<jsid>, > uint32, JS::Value*, JS::Value*) > void JS_ReportError(JSContext*, int8*) This looks very similar to the other nsINode one. Also a false positive. We'll need the opinion of a gfx person about that refresh driver one.

Matt, can the following stack happen in real life? > void PresShell::Paint(nsView*, nsRegion*, uint32) > uint8 mozilla::layers::ClientLayerManager::EndEmptyTransaction(uint32) > void mozilla::layers::ClientLayerManager::ForwardTransaction(uint8) > void nsRefreshDriver::RevokeTransactionId(uint64) > void nsRefreshDriver::FinishedWaitingForTransaction() > void nsRefreshDriver::DoRefresh() If we can run the refresh driver from painting, then we can run JS, which would be bad. If this can happen, is there a way to disable it from these paints that happen inside JS?

This doesn't look likely: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsDisplayListBuilder::EnterPresShell(nsIFrame*, uint8) void PresShell::UpdateCanvasBackground() nsPresShell.cpp:uint8 IsTransparentContainerElement(nsPresContext*) already_AddRefed<nsIPresShell> mozilla::dom::TabChildBase::GetPresShell() const already_AddRefed<nsIDocument> mozilla::dom::TabChildBase::GetDocument() const uint32 nsDocShell::GetDocument(nsIDOMDocument**) uint32 nsDocShell::EnsureContentViewer() uint32 nsDocShell::CreateAboutBlankContentViewer(nsIPrincipal*, nsIURI*, uint8) uint32 nsContentDLF::CreateBlankDocument(nsILoadGroup*, nsIPrincipal*, nsIDocument**) void nsDocument::SetDocumentCharacterSet(nsACString_internal*) uint32 FullscreenTransitionTask::Observer::Observe(nsISupports*, int8*, uint16*) uint32 mozilla::dom::PromiseResolveThenableJob::Run() I don't know enough to say anything about: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void nsImageFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) uint8 nsImageFrame::ShouldDisplaySelection() uint32 nsFrame::GetSelectionController(nsPresContext*, nsISelectionController**) uint32 CallQueryInterface(nsIPresShell*, nsISelectionController**) [with T = nsIPresShell; DestinationType = nsISelectionController] uint32 nsXPCWrappedJS::QueryInterface(nsID*, void**) uint32 nsXPCWrappedJSClass::DelegatedQueryInterface(nsXPCWrappedJS*, nsID*, void**) void XPCCallContext::XPCCallContext(uint32, JSContext*, class JS::Handle<JSObject*>, class JS::Handle<JSObject*>, class JS::Handle<jsid>, uint32, JS::Value*, JS::Value*) void XPCCallContext::XPCCallContext(uint32, JSContext*, class JS::Handle<JSObject*>, class JS::Handle<JSObject*>, class JS::Handle<jsid>, uint32, JS::Value*, JS::Value*) void JS_ReportError(JSContext*, int8*) but this seems disturbing (capturing a stack for a timeline marker): void PresShell::Paint(nsView*, nsRegion*, uint32) uint8 mozilla::layers::ClientLayerManager::EndEmptyTransaction(uint32) void mozilla::layers::ClientLayerManager::ForwardTransaction(uint8) void mozilla::dom::TabChild::DidRequestComposite(mozilla::TimeStamp*, mozilla::TimeStamp*) void mozilla::TimelineConsumers::AddMarkerForDocShell(nsDocShell*, int8*, mozilla::TimeStamp*, int32) typename mozilla::detail::UniqueSelector<T>::SingleObject mozilla::MakeUnique(Args&& ...) [with T = mozilla::TimelineMarker; Args = {const char*&, const mozilla::TimeStamp&, mozilla::MarkerTracingType&}; typename mozilla::detail::UniqueSelector<T>::SingleObject = mozilla::UniquePtr<mozilla::TimelineMarker, mozilla::DefaultDelete<mozilla::TimelineMarker> >] void mozilla::TimelineMarker::TimelineMarker(int8*, mozilla::TimeStamp*, int32, int32) void mozilla::TimelineMarker::TimelineMarker(int8*, mozilla::TimeStamp*, int32, int32) void mozilla::TimelineMarker::CaptureStackIfNecessary(int32, int32) void mozilla::TimelineMarker::CaptureStack() uint8 JS::CaptureCurrentStack(JSContext*, class JS::MutableHandle<JSObject*>, uint32) Then there's one very similar to the nsINode case, perhaps invalid for the same reason (I didn't look at the interface): void PresShell::Paint(nsView*, nsRegion*, uint32) uint8 nsPresContext::MayHavePaintEventListenerInSubDocument() uint8 nsPresContext::MayHavePaintEventListener() nsPresContext.cpp:uint8 MayHavePaintEventListener(nsPIDOMWindowInner*) nsCOMPtr<T>::nsCOMPtr(nsQueryInterface) [with T = nsIInProcessContentFrameMessageManager] nsCOMPtr<T>::nsCOMPtr(nsQueryInterface) [with T = nsIInProcessContentFrameMessageManager] void nsCOMPtr<T>::assign_from_qi(nsQueryInterface, const nsIID&) [with T = nsIInProcessContentFrameMessageManager; nsIID = nsID] uint32 nsQueryInterface::operator(nsID*, void**)(const nsIID&, void**) const uint32 nsXPCWrappedJS::QueryInterface(nsID*, void**) uint32 nsXPCWrappedJSClass::DelegatedQueryInterface(nsXPCWrappedJS*, nsID*, void**) void XPCCallContext::XPCCallContext(uint32, JSContext*, class JS::Handle<JSObject*>, class JS::Handle<JSObject*>, class JS::Handle<jsid>, uint32, JS::Value*, JS::Value*) void XPCCallContext::XPCCallContext(uint32, JSContext*, class JS::Handle<JSObject*>, class JS::Handle<JSObject*>, class JS::Handle<jsid>, uint32, JS::Value*, JS::Value*) void JS_ReportError(JSContext*, int8*) The interfaces I saw here: nsINode, nsIInProcessContentFrameMessageManager, nsPIDOMWindowInner, nsPIWindowRoot, nsISelectionController, nsIImageLoadingContent, nsITextControlElement. And nsIContent for a different stack. But then I gave up and suppressed them all; I don't know how many more interfaces get used there. After those, I get something entirely different: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void mozilla::FrameLayerBuilder::DumpRetainedLayerTree(mozilla::layers::LayerManager*, class std::basic_stringstream<char>*, uint8) void mozilla::layers::LayerManager::Dump(class std::basic_stringstream<char>*, int8*, uint8, uint8) void mozilla::layers::Layer::Dump(class std::basic_stringstream<char>*, int8*, uint8, uint8) void mozilla::layers::ContentHostTexture::Dump(class std::basic_stringstream<char>*, int8*, uint8) void mozilla::layers::CompositableHost::DumpTextureHost(class std::basic_stringstream<char>*, mozilla::layers::TextureHost*) nsCString gfxUtils::GetAsDataURI(mozilla::gfx::SourceSurface*) gfxUtils.cpp:nsCString EncodeSourceSurfaceAsPNGURI(mozilla::gfx::SourceSurface*) gfxUtils.cpp:uint32 EncodeSourceSurfaceInternal(mozilla::gfx::SourceSurface*, nsACString_internal*, nsAString_internal*, uint32, _IO_FILE*, nsCString*) uint32 nsSyncStreamListener::Available(uint64*) uint32 nsSyncStreamListener::WaitForData() uint8 NS_ProcessNextEvent(nsIThread*, uint8) Next, I get more sync IPC from PLayerTransactionChild::SendForceComposite(), but eventually something different: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) uint8 nsDisplayTransform::ShouldPrerenderTransformedContent(nsDisplayListBuilder*, nsIFrame*) void mozilla::EffectCompositor::SetPerformanceWarning(nsIFrame*, int32, mozilla::AnimationPerformanceWarning*) void mozilla::dom::KeyframeEffectReadOnly::SetPerformanceWarning(int32, mozilla::AnimationPerformanceWarning*) uint8 mozilla::AnimationPerformanceWarning::ToLocalizedString(nsXPIDLString*) const uint32 nsContentUtils::GetLocalizedString(uint32, int8*, nsXPIDLString*) uint32 nsStringBundle::GetStringFromName(uint16*, uint16**) uint32 nsStringBundle::LoadProperties() uint32 NS_NewChannel(nsIChannel**, nsIURI*, nsIPrincipal*, uint32, uint32, nsILoadGroup*, nsIInterfaceRequestor*, uint32, nsIIOService*) uint32 NS_NewChannelInternal(nsIChannel**, nsIURI*, nsINode*, nsIPrincipal*, nsIPrincipal*, uint32, uint32, nsILoadGroup*, nsIInterfaceRequestor*, uint32, nsIIOService*) uint32 nsJSChannel::SetLoadGroup(nsILoadGroup*) uint32 mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, uint32) uint32 nsIncrementalStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, uint32) uint32 ScriptPrecompiler::OnStreamComplete(nsIIncrementalStreamLoader*, nsISupports*, uint32, uint32, uint8*) uint32 xpc::CreateSandboxObject(JSContext*, class JS::MutableHandle<JS::Value>, nsISupports*, xpc::SandboxOptions*) JSObject* js::NewProxyObject(JSContext*, js::BaseProxyHandler*, class JS::Handle<JS::Value>, JSObject*, js::ProxyOptions*) js::ProxyObject* js::ProxyObject::New(JSContext*, js::BaseProxyHandler*, class JS::Handle<JS::Value>, js::TaggedProto, js::ProxyOptions*) JSObject* js::NewObjectWithGivenTaggedProto(js::ExclusiveContext*, js::Class*, class JS::Handle<js::TaggedProto>, int32, uint32, uint32) jsobj.cpp:JSObject* NewObject(js::ExclusiveContext*, class JS::Handle<js::ObjectGroup*>, int32, uint32, uint32) Is this still impossible (sync IPC)?: void PresShell::Paint(nsView*, nsRegion*, uint32) uint8 mozilla::layers::ClientLayerManager::EndEmptyTransaction(uint32) void mozilla::layers::ClientLayerManager::ForwardTransaction(uint8) uint8 mozilla::layers::ShadowLayerForwarder::EndTransaction(class nsTArray<mozilla::layers::EditReply>*, class mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits>*, uint64, uint8, uint32, uint8, mozilla::TimeStamp*, uint8*) void mozilla::layers::Transaction::FallbackDestroyActors() uint8 mozilla::layers::TextureClient::DestroyFallback(mozilla::layers::PTextureChild*) uint8 mozilla::layers::PTextureChild::SendDestroySync() uint8 mozilla::ipc::MessageChannel::Send(IPC::Message*, IPC::Message*) Was this one already decided?: void PresShell::Paint(nsView*, nsRegion*, uint32) void nsPresContext::NotifyInvalidation(struct mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits>*, uint32) void nsPresContext::NotifyInvalidation(nsRect*, uint32) void nsRootPresContext::EnsureEventualDidPaintEvent() already_AddRefed<nsITimer> nsPresContext::CreateTimer(nsTimerCallbackFunc, uint32_t) probably more sync IPC that can't happen?: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) already_AddRefed<mozilla::layers::LayerManager> nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, uint32_t) void nsRootPresContext::CollectPluginGeometryUpdates(mozilla::layers::LayerManager*) nsPresContext.cpp:void PluginGetGeometryUpdate(class nsTHashtable<nsRefPtrHashKey<nsIContent> >*, class nsTArray<nsIWidget::Configuration>*) void nsPluginFrame::GetWidgetConfiguration(class nsTArray<nsIWidget::Configuration>*) void* mozilla::widget::PuppetWidget::GetNativeData(uint32) uint8 mozilla::dom::PBrowserChild::SendGetWidgetNativeData(uint64*) uint8 mozilla::ipc::MessageChannel::Send(IPC::Message*, IPC::Message*) Now this one is interesting: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void nsFieldSetFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) uint8 nsDisplayBackgroundImage::AppendBackgroundItemsToTop(nsDisplayListBuilder*, nsIFrame*, nsRect*, nsDisplayList*, uint8) void nsDisplayBackgroundImage::nsDisplayBackgroundImage(nsDisplayListBuilder*, nsIFrame*, uint32, nsRect*, nsStyleBackground*) void nsDisplayBackgroundImage::nsDisplayBackgroundImage(nsDisplayListBuilder*, nsIFrame*, uint32, nsRect*, nsStyleBackground*) nsBackgroundLayerState nsCSSRendering::PrepareImageLayer(nsPresContext*, nsIFrame*, uint32, nsRect*, nsRect*, nsStyleImageLayers::Layer*, uint8*, int8) uint8 nsImageRenderer::PrepareImage() virtual already_AddRefed<nsIURI> nsIContent::GetBaseURI(bool) const uint32 nsScriptSecurityManager::CheckLoadURIWithPrincipal(nsIPrincipal*, nsIURI*, uint32) uint32 nsScriptSecurityManager::ReportError(JSContext*, nsAString_internal*, nsIURI*, nsIURI*) and another fun one: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void nsMathMLSelectedFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) nsIFrame* nsMathMLsemanticsFrame::GetSelectedFrame() uint32 nsMathMLmoFrame::TransmitAutomaticData() void nsMathMLmoFrame::ProcessOperatorData() uint8 nsMathMLOperators::LookupOperator(nsString*, uint32, uint32*, float32*, float32*) nsMathMLOperators.cpp:uint32 InitOperatorGlobals() nsMathMLOperators.cpp:uint32 InitOperators() uint32 NS_LoadPersistentPropertiesFromURISpec(nsIPersistentProperties**, nsACString_internal*) Is this another impossible Element?: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void ViewportFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) void ViewportFrame::BuildDisplayListForTopLayer(nsDisplayListBuilder*, nsDisplayList*) virtual nsTArray<mozilla::dom::Element*> nsDocument::GetFullscreenStack() const nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = mozilla::dom::Element] nsCOMPtr<T>::nsCOMPtr(const nsCOMPtr_helper&) [with T = mozilla::dom::Element] void nsCOMPtr<T>::assign_from_helper(const nsCOMPtr_helper&, const nsIID&) [with T = mozilla::dom::Element; nsIID = nsID] Why do I feel this is not converging?: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void nsSubDocumentFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) uint8 nsSubDocumentFrame::PassPointerEventsToChildren() uint32 nsPermissionManager::TestPermissionFromPrincipal(nsIPrincipal*, int8*, uint32*) uint32 nsPermissionManager::CommonTestPermission(nsIPrincipal*, int8*, uint32*, uint8, uint8) nsPermissionManager::PermissionHashKey* nsPermissionManager::GetPermissionHashKey(nsIPrincipal*, uint32, uint8) uint32 nsPermissionManager::RemoveFromPrincipal(nsIPrincipal*, int8*) uint32 nsPermissionManager::AddInternal(nsIPrincipal*, nsCString*, uint32, int64, uint32, int64, int64, uint32, uint32, uint8) void nsPermissionManager::NotifyObserversWithPermission(nsIPrincipal*, nsCString*, uint32, uint32, int64, uint16*) void nsPermissionManager::NotifyObservers(nsIPermission*, uint16*) Then a whole different direction: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) already_AddRefed<mozilla::layers::LayerManager> nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, uint32_t) already_AddRefed<mozilla::layers::ContainerLayer> mozilla::FrameLayerBuilder::BuildContainerLayerFor(nsDisplayListBuilder*, mozilla::FrameLayerBuilder::LayerManager*, nsIFrame*, nsDisplayItem*, nsDisplayList*, const mozilla::ContainerLayerParameters&, const Matrix4x4*, uint32_t) void mozilla::ContainerState::ProcessDisplayItems(nsDisplayList*) virtual already_AddRefed<mozilla::layers::Layer> nsDisplayImage::BuildLayer(nsDisplayListBuilder*, nsDisplayImage::LayerManager*, const ContainerLayerParameters&) virtual already_AddRefed<mozilla::layers::ImageContainer> mozilla::image::RasterImage::GetImageContainer(mozilla::layers::LayerManager*, uint32_t) void mozilla::image::ProgressTracker::OnUnlockedDraw() CopyOnWrite.h:void (_ZN7mozilla5image15ProgressTracker14OnUnlockedDrawEv$void mozilla::image::ProgressTracker::OnUnlockedDraw()::__lambda13))) mozilla::image::CopyOnWrite<T>::Read(ReadFunc) const [with ReadFunc = mozilla::image::ProgressTracker::OnUnlockedDraw()::<lambda(const mozilla::image::ObserverTable*)>; T = mozilla::image::ObserverTable; decltype (aReader(static_cast<const T*>(nullptr))) = void] ProgressTracker.cpp:void mozilla::image::ProgressTracker::OnUnlockedDraw(mozilla::image::ObserverTable*)::<lambda(const mozilla::image::ObserverTable*)> ProgressTracker.cpp:void mozilla::image::ImageObserverNotifier<const mozilla::image::ObserverTable*>::operator()(Lambda) [with Lambda = mozilla::image::ProgressTracker::OnUnlockedDraw()::<lambda(const mozilla::image::ObserverTable*)>::<lambda(mozilla::image::IProgressObserver*)>] ProgressTracker.cpp:void mozilla::image::ProgressTracker::OnUnlockedDraw(mozilla::image::IProgressObserver*)::<lambda(const mozilla::image::ObserverTable*)>::<lambda(mozilla::image::IProgressObserver*)> void imgRequestProxy::Notify(int32, struct mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits>*) uint32 mozilla::dom::ImageDocument::Notify(imgIRequest*, int32, struct mozilla::gfx::IntRectTyped<mozilla::gfx::UnknownUnits>*) void nsContentUtils::AddScriptRunner(nsIRunnable*) next stack: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsDisplayListBuilder::EnterPresShell(nsIFrame*, uint8) void PresShell::UpdateCanvasBackground() nsPresShell.cpp:uint8 IsTransparentContainerElement(nsPresContext*) nsPIDOMWindowOuter* nsDocShell::GetWindow() uint32 nsDocShell::EnsureScriptEnvironment() already_AddRefed<nsGlobalWindow> NS_NewScriptGlobalObject(bool, bool) static already_AddRefed<nsGlobalWindow> nsGlobalWindow::Create(nsGlobalWindow*) void nsGlobalWindow::nsGlobalWindow(nsGlobalWindow*) void nsGlobalWindow::nsGlobalWindow(nsGlobalWindow*) void nsGlobalWindow::Freeze() void nsGlobalWindow::NotifyDOMWindowFrozen(nsGlobalWindow*) uint32 nsObserverService::NotifyObservers(nsISupports*, int8*, uint16*) might need some help identifying broad areas of danger: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) void nsIFrame::BuildDisplayListForStackingContext(nsDisplayListBuilder*, nsRect*, nsDisplayList*) void nsImageFrame::BuildDisplayList(nsDisplayListBuilder*, nsRect*, nsDisplayListSet*) nsImageMap* nsImageFrame::GetImageMap() mozilla::dom::Element* nsImageFrame::GetMapElement() const mozilla::dom::Element* nsDocument::FindImageMap(nsAString_internal*) uint32 nsContentList::Length(uint8) void nsContentList::BringSelfUpToDate(uint8) void nsDocument::FlushPendingNotifications(uint32) void HTMLContentSink::FlushPendingNotifications(uint32) void nsContentSink::StartLayout(uint8) uint32 PresShell::Initialize(int32, int32) void PresShell::ScheduleBeforeFirstPaint() void nsContentUtils::AddScriptRunner(nsIRunnable*) that doesn't require PresShell::Initialize, btw: void PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) uint8 PresShell::ProcessReflowCommands(uint8) void PresShell::UnsuppressAndInvalidate() void PresShell::ScheduleBeforeFirstPaint() void nsContentUtils::AddScriptRunner(nsIRunnable*) nor does it require ProcessReflowCommands: void PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) void nsBindingManager::ProcessAttachedQueue(uint32) void nsBindingManager::ProcessAttachedQueueInternal(uint32) void nsXBLBinding::ExecuteAttachedHandler() uint32 nsXBLPrototypeBinding::BindingAttached(nsIContent*) uint32 nsXBLProtoImplAnonymousMethod::Execute(nsIContent*, JSAddonId*) JSObject* JS::CloneFunctionObject(JSContext*, class JS::Handle<JSObject*>, class JS::AutoVectorRooter<JSObject*>*) Then there's font metrics: void PresShell::Paint(nsView*, nsRegion*, uint32) uint32 nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion*, uint32, uint8, uint32) already_AddRefed<mozilla::layers::LayerManager> nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, uint32_t) mozilla::layers::ScrollMetadata nsLayoutUtils::ComputeScrollMetadata(nsIFrame*, nsIFrame*, nsIContent*, nsIFrame*, mozilla::layers::Layer*, uint64, nsRect*, const class mozilla::Maybe<nsRect>*, uint8, mozilla::ContainerLayerParameters*) nsSize nsHTMLScrollFrame::GetLineScrollAmount() const nsSize mozilla::ScrollFrameHelper::GetLineScrollAmount() const int32 nsFontMetrics::AveCharWidth() gfxFont::Metrics* nsFontMetrics::GetMetrics() const gfxFont::Metrics* nsFontMetrics::GetMetrics(uint32) const gfxFont* gfxFontGroup::GetFirstValidFont(uint32) void gfxUserFontEntry::Load() void gfxUserFontEntry::LoadNextSrc() gfxFontEntry* gfxUserFontSet::UserFontCache::GetFont(nsIURI*, nsIPrincipal*, gfxUserFontEntry*, uint8) uint32 NS_NewChannel(nsIChannel**, nsIURI*, nsIPrincipal*, uint32, uint32, nsILoadGroup*, nsIInterfaceRequestor*, uint32, nsIIOService*) uint32 NS_NewChannelInternal(nsIChannel**, nsIURI*, nsINode*, nsIPrincipal*, nsIPrincipal*, uint32, uint32, nsILoadGroup*, nsIInterfaceRequestor*, uint32, nsIIOService*) uint32 nsJSChannel::SetLoadGroup(nsILoadGroup*) uint32 mozilla::net::nsLoadGroup::RemoveRequest(nsIRequest*, nsISupports*, uint32) uint32 nsIncrementalStreamLoader::OnStopRequest(nsIRequest*, nsISupports*, uint32) Ok, I have to pause for now. Just for myself, this is my current command (it's specific to the callgraph file I'm using, so if someone else wants to continue this, let me know and I'll resolve the numbers to function names): route from #203226 to #1189234 avoiding NS_DebugBreak and #110527 and #1081502 and #65741 and #345294 and #666379 and #2881 and #9398 and #929881 and #902851 and #820028 and #147818 and #342578 and #344175 and #120108 and #77583 and #916364 and #138790 and #413425 and #280809 and #349829 and #413424 and #5733 and #288583 and NS_DispatchToMainThread and NS_DispatchToCurrentThread and #71130 and #118441 and #118442 and #137859 and #319127 and #361805 and #347195 and #27967 and #102552 and #1089908 and #73342 and #361819 and #6345 and #66114 and #111230 and #7704 and #354022 and #345247 and #97675 and #110424 and #655518 and #76378 and #217518 and #217519 and #799569 and #147809 and #191317 and MessageChannel::Send and #134803 and #5189 and #413426 and #731692 and #118786 and #345784 and #23785

(In reply to Bill McCloskey (:billm) from comment #55) > Matt, can the following stack happen in real life? > > > void PresShell::Paint(nsView*, nsRegion*, uint32) > > uint8 mozilla::layers::ClientLayerManager::EndEmptyTransaction(uint32) > > void mozilla::layers::ClientLayerManager::ForwardTransaction(uint8) > > void nsRefreshDriver::RevokeTransactionId(uint64) > > void nsRefreshDriver::FinishedWaitingForTransaction() > > void nsRefreshDriver::DoRefresh() > > If we can run the refresh driver from painting, then we can run JS, which > would be bad. If this can happen, is there a way to disable it from these > paints that happen inside JS? Nope, it can't happen. RevokeTransactionId asserts that mSkippedPaints is false before calling FinishedWaitingForTransaction, FinishedWaitingForTransaction only called DoRefresh if mSkippedPaints is true.

Pushed by wmccloskey@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/7b09d25fba12 Stop using sync image decoding for tab switch paint (r=mattwoodrow)

I looked at the various regressions hitting MOZ_RELEASE_ASSERT(rt->allowGCBarriers()) in the last 24 hours. Out of 347, 207 contained FontFaceSet in the stack (hopefully fixed by bug 1309867) and of those that didn't, 193 contained SendGetWidgetNativeData (hopefully fixed by bug 1309958). So hopefully now that those are fixed they will not affect Nightly stability. I'll file some bugs for the remaining crashes I see.

My arithmetic might be a little mangled there, but my point was really just that almost all of them (all but 14) should be fixed by the two bugs that have had patches landed recently.

In the release notes of 51 with "An even faster E10s! Tab Switching is better!" as wording.