Last Comment Bug 766792 - "ASSERTION: Somehow there's stuff in the op queue" in nsHtml5TreeOpExecutor::~nsHtml5TreeOpExecutor
: "ASSERTION: Somehow there's stuff in the op queue" in nsHtml5TreeOpExecutor::...
Status: RESOLVED FIXED
: assertion, testcase
Product: Core
Classification: Components
Component: HTML: Parser (show other bugs)
: Trunk
: x86_64 Mac OS X
-- normal (vote)
: ---
Assigned To: Olli Pettay [:smaug] (pto-ish for couple of days)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 594645
  Show dependency treegraph
 
Reported: 2012-06-20 17:09 PDT by Jesse Ruderman
Modified: 2012-06-21 07:51 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
testcase (370 bytes, text/html)
2012-06-20 17:09 PDT, Jesse Ruderman
no flags Details
stack trace (7.89 KB, text/plain)
2012-06-20 17:09 PDT, Jesse Ruderman
no flags Details
patch (895 bytes, patch)
2012-06-21 05:17 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
no flags Details | Diff | Splinter Review
patch (1.08 KB, patch)
2012-06-21 05:53 PDT, Olli Pettay [:smaug] (pto-ish for couple of days)
hsivonen: review+
Details | Diff | Splinter Review

Description User image Jesse Ruderman 2012-06-20 17:09:05 PDT
Created attachment 635139 [details]
testcase

With:

  user_pref("browser.tabs.loadDivertedInBackground", true);
  user_pref("dom.disable_open_during_load", false);
  user_pref("nglayout.debug.disable_xul_cache", true);
  user_pref("security.fileuri.strict_origin_policy", false);

The testcase triggers:

###!!! ASSERTION: Somehow there's stuff in the op queue.: 'mOpQueue.IsEmpty()', file parser/html/nsHtml5TreeOpExecutor.cpp, line 80

Related to bug 734015?
Comment 1 User image Jesse Ruderman 2012-06-20 17:09:29 PDT
Created attachment 635140 [details]
stack trace
Comment 2 User image Henri Sivonen (:hsivonen) 2012-06-20 22:57:58 PDT
(In reply to Jesse Ruderman from comment #0)
> Related to bug 734015?

Quite likely, actually. Either we could just delete the assertion, which could mask other bugs or we could explicitly clear the queue before the assertion if the executor has been backgrounded.
Comment 3 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-06-21 03:12:06 PDT
I can understand that bug 734015 ca make such assertion easier to fire, but how could it cause it?
Comment 4 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-06-21 04:00:49 PDT
I'll investigate this.
Comment 5 User image Henri Sivonen (:hsivonen) 2012-06-21 04:06:00 PDT
My expectation is that what happens is this: previously, the parser thread probably scheduled to rambles on the main thread: first a runnable that caused a flush and then a runnable that caused the executor to be released. Now the flash gets deferred, so the queue won't be empty by the time the executor is released. This is harmless, if this only happens when the parser has been terminated anyway, which I expect to be the case.
Comment 6 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-06-21 05:17:29 PDT
Created attachment 635265 [details] [diff] [review]
patch

Could we do this. Other option is to flush in nsHtml5ExecutorFlusher dtor
which ends up to
if (!mParser) {
  mOpQueue.Clear();
}
Comment 7 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-06-21 05:53:11 PDT
Created attachment 635275 [details] [diff] [review]
patch
Comment 8 User image Henri Sivonen (:hsivonen) 2012-06-21 06:12:24 PDT
Comment on attachment 635275 [details] [diff] [review]
patch

Thanks.
Comment 9 User image Olli Pettay [:smaug] (pto-ish for couple of days) 2012-06-21 07:51:31 PDT
https://hg.mozilla.org/mozilla-central/rev/c83282305cb9

Note You need to log in before you can comment on or make changes to this bug.