Closed
Bug 997506
Opened 10 years ago
Closed 10 years ago
Intermittent 9 assertions "Why did this not get handled while processing mRestyleRoots?" from various tests involving changes to font preferences
Categories
(Core :: CSS Parsing and Computation, defect)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
mozilla31
People
(Reporter: dbaron, Assigned: dbaron)
References
Details
(Keywords: assertion, intermittent-failure)
Attachments
(1 file)
4.45 KB,
patch
|
tnikkel
:
review+
|
Details | Diff | Splinter Review |
We just had a new intermittent failure show up on multiple trees at the same time, in multiple tests (all related to changing font preferences). The failure shows up in the form of 9 assertions of this form: 16:05:31 INFO - ^G[2489] ###!!! ASSERTION: Why did this not get handled while processing mRestyleRoots?: '!element->HasFlag(collector->tracker->RootBit()) || (element->GetFlattenedTreeParent() && (!element->GetFlattenedTreeParent()->GetPrimaryFrame()|| element->GetFlattenedTreeParent()->GetPrimaryFrame()->IsLeaf())) || (aData.mChangeHint & nsChangeHint_ReconstructFrame)', file /builds/slave/fx-team-l64-d-0000000000000000/build/layout/base/RestyleTracker.cpp, line 87 16:05:31 INFO - PL_DHashTableEnumerate(PLDHashTable*, PLDHashOperator (*)(PLDHashTable*, PLDHashEntryHdr*, unsigned int, void*), void*) [xpcom/glue/pldhash.cpp:663] 16:05:31 INFO - nsBaseHashtable<nsISupportsHashKey, mozilla::RestyleTracker::RestyleData, mozilla::RestyleTracker::RestyleData>::Enumerate(PLDHashOperator (*)(nsISupports*, mozilla::RestyleTracker::RestyleData&, void*), void*) [obj-firefox/dist/include/nsBaseHashtable.h:209] 16:05:31 INFO - mozilla::RestyleTracker::DoProcessRestyles() [obj-firefox/dist/include/nsTHashtable.h:238] 16:05:31 INFO - mozilla::RestyleManager::ProcessPendingRestyles() [layout/base/RestyleManager.cpp:1425] 16:05:31 INFO - mozilla::RestyleManager::RebuildAllStyleData(nsChangeHint) [obj-firefox/dist/include/nsContentUtils.h:2245] 16:05:31 INFO - nsPresContext::UpdateAfterPreferencesChanged() [layout/base/nsPresContext.cpp:936] 16:05:31 INFO - nsTimerImpl::Fire() [xpcom/threads/nsTimerImpl.cpp:552] 16:05:31 INFO - nsTimerEvent::Run() [xpcom/threads/nsTimerImpl.cpp:638] 16:05:31 INFO - nsThread::ProcessNextEvent(bool, bool*) [xpcom/threads/nsThread.cpp:699] 16:05:31 INFO - NS_ProcessNextEvent(nsIThread*, bool) [xpcom/glue/nsThreadUtils.cpp:263] 16:05:31 INFO - mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) [ipc/glue/MessagePump.cpp:96] 16:05:31 INFO - MessageLoop::RunInternal() [ipc/chromium/src/base/message_loop.cc:227] 16:05:31 INFO - MessageLoop::Run() [ipc/chromium/src/base/message_loop.cc:491] 16:05:31 INFO - nsBaseAppShell::Run() [widget/xpwidgets/nsBaseAppShell.cpp:166] 16:05:31 INFO - nsAppStartup::Run() [toolkit/components/startup/nsAppStartup.cpp:278] 16:05:31 INFO - XREMain::XRE_mainRun() [toolkit/xre/nsAppRunner.cpp:4020] 16:05:31 INFO - XREMain::XRE_main(int, char**, nsXREAppData const*) [toolkit/xre/nsAppRunner.cpp:4088] 16:05:31 INFO - XRE_main [toolkit/xre/nsAppRunner.cpp:4301] 16:05:31 INFO - do_main [browser/app/nsBrowserApp.cpp:282] 16:05:31 INFO - main [browser/app/nsBrowserApp.cpp:645] So far it has been seen on these pushes: https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=95e6b5e31ce1 (XP debug crashtest) https://tbpl.mozilla.org/?tree=Mozilla-Inbound&rev=0d37b75f8cd3 (Linux debug mochitest-5) https://tbpl.mozilla.org/?branch=fx-team&rev=7b4cfd722bcd (Linux debug mochitest-4) Still investigating the cause, and unclear why it would suddenly show up on 2 branches at once.
Assignee | ||
Comment 1•10 years ago
|
||
last link should be *64-bit* Linux debug mochitest-4
Updated•10 years ago
|
Summary: intermittnt 9 assertions "Why did this not get handled while processing mRestyleRoots?" from various tests involving changes to font preferences → Intermittent 9 assertions "Why did this not get handled while processing mRestyleRoots?" from various tests involving changes to font preferences
Assignee | ||
Comment 2•10 years ago
|
||
RyanVM was eyeing bug 984226, but that's not on fx-team. I'm a little suspicious of bug 983465. I also should look to see if there are any editor-ish changes since this reminds me rather a lot of bug 613816 -- in fact, maybe the pattern of 9 assertions is related to 9 editor-inserted nodes?
Comment 3•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=37970356&tree=Mozilla-Inbound
Comment 4•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=37971325&tree=Mozilla-Inbound
Keywords: assertion,
intermittent-failure
Comment 5•10 years ago
|
||
For a fun twist on this - got 17 expecting 8. https://tbpl.mozilla.org/php/getParsedLog.php?id=37971931&tree=Fx-Team
Comment 6•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=37972517&tree=Mozilla-Inbound
Comment 8•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=37984110&tree=Mozilla-Inbound
Comment 10•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38031196&tree=Mozilla-Inbound
Comment 12•10 years ago
|
||
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-7) from comment #2) > I also should look to see if there are any editor-ish changes since this > reminds me rather a lot of bug 613816 -- in fact, maybe the pattern of 9 > assertions is related to 9 editor-inserted nodes? I don't think this is related to bug 613816, because there are no editor resize handle related frames on the stack.
Assignee | ||
Comment 13•10 years ago
|
||
Why would they be on the stack? There aren't any frames on the stack at all.
Comment 14•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38046694&tree=Mozilla-Inbound
Comment 15•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38047064&tree=Mozilla-Inbound
Comment 16•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38051868&tree=Fx-Team https://tbpl.mozilla.org/php/getParsedLog.php?id=38051726&tree=Fx-Team
Comment 17•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38062665&tree=Mozilla-Central
Comment 18•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38064934&tree=Mozilla-Inbound
Comment 19•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38070906&tree=Mozilla-Inbound
Comment 21•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38088811&tree=B2g-Inbound Can we consider making this assertion a warning while this is being investigated?
Comment 22•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38093122&tree=B2g-Inbound
Comment 23•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38094021&tree=Mozilla-Inbound
Comment 24•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38098562&tree=Mozilla-Inbound
Comment 27•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38110036&tree=Mozilla-Inbound
Comment 28•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38117062&tree=Mozilla-Inbound
Comment 30•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38123982&tree=Mozilla-Inbound
Comment 31•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38130514&tree=Mozilla-Inbound
Comment 32•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38143242&tree=Mozilla-Central
Comment 34•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38149350&tree=Mozilla-Inbound https://tbpl.mozilla.org/php/getParsedLog.php?id=38149311&tree=Mozilla-Inbound
Comment 36•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38158988&tree=B2g-Inbound
Comment 37•10 years ago
|
||
(In reply to David Baron [:dbaron] (needinfo? me) (UTC-7) from comment #2) > I'm a little suspicious of bug 983465. https://tbpl.mozilla.org/?tree=Try&rev=7fdae467289b with it backed out still hit this plenty often.
Comment 38•10 years ago
|
||
(In reply to comment #13) > Why would they be on the stack? There aren't any frames on the stack at all. By frames, I meant stack frames.
Comment 39•10 years ago
|
||
https://tbpl.mozilla.org/?rev=ba0fc8bc4aa0&tree=Fx-Team says it was already on fx-team prior to that last merge from m-c before the (previously) first instance there.
Comment 40•10 years ago
|
||
Nine assertions. How many tiles are there on the new tab page? Another hundred retriggers will make me feel better, but so far I'm feeling pretty confident in https://tbpl.mozilla.org/?tree=Fx-Team&fromchange=b1470828166b&tochange=cd8697531517&jobname=Ubuntu%20VM%2012.04%20x64%20fx-team%20debug%20test%20crashtest saying it's either bug 991210 or bug 991111 (and if that's saying that we're loading the new tab page between each crashtest load, umm, we might want to not do that).
Comment 41•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=46c730bfd7dd is bug 991210 backed out (bug 991111 doesn't want to be backed out without it being someone who understands it manually resolving the way that no part of it cleanly backs out).
Comment 42•10 years ago
|
||
The main change in bug 991210 there was to have the html for the grid of 9 sites/tiles added to the page as it was loading (and later add site data) as opposed to waiting for an event for when site data was ready to both create the html and populate with site data.
Blocks: tiles-dev
Updated•10 years ago
|
Flags: firefox-backlog?
Comment 43•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38189819&tree=Mozilla-Inbound
Comment 44•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38193104&tree=Mozilla-Inbound
Comment 45•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38193104&tree=Mozilla-Inbound
Updated•10 years ago
|
Flags: firefox-backlog? → firefox-backlog-
Comment 50•10 years ago
|
||
C
Comment 51•10 years ago
|
||
Nice. https://tbpl.mozilla.org/php/getParsedLog.php?id=38217680&tree=Mozilla-Inbound
Comment 52•10 years ago
|
||
And given how much we dislike manually starring, and how we know exactly what to back out to not have to, it would be good to see some furious action on this bug.
Comment 53•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38221381&tree=Mozilla-Inbound
Comment 54•10 years ago
|
||
(In reply to Phil Ringnalda (:philor) from comment #41) > https://tbpl.mozilla.org/?tree=Try&rev=46c730bfd7dd Even with the back out, the full log for crashtest has ASSERTION: Why did this not get handled while processing mRestyleRoots? -- Just not the 9 from new tab tiles. I also see that bug 870100 disabled some newtab thumbnail related pref for reftests. Should we do the same for the full newtab page? https://hg.mozilla.org/try/rev/46848f558638 https://tbpl.mozilla.org/?tree=Try&rev=c62c90f715c3
Comment 55•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38237969&tree=Mozilla-Central
Comment 56•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38237969&tree=Mozilla-Central
Comment 57•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38245243&tree=Mozilla-Inbound
Comment 58•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38248452&tree=B2g-Inbound
Comment 59•10 years ago
|
||
What are we actually doing that's causing newtab to do stuff? I was assuming this meant the "Loading a blank page" thing was loading the newtab page between every test, and resetting font prefs slowed that down enough that it started to render things, but that "blank page" is actually "data:text/html;charset=UTF-8,%3C%21%2D%2DCLEAR%2D%2D%3E". (And we also see it, though not as often, in mochitest-4 and -5, so we'd need to stop doing the same thing in mochitests, too.)
Comment 60•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38271782&full=1&branch=mozilla-central
Comment 62•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38290670&tree=Mozilla-Inbound
Comment 63•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38290344&tree=Mozilla-Central
Comment 64•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38299445&tree=B2g-Inbound
Assignee | ||
Comment 65•10 years ago
|
||
The reason changing font prefs is relevant is that changing font prefs is what triggers the restyle operation that we're asserting during. I'm trying to understand the actual condition of the assertion, though. The underlying problem in bug 613816 is that a parent frame has child frames that it doesn't know about; that seems like a problem we ought to fix by ensuring that those frames are in *some* child list. (And it seem like it could be problematic for other reasons.) I don't know if the problem is similar here. And it seems weird that the assertion condition is ok with some cases of not being reachable via the frame tree, some dating back to the original landing in https://hg.mozilla.org/mozilla-central/rev/9ec74c8e5690 with a tweak in https://hg.mozilla.org/mozilla-central/rev/13a9aecb404f ; it seems like those conditions are really just saying that things are equally bad in that case, but we're just not going to assert about it. I'm curious if bz has any thoughts here.
Flags: needinfo?(bzbarsky)
Assignee | ||
Comment 66•10 years ago
|
||
Here's a try push: https://tbpl.mozilla.org/?tree=Try&rev=8c6cb85b2232 with this patch: https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/85f2feb736de/debug-997506 that should give a bit more information about what sort of element and frame we're asserting about. It'll probably need some retriggers to show the bug. (Sorry, meant to do something like this sooner.)
Comment 67•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38311130&tree=Mozilla-Inbound
Comment 68•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38317607&tree=Mozilla-Inbound
Comment 69•10 years ago
|
||
Conveniently, try didn't need any retriggers, you got it in one round.
Comment 70•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38329511&tree=Mozilla-Central
Assignee | ||
Comment 71•10 years ago
|
||
Not too surprisingly, the element in question looks like this (this is from code that regenerates the markup from the content tree):
<div class="newtab-site" draggable="true" type="undefined"><a class="newtab-link" title="file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/769303-2.html" href="file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/769303-2.html"> <span class="newtab-thumbnail" style="background-image: url(\"moz-page-thumb://thumbnail?url=file%3A%2F%2F%2Fbuilds%2Fslave%2Ftest%2Fbuild%2Ftests%2Freftest%2Ftests%2Flayout%2Fgeneric%2Fcrashtests%2F769303-2.html\");"></span> <span class="newtab-title">file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/769303-2.html</span></a><input type="button" title="Pin this site at its current position" class="newtab-control newtab-control-pin"></input><input type="button" title="Remove this site" class="newtab-control newtab-control-block"></input><input type="button" title="Show information on sponsored tiles" class="newtab-control newtab-control-sponsored"></input></div>
Or, alternatively:
>00:38:09 INFO - div@0xe698278 class="newtab-site" draggable="true" type="undefined" state=[40000010000] flags=[03220404] primaryframe=(nil) refcount=7<
>00:38:09 INFO - a@0xb808438 class="newtab-link" title="file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/769303-2.html" href="file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/769303-2.html" state=[40000010800] flags=[03200400] primaryframe=(nil) refcount=7<
>00:38:09 INFO - Text@0xa4e5530 flags=[00000000] primaryframe=(nil) refcount=1< >
>00:38:09 INFO - span@0xa4e5578 class="newtab-thumbnail" style="background-image: url(\"moz-page-thumb://thumbnail?url=file%3A%2F%2F%2Fbuilds%2Fslave%2Ftest%2Fbuild%2Ftests%2Freftest%2Ftests%2Flayout%2Fgeneric%2Fcrashtests%2F769303-2.html\");" state=[40000010000] flags=[00200400] primaryframe=(nil) refcount=5<>
>00:38:09 INFO - Text@0xc3163a8 flags=[00000000] primaryframe=(nil) refcount=1< >
>00:38:09 INFO - span@0xc3163f0 class="newtab-title" state=[40000010000] flags=[00200400] primaryframe=(nil) refcount=3<
>00:38:09 INFO - Text@0xd8690c8 flags=[00000000] primaryframe=(nil) refcount=1<file:///builds/slave/test/build/tests/reftest/tests/layout/generic/crashtests/769303-2.html>
>00:38:09 INFO - >
>00:38:09 INFO - >
>00:38:09 INFO - input@0xc7359d0 type="button" title="Pin this site at its current position" class="newtab-control newtab-control-pin" state=[40000010240] flags=[00200400] primaryframe=(nil) refcount=1<>
>00:38:09 INFO - input@0xb0f3cb8 type="button" title="Remove this site" class="newtab-control newtab-control-block" state=[40000010240] flags=[00200400] primaryframe=(nil) refcount=1<>
>00:38:09 INFO - input@0xbc78660 type="button" title="Show information on sponsored tiles" class="newtab-control newtab-control-sponsored" state=[40000010240] flags=[00200404] primaryframe=(nil) refcount=2<>
>00:38:09 INFO - >
The null-primaryframe aspect is interesting, although maybe not surprising.
Assignee | ||
Comment 73•10 years ago
|
||
One theory is that the regression is related to something that's making parts of or all of the new tab page stick around much longer than they're supposed to.
Assignee | ||
Comment 74•10 years ago
|
||
I added some further debugging that's showing some pretty crazy behavior, even locally -- I see restyles related to the newtab page at random times (especially during page navigation), even when the newtab page isn't showing and hasn't been shown during the session. My current debugging patch is https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/d053fd83e353/debug-997506 (the stuff in RestyleTracker.h is what's new). More later...
Assignee | ||
Comment 76•10 years ago
|
||
So if I start Firefox up, such that I see the default homepage, and have no back/forward history (although I admit I had the default start page pref set to show my tabs from last time), I still see restyles happening in the new tab page even though that new tab page isn't visible anywhere. The top of the C++ stack is: #0 mozilla::RestyleTracker::AddPendingRestyleToTable (this=0x32bd028, aElement=0x33d75f0, aRestyleHint=eRestyle_Subtree, aMinChangeHint=0) at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/style/../base/RestyleTracker.h:375 #1 0x00007ffff3125b6e in mozilla::RestyleTracker::AddPendingRestyle ( this=0x32bd028, aElement=0x33d75f0, aRestyleHint=eRestyle_Subtree, aMinChangeHint=0) at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/style/../base/RestyleTracker.h:412 #2 0x00007ffff31c68fb in mozilla::RestyleManager::PostRestyleEventCommon ( this=0x32bcfe0, aElement=<optimized out>, aRestyleHint=<optimized out>, aMinChangeHint=<optimized out>, aForAnimation=<optimized out>) at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/RestyleManager.cpp:1537 #3 0x00007ffff31a37ba in AttributeWillChange (aModType=2, aAttribute= 0x743130, aNameSpaceID=0, aElement=0x33d75f0, this=0x32c5df0, aDocument=<optimized out>) at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsPresShell.cpp:4219 #4 PresShell::AttributeWillChange (this=0x32c5df0, aDocument=0x32b9d10, aElement=0x33d75f0, aNameSpaceID=0, aAttribute=0x743130, aModType=2) at /home/dbaron/builds/ssd/mozilla-central/mozilla/layout/base/nsPresShell.cpp:4204 #5 0x00007ffff2df0b93 in nsNodeUtils::AttributeWillChange ( aElement=0x33d75f0, aNameSpaceID=0, aAttribute=0x743130, aModType=2) at /home/dbaron/builds/ssd/mozilla-central/mozilla/content/base/src/nsNodeUtils.cpp:100 #6 0x00007ffff2d7783c in SetAttr (aNotify=true, aValue=..., aPrefix=0x0, aName=0x743130, aNamespaceID=0, this=0x33d75f0) at /home/dbaron/builds/ssd/mozilla-central/mozilla/content/base/src/Element.cpp:1781 #7 mozilla::dom::Element::SetAttr (this=0x33d75f0, aNamespaceID=0, aName=0x743130, aPrefix=0x0, aValue=..., aNotify=true) at /home/dbaron/builds/ssd/mozilla-central/mozilla/content/base/src/Element.cpp:1753 #8 0x00007ffff2ed5149 in nsGenericHTMLElement::SetAttr (this=0x33d75f0, aNameSpaceID=0, aName=0x743130, aPrefix=<optimized out>, aValue=..., aNotify=true) at /home/dbaron/builds/ssd/mozilla-central/mozilla/content/html/content/src/nsGenericHTMLElement.cpp:904 #9 0x00007ffff2d740e4 in SetAttr (aNotify=true, aName=<optimized out>, aNameSpaceID=0, aValue=..., this=0x33d75f0) at ../../../dist/include/mozilla/dom/Element.h:452 #10 SetAttribute (aError=..., aValue=..., aName=..., this=0x33d75f0) ---Type <return> to continue, or q <return> to quit--- at /home/dbaron/builds/ssd/mozilla-central/mozilla/content/base/src/Element.cpp:853 #11 mozilla::dom::Element::SetAttribute (this=0x33d75f0, aName=..., aValue=..., aError=...) at /home/dbaron/builds/ssd/mozilla-central/mozilla/content/base/src/Element.cpp:826 #12 0x00007ffff26569ad in mozilla::dom::ElementBinding::setAttribute (cx= 0x8fb340, obj=..., self=0x33d75f0, args=...) at /home/dbaron/builds/ssd/mozilla-central/obj/firefox-debugopt/dom/bindings/ElementBinding.cpp:269 #13 0x00007ffff28f274c in mozilla::dom::GenericBindingMethod (cx=0x8fb340, argc=<optimized out>, vp=<optimized out>) at /home/dbaron/builds/ssd/mozilla-central/mozilla/dom/bindings/BindingUtils.cpp:2301 #14 0x00007ffff3edece7 in js::CallJSNative (cx=0x8fb340, native=0x7ffff28f2627 <mozilla::dom::GenericBindingMethod(JSContext*, unsigned int, JS::Value*)>, args=...) at /home/dbaron/builds/ssd/mozilla-central/mozilla/js/src/jscntxtinlines.h:239 and the JS stack is: InterpreterFrame at 0x7f48e8 callee fun: <function Site_render (chrome://browser/content/newtab/newTab.js:980) at 0x7fff6d123940> file chrome://browser/content/newtab/newTab.js line 980 pc = 0x7fff98030830 current op: call this: <Object at 0x7fffd8c0fb90> rval: undefined flags: scopeChain: (JSObject *) 0x7fff78496100 InterpreterFrame at 0x7f4850 callee fun: <function Site (chrome://browser/content/newtab/newTab.js:869) at 0x7fff6d120280> file chrome://browser/content/newtab/newTab.js line 869 pc = 0x7fff98027c22 current op: call this: <Object at 0x7fffd8c0fb90> rval: undefined flags: constructing scopeChain: (JSObject *) 0x7fff78496100 InterpreterFrame at 0x7f47a0 callee fun: <function Grid_createSite (chrome://browser/content/newtab/newTab.js:592) at 0x7fff6d123140> file chrome://browser/content/newtab/newTab.js line 592 pc = 0x7fff98026ad8 current op: new this: <Object at 0x7fff6d120f80> rval: undefined flags: scopeChain: (JSObject *) 0x7fff78496100 InterpreterFrame at 0x7f46d8 callee fun: <function Grid_renderSites (chrome://browser/content/newtab/newTab.js:703) at 0x7fff6d123340> file chrome://browser/content/newtab/newTab.js line 703 pc = 0x7fff980270e3 current op: call staticScope: <Block object at 0x7fff6d113c80> this: <Object at 0x7fff6d120f80> rval: undefined flags: scopeChain: (JSObject *) 0x7fff78496100 InterpreterFrame at 0x7f4640 callee fun: <function Grid_init/< (chrome://browser/content/newtab/newTab.js:578) at 0x7fff6d143ca0> file chrome://browser/content/newtab/newTab.js line 578 pc = 0x7fff98026db1 current op: call this: <Object at 0x7fff6d120f80> rval: undefined flags: scopeChain: (JSObject *) 0x7fff78496100 InterpreterFrame at 0x7f4598 callee fun: <function executeCallbacks (resource://gre/modules/NewTabUtils.jsm:769) at 0x7fff6edf7e00> file resource://gre/modules/NewTabUtils.jsm line 769 pc = 0x12250f8 current op: call staticScope: <Block object at 0x7fffc853ba80> this: undefined rval: undefined flags: scopeChain: (JSObject *) 0x7fffd8c01890 InterpreterFrame at 0x7f4518 callee fun: <function Links_populateCache/< (resource://gre/modules/NewTabUtils.jsm:784) at 0x7fff6d102fa0> file resource://gre/modules/NewTabUtils.jsm line 784 pc = 0x12251c3 current op: call this: undefined rval: undefined flags: scopeChain: (JSObject *) 0x7fffd8c01890 InterpreterFrame at 0x7f4460 callee fun: <function Links_populateProviderCache/< (resource://gre/modules/NewTabUtils.jsm:854) at 0x7fff6edf9040> file resource://gre/modules/NewTabUtils.jsm line 854 pc = 0x1225a4b current op: call this: <Object at 0x7fffc8549200> rval: undefined flags: scopeChain: (JSObject *) 0x7fffd8c01980 JIT frame callee fun: <function PlacesProvider_getLinks/callback.handleCompletion (resource://gre/modules/NewTabUtils.jsm:589) at 0x7fff6edf7f00> file resource://gre/modules/NewTabUtils.jsm line 589 pc = 0x1224487 current op: call this: <Object at 0x7fffd8c01a90> flags: scopeChain: (JSObject *) 0x7fffd8c019f0 It seems pretty broken that we're running JS in the new tab page when that page isn't showing. (And, sure enough, the elements in it at least don't have frames.) If I look, in frame 11 of the above stack, at *this->mNodeInfo.mRawPtr->mDocument->mDocumentURI.mRawPtr, I see it's the URL about:newtab. If I look at its mDocumentContainer, I find a docshell whose mCurrentURI is still about:newtab. That docshell's mParent is a docshell, whose mCurrentURI is data:application/vnd.mozilla.xul+xml;charset=utf-8,<window%20id='win'/> . In turn, that docshell's mParent is a docshell, whose mCurrentURI is resource://gre-resources/hiddenWindow.html . (That docshell's parent is an nsDocLoader.) So there seems to be a semi-active about:newtab attached to the hidden window that's having restyles posted in it.
Assignee | ||
Comment 77•10 years ago
|
||
This about:newtab document has a pres context, pres shell, frame manager, and a correct-looking frame tree. And in that frame tree, we have a frame for the parent of the element on which we're posting the restyle, but no frame for the element itself. The parent's frame is: Box(div)(0)@337af80 next=337b158 {0,0,0,0} [state=0009160080c00602] [content=3329860] [sc=337abe0,parent=337a1e8]<>
Comment 78•10 years ago
|
||
You're seeing the about:newtab preloader. BrowserNewTabPreloader.jsm keeps an about:newtab in a xul:browser in the hidden window: http://mxr.mozilla.org/mozilla-central/source/browser/modules/BrowserNewTabPreloader.jsm When you open a new browser tab, that tab's docshell is swapped out for the docshell containing the preloaded about:newtab: http://mxr.mozilla.org/mozilla-central/source/browser/base/content/tabbrowser.xml#1611 Then the preloader loads a new about:newtab so it's ready for swapping the next time you open a new tab. The "active" nature of the preloaded about:newtab page is due to the fact that we update that page as necessary as you're browsing to reflect changes in the rankings of your top sites.
Comment 79•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38352384&tree=Mozilla-Inbound
Assignee | ||
Comment 81•10 years ago
|
||
Since I'm still stumped as to why the assertion is happening (although I'm more convinced that we should be asserting what it's asserting), another try push: https://tbpl.mozilla.org/?tree=Try&rev=729422675ce3 https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/560fb7b30794/debug-997506
Assignee | ||
Comment 83•10 years ago
|
||
Er, again, with the right debugging patch: <been waiting for try for 5 or 10 minutes now, I'll give you the URL later> Also, after reading comment 78, I'm ready to go back to my safe little Firefox OS world now.
Assignee | ||
Comment 84•10 years ago
|
||
I gave up on pushing to try; this bug is now blocked on either (1) the try repository working again or (2) somebody producing a reduced testcase from the frontend code.
Assignee | ||
Comment 85•10 years ago
|
||
https://tbpl.mozilla.org/?tree=Try&rev=377b9ccb648f
Comment 89•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38372703&tree=B2g-Inbound
Assignee | ||
Comment 90•10 years ago
|
||
Well, I got debugging output in: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/dbaron@mozilla.com-377b9ccb648f/try-linux64-debug/try_ubuntu64_vm-debug_test-crashtest-bm68-tests1-linux64-build266.txt.gz but so far I haven't found anything interesting (i.e., broken). The first assertion is asserting on a restyle of element 0x42e7160, restyle hint is 2 (eRestyle_Subtree), and change hint is 0. (This matches what shows in the stack in comment 76.) A quote from the content tree is: > div@0x162b2a0 id="newtab-grid" style="height: 556px; max-height: 556px; max-width: 832px;" state=[40000010000] flags=[00240004] primaryframe=0x436cf60 refcount=25< > div@0x3861bc0 class="newtab-cell" state=[40000010000] flags=[0024140c] primaryframe=0x436d898 refcount=15< > div@0x42e7160 class="newtab-site" draggable="true" type="undefined" state=[40000010000] flags=[03220404] primaryframe=(nil) refcount=7< Note that the element we're restyling has no frame, and its parent has a frame (a Box). I suppose I could have printed its GetFlattenedTreeParent, although I don't see any XBL involved, so that seems unlikely to be interesting.
Comment 91•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38377951&tree=Mozilla-Central
Assignee | ||
Comment 92•10 years ago
|
||
I realize that the bit that I don't know -- why the child has no frame -- is one of the few things that I *can* debug locally. So I'll try that, but probably not until tomorrow morning.
Assignee | ||
Comment 93•10 years ago
|
||
Actually, I don't think there's anything useful I can debug locally, so without a reproducable testcase, I'm going to give upon this and go back to other work.
Assignee | ||
Comment 94•10 years ago
|
||
Wait, I think the problem is that here: (In reply to David Baron [:dbaron] (needinfo? me) (UTC-7) from comment #0) > 16:05:31 INFO - mozilla::RestyleManager::ProcessPendingRestyles() > [layout/base/RestyleManager.cpp:1425] > 16:05:31 INFO - > mozilla::RestyleManager::RebuildAllStyleData(nsChangeHint) > [obj-firefox/dist/include/nsContentUtils.h:2245] we don't call mFrameConstructor->CreateNeededFrames() like we do on all the other ways of entering ProcessPendingRestyles.
Assignee | ||
Comment 95•10 years ago
|
||
I bet this fixes it: https://tbpl.mozilla.org/?tree=Try&rev=04f57c66868f
Assignee | ||
Comment 96•10 years ago
|
||
This is needed to avoid hitting the assertion "Why did this not get handled while processing mRestyleRoots?" in CollectRestyles.
Attachment #8411638 -
Flags: review?(tnikkel)
Assignee | ||
Updated•10 years ago
|
Flags: needinfo?(bzbarsky)
Comment 98•10 years ago
|
||
Comment on attachment 8411638 [details] [diff] [review] Call CreateNeededFrames for all ProcessPendingRestyles calls rather than just most Good catch. In the content dump above the flags do indeed have the NEEDS_FRAME bit set.
Attachment #8411638 -
Flags: review?(tnikkel) → review+
Comment 99•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38408160&tree=Mozilla-Inbound
Comment 100•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38411324&tree=B2g-Inbound
Assignee | ||
Comment 101•10 years ago
|
||
(In reply to Timothy Nikkel (:tn) from comment #98) > Good catch. In the content dump above the flags do indeed have the > NEEDS_FRAME bit set. Yes, indeed; I didn't even think to check that. (For the record, that's 0x20000 in the flags word.)
Assignee | ||
Comment 102•10 years ago
|
||
landing |
https://hg.mozilla.org/integration/mozilla-inbound/rev/114a84c729db
Comment 103•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38421271&tree=Mozilla-Central
Comment 105•10 years ago
|
||
https://tbpl.mozilla.org/php/getParsedLog.php?id=38472879&tree=B2g-Inbound
Comment 106•10 years ago
|
||
landing |
https://hg.mozilla.org/mozilla-central/rev/114a84c729db
Assignee: nobody → dbaron
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Comment 107•10 years ago
|
||
Does this affect older branches as well? If so, should we backport the fix?
status-firefox29:
--- → wontfix
status-firefox30:
--- → ?
status-firefox31:
--- → fixed
Flags: needinfo?(dbaron)
Assignee | ||
Comment 108•10 years ago
|
||
The underlying Gecko bug is a regression from https://hg.mozilla.org/mozilla-central/rev/f80de1a7c604 which landed in April 2010. I'm not sure what that says about whether to backport the fix, but I'm not particularly inclined to do so.
Blocks: lazyfc
Flags: needinfo?(dbaron)
Updated•10 years ago
|
status-firefox-esr24:
--- → wontfix
You need to log in
before you can comment on or make changes to this bug.
Description
•