Open
Bug 1516022
Opened 6 years ago
Updated 2 years ago
promiseDocumentFlushed callbacks will fire even if subframes might need flushes
Categories
(Core :: DOM: Core & HTML, defect, P2)
Core
DOM: Core & HTML
Tracking
()
NEW
People
(Reporter: mconley, Assigned: mconley, NeedInfo)
References
Details
Attachments
(2 files)
Comment hidden (obsolete) |
Assignee | ||
Comment 1•6 years ago
|
||
We should probably disallow .top.promiseDocumentFlushed somehow.
Assignee | ||
Comment 2•6 years 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
Updated•6 years ago
|
Priority: -- → P2
Assignee | ||
Comment 3•6 years ago
|
||
Depends on D14101
Assignee | ||
Updated•6 years 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•6 years ago
|
Assignee: nobody → mconley
Assignee | ||
Comment 4•6 years ago
|
||
Depends on D16092
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
Comment 6•6 years ago
|
||
Backed out for mochitest failures on browser_editcontrols_update. There is also this assertion starting from the above push:
Assertion failure: parsedPolicyStr.Find("default-src") >= 0 (about: page must contain a CSP including default-src), at /builds/worker/workspace/build/src/dom/base/Document.cpp:4844
Failure logs:
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=222273547&repo=autoland&lineNumber=6911
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=222273055&repo=autoland&lineNumber=12565
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=222275200&repo=autoland&lineNumber=17955
Backkout link: https://hg.mozilla.org/integration/autoland/rev/0b3a6c7175ca027b27db5943f178f36c2c1e0672
Flags: needinfo?(mconley)
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
Updated•2 years ago
|
Severity: normal → S3
You need to log in
before you can comment on or make changes to this bug.