promiseDocumentFlushed callbacks will fire even if subframes might need flushes

NEW
Assigned to

Status

()

defect
P2
normal
4 months ago
a month ago

People

(Reporter: mconley, Assigned: mconley, NeedInfo)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

Comment hidden (obsolete)
(Assignee)

Comment 1

4 months ago
We should probably disallow .top.promiseDocumentFlushed somehow.
(Assignee)

Comment 2

4 months ago
(In reply to Mike Conley (:mconley) (:⚙️) (Away from Bugmail Dec 24 - Jan 7) from comment #0)
> Bug 1448096 landed a patch that uses promiseDocumentFlushed to try to avoid
> sync layout flushes while resizing.
> 
> I reviewed that patch, and it landed, and I missed something pretty big -
> promiseDocumentFlushed isn't designed to work within subframes. In fact, the
> patch itself makes reference to that here:
> 
> https://searchfox.org/mozilla-central/rev/
> 413dd3a1fab3446749f3690d93ee17e3196f8c37/devtools/client/inspector/inspector.
> js#624-625
> 
> so it reaches up to the window.top and uses its promiseDocumentFlushed
> instead.
> 

So, this is accurate, but the last part of comment 0 where I quote bug 1441173 - I kinda jumped the gun there. That bug talks about how if we call promiseDocumentFlushed on the subframe, it won't know about parent frames that might need flushing.

I filed this originally because I was getting DevTools failures when trying to land bug 1441168. Using rr, those failures seem to be because promiseDocumentFlushed is running even though one of the subframes of the main window _does_ need a flush.

That sounds like a bug in promiseDocumentFlushed.

So maybe the bigger problem is - how do we make promiseDocumentFlushed resilient to this case?
Component: Inspector → DOM
Product: DevTools → Core
Summary: Inspector isn't using promiseDocumentFlushed correctly → promiseDocumentFlushed callbacks will fire even if subframes have mDelayedResize's pending
Priority: -- → P2
(Assignee)

Updated

4 months ago
Summary: promiseDocumentFlushed callbacks will fire even if subframes have mDelayedResize's pending → promiseDocumentFlushed callbacks will fire even if subframes might need flushes
(Assignee)

Updated

4 months ago
Assignee: nobody → mconley
(Assignee)

Comment 4

4 months ago
Depends on D16092
(Assignee)

Updated

3 months ago
Blocks: 1441168

Comment 5

3 months ago
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c683fbe0b984
promiseDocumentFlushed should ensure no subframes need flushing before running the callback. r=bzbarsky
https://hg.mozilla.org/integration/autoland/rev/4fae3add610f
Regression test. r=Gijs
Component: DOM → DOM: Core & HTML
Product: Core → Core
You need to log in before you can comment on or make changes to this bug.