stylo: Assertion failure: mOwner == ExpectedOwnerForChild(aFrame) (Missed some frame in the hierarchy?)

RESOLVED FIXED in Firefox 57

Status

()

defect
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: truber, Assigned: bzbarsky)

Tracking

(Blocks 2 bugs, {assertion, testcase})

Trunk
mozilla58
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +
qe-verify -

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox56 unaffected, firefox57 fixed, firefox58 fixed)

Details

Attachments

(2 attachments)

Posted file testcase.html
The attached testcase causes an assertion in m-c rev 20170922-14db7c0bcf9a

Assertion failure: mOwner == ExpectedOwnerForChild(aFrame) (Missed some frame in the hierarchy?), at /builds/worker/workspace/build/src/layout/base/ServoRestyleManager.cpp:122
#0: mozilla::ServoRestyleState::ChangesHandledFor, at layout/base/ServoRestyleManager.cpp:121
#1: mozilla::ServoRestyleManager::TextPostTraversalState::ComputeHintIfNeeded, at layout/base/ServoRestyleManager.cpp:495
#2: mozilla::ServoRestyleManager::ProcessPostTraversalForText, at layout/base/ServoRestyleManager.cpp:998
#3: mozilla::ServoRestyleManager::ProcessPostTraversal, at layout/base/ServoRestyleManager.cpp:925
#4: mozilla::ServoRestyleManager::ProcessPostTraversal, at layout/base/ServoRestyleManager.cpp:920
#5: mozilla::ServoRestyleManager::DoProcessPendingRestyles, at layout/base/ServoRestyleManager.cpp:1121
#6: mozilla::PresShell::DoFlushPendingNotifications, at layout/base/PresShell.cpp:4170
#7: nsRefreshDriver::Tick, at layout/base/nsRefreshDriver.cpp:1921
#8: mozilla::RefreshDriverTimer::TickRefreshDrivers, at layout/base/nsRefreshDriver.cpp:307
#9: mozilla::RefreshDriverTimer::Tick, at layout/base/nsRefreshDriver.cpp:329
#10: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver, at layout/base/nsRefreshDriver.cpp:770
#11: mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync, at layout/base/nsRefreshDriver.cpp:584
#12: mozilla::layout::VsyncChild::RecvNotify, at layout/ipc/VsyncChild.cpp:67
#13: mozilla::layout::PVsyncChild::OnMessageReceived, at 9a6479c28b0f57ad4e6d1aec1c5258678abc48225caeaa2a25f8a6893b83595687422ba915c00ef9e645c78dda14478aa8e2f891ae54d4764ccdba97f65bc47d/ipc/ipdl/PVsyncChild.cpp:155
#14: mozilla::ipc::MessageChannel::DispatchAsyncMessage, at ipc/glue/MessageChannel.cpp:2115
#15: mozilla::ipc::MessageChannel::DispatchMessage, at ipc/glue/MessageChannel.cpp:2045
#16: mozilla::ipc::MessageChannel::RunMessage, at ipc/glue/MessageChannel.cpp:1891
#17: mozilla::ipc::MessageChannel::MessageTask::Run, at ipc/glue/MessageChannel.cpp:1924
#18: nsThread::ProcessNextEvent, at xpcom/threads/nsThread.cpp:1039
#19: NS_ProcessNextEvent, at xpcom/threads/nsThreadUtils.cpp:521
#20: mozilla::ipc::MessagePump::Run, at ipc/glue/MessagePump.cpp:125
#21: MessageLoop::RunInternal, at ipc/chromium/src/base/message_loop.cc:326
#22: MessageLoop::Run, at ipc/chromium/src/base/message_loop.cc:319
#23: nsBaseAppShell::Run, at widget/nsBaseAppShell.cpp:158
#24: XRE_RunAppShell, at toolkit/xre/nsEmbedFunctions.cpp:880
#25: mozilla::ipc::MessagePumpForChildProcess::Run, at ipc/glue/MessagePump.cpp:269
#26: MessageLoop::RunInternal, at ipc/chromium/src/base/message_loop.cc:326
#27: MessageLoop::Run, at ipc/chromium/src/base/message_loop.cc:319
#28: XRE_InitChildProcess, at toolkit/xre/nsEmbedFunctions.cpp:705
#29: content_process_main, at ipc/contentproc/plugin-container.cpp:63
#30: main, at browser/app/nsBrowserApp.cpp:285
#31: libc-2.26.so+0x20f6a
#32: MOZ_ReportAssertionFailure, at mfbt/Assertions.h:165
Flags: in-testsuite?
Looks similar to bug 1391736?
Very likely.  I will patch that one and then check whether it helps here.
Depends on: 1391736
Priority: -- → P3
Assignee: nobody → bzbarsky
The expected owner is the DOM parent.  The first-letter then does some reparenting of the text style later.
Attachment #8914480 - Flags: review?(emilio)
Comment on attachment 8914480 [details] [diff] [review]
ExpectedOwnerForChild should not return a first-letter frame for a text child

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

r=me
Attachment #8914480 - Flags: review?(emilio) → review+

Comment 5

2 years ago
Pushed by bzbarsky@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/06d4a47016eb
ExpectedOwnerForChild should not return a first-letter frame for a text child.  r=emilio
https://hg.mozilla.org/mozilla-central/rev/06d4a47016eb
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
Flags: in-testsuite? → in-testsuite+
Comment on attachment 8914480 [details] [diff] [review]
ExpectedOwnerForChild should not return a first-letter frame for a text child

Approval Request Comment
[Feature/Bug causing the regression]: Stylo
[User impact if declined]: None, this is debug-only code.  But this is
   interfering with fuzzing.
[Is this code covered by automated tests?]: Yes.
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: Debug-only code.
[String changes made/needed]: None.
Attachment #8914480 - Flags: approval-mozilla-beta?
Comment on attachment 8914480 [details] [diff] [review]
ExpectedOwnerForChild should not return a first-letter frame for a text child

Stylo related, Beta57+
Attachment #8914480 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Boris Zbarsky [:bz] (still digging out from vacation mail) from comment #7)
> [Is this code covered by automated tests?]: Yes.
> [Has the fix been verified in Nightly?]: Yes.
> [Needs manual test from QE? If yes, steps to reproduce]: No.

Setting qe-verify- based on Boris's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
See Also: → 1410226
You need to log in before you can comment on or make changes to this bug.