Closed
Bug 1112465
Opened 9 years ago
Closed 9 years ago
Crash when navigating from HTML page to about:config while inspector is shown (without e10s)
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla37
Tracking | Status | |
---|---|---|
firefox35 | --- | unaffected |
firefox36 | + | fixed |
firefox37 | + | fixed |
People
(Reporter: Hb, Assigned: pbro)
References
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
961 bytes,
patch
|
ehsan.akhgari
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-975ac09e-fe89-4718-9db9-bb80d2141216. ============================================================= STR 1. Open a web page 2. Open Inspector 3. Select Font section in Inspector 4. Type about:config in the Address Bar 5. Hit Enter Crash
With a new profile this crash can't be repeated. Former crashes with this issue were bp-83fd09f1-4a65-491f-b4b1-e87a92141217 , bp-88080806-0766-476f-a487-23a072141216 and bp-dd3c9af6-8b05-4897-b8c9-f09032141216
Status: NEW → UNCONFIRMED
Ever confirmed: false
Comment 2•9 years ago
|
||
[Tracking Requested - why for this release]: [Tracking Requested - why for this release]: I can reproduce with new profile bp-625db745-7942-490b-8d8a-51c182141217 https://hg.mozilla.org/integration/fx-team/pushloghtml?fromchange=0fa23eddfe12&tochange=8e9b2357abcb Triggered by: Bug 985597
Blocks: 985597
Status: UNCONFIRMED → NEW
status-firefox35:
--- → unaffected
status-firefox36:
--- → affected
status-firefox37:
--- → affected
tracking-firefox36:
--- → ?
tracking-firefox37:
--- → ?
Component: General → DOM
Ever confirmed: true
Product: Firefox → Core
Updated•9 years ago
|
Version: Trunk → 36 Branch
Updated•9 years ago
|
Flags: needinfo?(pbrosset)
Assignee | ||
Comment 3•9 years ago
|
||
The crash seems to be caused by nsIDocument::RemoveAnonymousContent(mozilla::dom::AnonymousContent&, mozilla::ErrorResult&). This method is called when the devtools highlighter (in the inspector) is removed from the document. So, typically, when navigating from a page to about:config here. If this is indeed the case, I don't think this is related to the font inspector at all, but rather just to the fact that the inspector is opened. I haven't been able to reproduce so far on Mac at all.
Flags: needinfo?(pbrosset)
Comment 4•9 years ago
|
||
(In reply to Patrick Brosset [:pbrosset] [:patrick] from comment #3) > The crash seems to be caused by > nsIDocument::RemoveAnonymousContent(mozilla::dom::AnonymousContent&, > mozilla::ErrorResult&). > This method is called when the devtools highlighter (in the inspector) is > removed from the document. So, typically, when navigating from a page to > about:config here. > If this is indeed the case, I don't think this is related to the font > inspector at all, but rather just to the fact that the inspector is opened. > I haven't been able to reproduce so far on Mac at all. You should need to try on non-e10s window.
Updated•9 years ago
|
Flags: needinfo?(pbrosset)
Assignee | ||
Comment 5•9 years ago
|
||
Ok, I found a way to reproduce the error. It's somewhat related to e10s. about:config isn't opened in the child process, its tab is in the parent process. And it seems that when we move from an e10s tab (the page you first open) to a non e10s tab (the about:config page), this closes the toolbox, and that prevents the crash from occurring. So, to replicate the crash: - open about:sessionrestore (this page is also in a non-e10s tab and is HTML, so the inspector uses the new highlighter) - open the inspector - navigate to about:config -> crash
Flags: needinfo?(pbrosset)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → pbrosset
Summary: Crash when opening about:config while Font Inspector is shown → Crash when navigating from HTML page to about:config while inspector is shown (without e10s)
Assignee | ||
Updated•9 years ago
|
OS: Windows 7 → All
Hardware: x86_64 → All
Assignee | ||
Comment 6•9 years ago
|
||
Happens when, without e10s, you navigate from an HTML page to a XUL page (like about:newtab), with the inspector open. So it's not only related to about:config. That's weird because this case was specifically taken care of in bug 1020244 and bug 985597, which were the bugs that were used to implement the new highlighter. The new highlighter isn't compatible with XUL, but that's why we specifically put in place a mechanism that would fall back to another, simpler, highlighter when navigating to XUL. So this mechanism must be failing somehow. I'm currently clobber building a debug build to try and find a fix.
Assignee | ||
Comment 7•9 years ago
|
||
Here is the relevant part of the backtrace: * thread #1: tid = 0x4d4ba9, 0x0000000102c59d8c XUL`nsCOMPtr<mozilla::dom::Element>::get(this=0x00000000000000a0) const + 12 at nsCOMPtr.h:686, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0xa0) * frame #0: 0x0000000102c59d8c XUL`nsCOMPtr<mozilla::dom::Element>::get(this=0x00000000000000a0) const + 12 at nsCOMPtr.h:686 frame #1: 0x0000000102c58865 XUL`nsCOMPtr<mozilla::dom::Element>::operator mozilla::dom::Element*(this=0x00000000000000a0) const + 21 at nsCOMPtr.h:694 frame #2: 0x0000000103206fac XUL`nsCanvasFrame::GetCustomContentContainer(this=0x0000000000000000) const + 28 at nsCanvasFrame.h:104 frame #3: 0x00000001031c7511 XUL`nsIDocument::RemoveAnonymousContent(this=0x0000000118de4000, aContent=0x000000012c3e3200, aRv=0x00007fff5fbece48) + 97 at nsDocument.cpp:5245
Assignee | ||
Comment 8•9 years ago
|
||
I think this should fix it. What was happening is that by the time devtools tries to remove the inserted AnonymousContent, the document it does it on is already the destination XUL document. And it turns out only InsertAnonymousContent was checking for |shell->GetCanvasFrame()|, not RemoveAnonymousContent. So this patch only adds this simple test to avoid crashing.
Attachment #8537804 -
Flags: review?(ehsan.akhgari)
Updated•9 years ago
|
Attachment #8537804 -
Flags: review?(ehsan.akhgari) → review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 9•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/81bca1a05670
Keywords: checkin-needed
Comment 11•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/81bca1a05670
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Comment 12•9 years ago
|
||
Patrick, can we have an uplift request for aurora? Thanks
Flags: needinfo?(pbrosset)
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8537804 [details] [diff] [review] bug1112465-check-for-canvasframe-in-removeAnonymousContent v1.patch Approval Request Comment [Feature/regressing bug #]: This is a regression introduced by a refactor of the devtools highlighter done in bug 985597. [User impact if declined]: If declined, users opening the devtools inspector on an HTML page and then navigating to a XUL page will have a crash. [Describe test coverage new/current, TBPL]: The highlighter is fairly heavily tested by devtools mochitests, but somehow, we didn't have a test for this very specific scenario, hence the regression. The patch didn't introduce a new test though. I'll file a follow-up bug to add one, but I think the patch can be uplifted without one. [Risks and why]: Not much risk involved, the fix is a simple one line check. [String/UUID change made/needed]: None.
Flags: needinfo?(pbrosset)
Attachment #8537804 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8537804 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•