1611038 - Assertion failure: !mResizeEventFlushObservers.Contains(aPresShell) && !mDelayedResizeEventFlushObservers.Contains(aPresShell) (Double-adding resize event flush observer), at nsRefreshDriver.h:197

Status: RESOLVED FIXED

(Core :: Layout, defect, P2)

Description

[Hiroyuki Ikezoe (:hiro)]

I've seen this assertion occasionally on my local Android x86_64 emulator when I enable the dynamic toolbar on reference-browser.

What I am currently guessing is that firing visual viewport resize events (bug 1586986) somehow ends up causing recursive resize events, but when I was working on bug 1586986 I had never seen this assertion so presumably there may be some changes affecting the behavior since then.

Anyways, I haven't found a reliable way to reproduce this assertion yet.

Comment 1

[Hiroyuki Ikezoe (:hiro)]

So the culprit is an AddResizeEventFlushObserver call in nsPresContext::UpdateDynamicToolbar. It's not recursive at all, in fact setVerticalClipping calls from Android application are not aligned with our refresh driver's tick. Anyways, we need to check mResizeEventPending just like what we do in PresShell::ResizeReflowIgnoreOverride.

Comment 2

[Hiroyuki Ikezoe (:hiro)]

Emilio, do you recall the reason why setting mResizeEventPending to true is outside of if (MOZ_LIKELY(!mDocument->GetBFCacheEntry())) block? I'd like to introduce a utility function to call AddResizeEventFlushObserver if applicable, it will be something like this;

void PresShell::MaybeAddResizeEventFlushObserver() {                            
  if (!mIsDestroying && !mResizeEventPending &&                                 
      MOZ_LIKELY(!mDocument->GetBFCacheEntry())) {                              
    mResizeEventPending = true;                                                 
    mPresContext->RefreshDriver()->AddResizeEventFlushObserver(this);           
  }                                                                             
}

But it's entirely unclear whether there is a case that we need to set mResizeEventPending to true even if we don't observe refresh observer in the case of the document is not in the BF cache?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

I think it'd be just fine (even better) to put it inside.

Comment 4

[Hiroyuki Ikezoe (:hiro)]

Great! Thank you!

Comment 5

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1611038 - Introduce PresShell::AddResizeEventFlushObserverIfNeeded. r?emilio

Comment 6

[Hiroyuki Ikezoe (:hiro)]

Attachment: Bug 1611038 - Use PresShell::AddResizeEventFlushObserverIfNeeded in nsPresContext::UpdateDynamicToolbarOffset. r?botond

It's possible that UpdateDynamicTollbarOffset is called repeatedly before we
actually fire a window resize event.

Depends on D60811

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cb8aa19a013c
Introduce PresShell::AddResizeEventFlushObserverIfNeeded. r=emilio
https://hg.mozilla.org/integration/autoland/rev/e1d84b595756
Use PresShell::AddResizeEventFlushObserverIfNeeded in nsPresContext::UpdateDynamicToolbarOffset. r=botond

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/cb8aa19a013c
https://hg.mozilla.org/mozilla-central/rev/e1d84b595756