Closed Bug 1386862 Opened 2 years ago Closed 2 years ago

Stylo: DevTools test triggers "Assertion failure: !data->mSheet->IsModified() (should not get marked modified during parsing)"

Categories

(Core :: CSS Parsing and Computation, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox-esr52 --- unaffected
firefox55 --- unaffected
firefox56 --- disabled
firefox57 --- fixed

People

(Reporter: jryans, Assigned: bradwerth)

References

Details

(Keywords: assertion)

Attachments

(1 file)

DevTools test devtools/client/inspector/rules/test/browser_rules_edit-selector_12.js triggers[1] an assertion on Stylo debug builds:

Assertion failure: !data->mSheet->IsModified() (should not get marked modified during parsing), at /home/worker/workspace/build/src/layout/style/Loader.cpp:1905
#01: mozilla::css::Loader::SheetComplete [xpcom/ds/nsTArray.h:398]
#02: mozilla::css::Loader::ParseSheet [layout/style/Loader.cpp:1811]
#03: mozilla::css::SheetLoadData::OnStreamComplete [layout/style/Loader.cpp:1010]
#04: nsUnicharStreamLoader::OnStopRequest [netwerk/base/nsUnicharStreamLoader.cpp:99]
#05: mozilla::net::HttpChannelChild::DoOnStopRequest [netwerk/protocol/http/HttpChannelChild.cpp:1174]
#06: mozilla::net::HttpChannelChild::OnStopRequest [netwerk/protocol/http/HttpChannelChild.cpp:1073]
#07: mozilla::net::ChannelEventQueue::FlushQueue [netwerk/ipc/ChannelEventQueue.cpp:65]
#08: mozilla::net::ChannelEventQueue::ResumeInternal()::CompleteResumeRunnable::Run()
#09: mozilla::SchedulerGroup::Runnable::Run [xpcom/threads/SchedulerGroup.cpp:356]
#10: nsThread::ProcessNextEvent [mfbt/ScopeExit.h:111]
#11: NS_ProcessNextEvent [xpcom/threads/nsThreadUtils.cpp:480]
#12: mozilla::ipc::MessagePump::Run [ipc/glue/MessagePump.cpp:98]
#13: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:327]
#14: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:299]
#15: nsBaseAppShell::Run [widget/nsBaseAppShell.cpp:158]
#16: XRE_RunAppShell [toolkit/xre/nsEmbedFunctions.cpp:882]
#17: mozilla::ipc::MessagePumpForChildProcess::Run [ipc/glue/MessagePump.cpp:270]
#18: MessageLoop::RunInternal [ipc/chromium/src/base/message_loop.cc:327]
#19: MessageLoop::Run [ipc/chromium/src/base/message_loop.cc:299]
#20: XRE_InitChildProcess [toolkit/xre/nsEmbedFunctions.cpp:703]
#21: content_process_main [ipc/contentproc/plugin-container.cpp:66]
#22: main [browser/app/nsBrowserApp.cpp:288]
#23: libc.so.6 + 0x20830
#24: _start

[1]: https://treeherder.mozilla.org/logviewer.html#?job_id=120109123&repo=try&lineNumber=14805
This is the same issue as bug 1387933.

The problem here is that, when user changes selector via devtools, devtools reparses the stylesheet which includes an @import rule. On Gecko, this reparsing would reuse the existing stylesheet of the @import rule, while on Stylo, we would create a new stylesheet and load it again. And since devtools progressively calls EnsureUniqueInnerOnCSSSheets again, we would mark the new stylesheet dirty before it gets load.

Note that I think this is actually an existing issue that if devtools is opened before all stylesheets load, we can hit this assertion in both Gecko and Servo.
Depends on: 1387933
Brad, is this still an issue? If so, can you have a look?
Flags: needinfo?(bwerth)
I'll confirm it.
Flags: needinfo?(bwerth)
Assignee: nobody → bwerth
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WORKSFORME
It appears this test is currently skipped:

http://searchfox.org/mozilla-central/rev/cd82cacec2cf734768827ff85ba2dba90a534c5e/devtools/client/inspector/rules/test/browser.ini#151

If it's working, we should remove the skip annotation.
Flags: needinfo?(bwerth)
Status: RESOLVED → REOPENED
Flags: needinfo?(bwerth)
Resolution: WORKSFORME → ---
Attachment #8902441 - Flags: review?(jryans)
Comment on attachment 8902441 [details]
Bug 1386862: stylo: Turn on the devtools/client/inspector/rules/test/browser_rules_edit-selector_12.js test.

https://reviewboard.mozilla.org/r/174032/#review180100
Attachment #8902441 - Flags: review+
Comment on attachment 8902441 [details]
Bug 1386862: stylo: Turn on the devtools/client/inspector/rules/test/browser_rules_edit-selector_12.js test.

https://reviewboard.mozilla.org/r/174032/#review180114
Attachment #8902441 - Flags: review?(jryans) → review+
Pushed by bwerth@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4f95bb8143aa
stylo: Turn on the devtools/client/inspector/rules/test/browser_rules_edit-selector_12.js test. r=bholley,jryans
https://hg.mozilla.org/mozilla-central/rev/4f95bb8143aa
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.