Closed
Bug 503391
Opened 15 years ago
Closed 15 years ago
[Mac 10.4] Crash in [@ nsViewManager::IsViewInserted(nsView*)] using Google Toolbar
Categories
(Core :: Widget: Cocoa, defect)
Tracking
()
VERIFIED
FIXED
Tracking | Status | |
---|---|---|
status1.9.2 | --- | beta1-fixed |
status1.9.1 | --- | .2-fixed |
People
(Reporter: marcia, Assigned: smichaud)
References
Details
(6 keywords, Whiteboard: [sg:critical?] not a crash in 1.9.0)
Crash Data
Attachments
(4 files, 1 obsolete file)
1.07 KB,
text/html
|
Details | |
5.72 KB,
text/plain
|
Details | |
517 bytes,
text/plain
|
Details | |
2.47 KB,
patch
|
jaas
:
review+
beltzner
:
approval1.9.1.2+
dveditz
:
approval1.9.0.14+
|
Details | Diff | Splinter Review |
Seen while running Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1) Gecko/20090624 Firefox/3.5 GTB5 STR: 1. Install the latest version of Google Toolbar. Retain all of the default settings. 2. Type at least 7 letters in the Google search box. Sometimes I have to backspace a few times to initiate the crash. http://tinyurl.com/l7pta5 has the list of crashes to date for 3.5. It currently ranks as the #9 top crash on Mac. This is 10.4 only and does not affect 10.5. One of my sample breakpad reports: http://crash-stats.mozilla.com/report/index/1c0b98a4-3c03-42ab-bcef-cf5492090709 mw22 indicates he will work on a reduced test case later this afternoon.
Reporter | ||
Comment 1•15 years ago
|
||
This crashes after 1 second. I get this stacktrace constantly with it: http://crash-stats.mozilla.com/report/index/03130994-0d9e-4d80-8f58-7dafa2090709 0 @0x0 1 XUL nsXULPopupManager::HidePopupCallback layout/xul/base/src/nsXULPopupManager.cpp:761 2 XUL nsXULPopupManager::FirePopupHidingEvent layout/xul/base/src/nsXULPopupManager.cpp:1096 3 XUL nsXULPopupManager::HidePopup layout/xul/base/src/nsXULPopupManager.cpp:713 4 XUL nsPopupBoxObject::HidePopup layout/xul/base/src/nsPopupBoxObject.cpp:93 5 XUL NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179 6 XUL XPCWrappedNative::CallMethod js/src/xpconnect/src/xpcwrappednative.cpp:2454 7 XUL XPC_WN_CallMethod js/src/xpconnect/src/xpcwrappednativejsops.cpp:1590 8 libmozjs.dylib js_Invoke js/src/jsinterp.cpp:1386 9 libmozjs.dylib js_Interpret js/src/jsinterp.cpp:5179 10 libmozjs.dylib js_Invoke js/src/jsinterp.cpp:1394 11 libmozjs.dylib js_InternalInvoke js/src/jsinterp.cpp:1447 12 libmozjs.dylib JS_CallFunctionValue js/src/jsapi.cpp:5187 13 XUL nsJSContext::CallEventHandler dom/src/base/nsJSEnvironment.cpp:2035 14 XUL nsGlobalWindow::RunTimeout etc.. I think it is the same bug, I haven't been able to crash with this testcase on MacOS X 10.5. Probably the first 2 frames are munged, the other frames look the same. The content of the iframe is this: <?xml-stylesheet href="chrome://browser/skin/" type="text/css"?> <window xmlns="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"> <popup id="auto" style="width: 100px; height: 100px;"/> <script id="script" xmlns="http://www.w3.org/1999/xhtml"><![CDATA[ function doe() { document.getElementById('auto').openPopup(); setTimeout(doe2, 40); } function doe2() { window.frameElement.style.display = 'none'; document.getElementById('auto').hidePopup(); } setTimeout(doe, 100); ]]></script></window>
Comment 2•15 years ago
|
||
Sorry, that last Marcia was me.
mw22: please file a new bug for that, it's different (simple null pointer). Signature nsViewManager::IsViewInserted(nsView*) UUID 1c0b98a4-3c03-42ab-bcef-cf5492090709 Time 2009-07-09 13:50:29.354813 Uptime 28 Last Crash 83 seconds before submission Product Firefox Version 3.5 Build ID 20090624012136 Branch 1.9.1 OS Mac OS X OS Version 10.4.11 8S2167 CPU x86 CPU Info GenuineIntel family 6 model 14 stepping 8 Crash Reason EXC_BAD_ACCESS / KERN_INVALID_ADDRESS Crash Address 0x153b880 User Comments Google toolbar new version Processor Notes Crashing Thread Frame Module Signature [Expand] Source 0 XUL nsViewManager::IsViewInserted view/src/nsView.h:124 1 XUL nsViewManager::SetViewVisibility view/src/nsViewManager.cpp:1761 2 XUL nsMenuPopupFrame::HidePopup layout/xul/base/src/nsMenuPopupFrame.cpp:668 3 XUL nsXULPopupManager::HidePopupCallback layout/xul/base/src/nsXULPopupManager.cpp:761 4 XUL nsXULPopupManager::FirePopupHidingEvent layout/xul/base/src/nsXULPopupManager.cpp:1096 5 XUL nsXULPopupManager::HidePopup layout/xul/base/src/nsXULPopupManager.cpp:713 6 XUL nsPopupBoxObject::HidePopup layout/xul/base/src/nsPopupBoxObject.cpp:93 7 XUL NS_InvokeByIndex_P xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179 8 XUL XPCWrappedNative::CallMethod js/src/xpconnect/src/xpcwrappednative.cpp:2454 9 XUL XPC_WN_CallMethod js/src/xpconnect/src/xpcwrappednativejsops.cpp:1590 10 libmozjs.dylib js_Invoke js/src/jsinterp.cpp:1386 11 libmozjs.dylib js_Interpret js/src/jsinterp.cpp:5179
Component: Widget: Mac → Layout: View Rendering
QA Contact: mac → layout.view-rendering
Summary: [Mac 10.4] Crash in [nsViewManager::IsViewInserted(nsView*)] using Google Toolbar → [Mac 10.4] Crash in [@ nsViewManager::IsViewInserted(nsView*)] using Google Toolbar
Comment 4•15 years ago
|
||
It seems to me it is the same bug, because the crash only occurs on Mac OS 10.4, just like all the Google toolbar crashes. Also, with the unminimized testcases, I got the @ nsViewManager::IsViewInserted and @ nsViewManager::SetViewVisibility crashes. Anyway, I think this should be fixed for the 1.9.1 branch, this didn't crash in Firefox 3, so it is a regression. Like Marcia said, this is a topcrash on the Mac (only affecting Mac OS 10.4 in combination with the Google toolbar). But I think this is responsible for the @ nsViewManager::IsViewInserted, the @ nsViewManager::SetViewVisibility and the @ nsMenuPopupFrame::HidePopup topcrashes. They are resp. nr. 7, 10 and 18 on the topcrasher list for MacOS X. As further evidence that these are the same crashes, they all happen on MacOS X 10.4 only and comments often mention they were searching something within the Google toolbar.
Flags: wanted1.9.2?
Flags: wanted1.9.1.x?
Flags: blocking1.9.2?
Flags: blocking1.9.1.1?
Keywords: regression,
testcase
Comment 5•15 years ago
|
||
Also the nr. 35 crash seems related to this, the @ nsViewManager::ResizeView crash.
Comment 6•15 years ago
|
||
I'd love to mark this blocking for a 1.9.1.x release, but I'd love even more to have an owner who's at least investigating it. Does this belong in the hands of someone on the Mac team or someone on the Layout team?
Flags: wanted1.9.1.x? → wanted1.9.1.x+
Comment 7•15 years ago
|
||
It seems to me this is a Mac widget issue.
Comment 8•15 years ago
|
||
Josh, can you own this or find someone to own it?
Assignee: nobody → joshmoz
Assignee | ||
Comment 9•15 years ago
|
||
This could be a Cocoa widget bug. But it's at least as likely to be a Layout bug (which for some reason is only triggered on 10.4). I'd be willing to investigate this further ... at least far enough to find out whether or not it's a Cocoa widgets bug (and then to fix it if it is). Unless Josh wants to do this :-)
Updated•15 years ago
|
Assignee | ||
Comment 11•15 years ago
|
||
I can easily reproduce this bug's crash with the testcase (attachment 387797). No need to use/install the Google Toolbar. I did have to wait 10-15 seconds for the crash to happen, though. I tested on OS X 10.4.11 using an opt build with debug symbols made from recent trunk (mozilla-central) code.
Assignee | ||
Comment 12•15 years ago
|
||
For reference, here's the source of the testcase's embedded iframe, with its extended characters translated.
Assignee | ||
Comment 13•15 years ago
|
||
Here's the regression range for this bug: firefox-2008-02-19-04-trunk firefox-2008-02-20-04-trunk I've currently no idea which patch in this range might have triggered the problem. I'm also not going to be able to spend any more time on this for at least the next three weeks -- Josh wants me to work on other things, and I'm also going on vacation for a week.
Assignee: smichaud → nobody
Comment 14•15 years ago
|
||
Bonsai link for regression range: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-02-19+04%3A00%3A00&maxdate=2008-02-20+05%3A00%3A00&cvsroot=%2Fcvsroot Anyone else can/wants to take this? This is a very bad bug, where basically everyone on MacOSX 10.4 crashes when they type something in the Google Toolbar text input.
Comment 15•15 years ago
|
||
roc: Can you please find an owner for this? (Knowing that it could move to a Cocoa widget person if the bug moves that way, of course...)
Assignee: nobody → roc
It might be related to the popup manager. We need someone who has 10.4. I don't. I don't know who does. Neil, do you?
Assignee: roc → administration
Assignee | ||
Comment 17•15 years ago
|
||
For what it's worth: Bug 503196 may turn out to be related to this bug, or even to have the same cause. And that bug is reproducible on OS X 10.5.X.
Comment 18•15 years ago
|
||
I don't. Looking at the regression range, there are a number of bugs that affect Cocoa. One bug in particular, 417124, adds non-Leopard-only code which redraws windows when popups are shown. The crashstack suggests a crash trying to retrieve the parent of a popup view. Could be coincidental, but would be a good place to start looking.
Assignee | ||
Comment 19•15 years ago
|
||
Another note: I discovered that the following line in nsMenuPopupFrame::HidePopup() can result in both viewManager and view being deleted before the next line: viewManager->SetViewVisibility(view, nsViewVisibility_kHide);
Assignee | ||
Comment 20•15 years ago
|
||
Another note: AddRefing viewManager doesn't fix the crash :-) I'll stop here. Good luck!
I bet hiding the view is hiding the widget which somehow allows events to run which (because of the endless-reload script) is allowing the entire presentation to be torn down before we get to the next line.
Blocks: 417124
Actually, looking at bug 417124 the patch there is clearly capable of causing this kind of crash. Triggering synchronous painting during a Show(PR_FALSE) can flush reflows and result in frame or even prescontext/viewmanager destruction.
No longer blocks: 417124
Blocks: 417124
I think we just need a new version of the patch for bug 417124 that invalidates the windows for asynchronous painting instead of synchronous painting.
Assignee | ||
Updated•15 years ago
|
Assignee: administration → nobody
Component: Layout: View Rendering → Widget: Cocoa
QA Contact: layout.view-rendering → cocoa
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → smichaud
Assignee | ||
Comment 24•15 years ago
|
||
> I think we just need a new version of the patch for bug 417124 that > invalidates the windows for asynchronous painting instead of > synchronous painting. You're right, Roc. And thanks to both you and Neil for pointing out things I'd missed, which make this bug quite easy to fix. A tryserver build will follow in a few hours. As best I can tell my patch doesn't regress bug 417124. But I can't reproduce that bug, even on OS X 10.4, using the trunk nightly from the day before that patch landed (firefox-2008-02-19-04-trunk). So others will need to test more thoroughly.
Attachment #388927 -
Flags: review?(joshmoz)
Assignee | ||
Comment 25•15 years ago
|
||
Here's a tryserver build made with my patch from comment #24: https://build.mozilla.org/tryserver-builds/smichaud@pobox.com-bugzilla503391/bugzilla503391-macosx.dmg
Reporter | ||
Comment 26•15 years ago
|
||
Using the build in Comment 25, Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.2a1pre) Gecko/20090716 Minefield/3.6a1pre GTB5 I was not able to crash using either the STR in Comment 0 or using the testcase attached to the bug. Good work!
Steven, are you sure that setViewsNeedDisplay will actually cause repainting? You haven't marked any views as needing repainting, so I wonder if this will just cause it to look at its views and then stop without painting.
Assignee | ||
Comment 28•15 years ago
|
||
> Steven, are you sure that setViewsNeedDisplay will actually cause > repainting? You haven't marked any views as needing repainting, so > I wonder if this will just cause it to look at its views and then > stop without painting. I'm not really sure :-) I'm also not sure it'd be enough to mark the window's content view as needing display. That's why I noted that I can't reproduce bug 417124, and asked others to test. It may be that we simply no longer need to refresh the parent of a popup that's getting hidden.
Reporter | ||
Comment 29•15 years ago
|
||
I was not able to reproduce bug 417124 following Jesse's STR in the bug. I tried a few other sites and the only thing I did notice was that on the weather.com site the focus ring does not behave the same way in Firefox as it does in Safari - but that may be another issuse.
Assignee | ||
Comment 30•15 years ago
|
||
Here's an alternate version of my patch that simply reverses the main part of the patch for bug 417124. There seems to be a good chance that it's no longer needed. A tryserver build will follow in a few hours.
Attachment #389120 -
Flags: review?(joshmoz)
Assignee | ||
Comment 31•15 years ago
|
||
I'm also going to test on the two branches ... though I may not get to that until next week.
Assignee | ||
Comment 32•15 years ago
|
||
Here's a tryserver build made with my alternative patch from comment #30: https://build.mozilla.org/tryserver-builds/smichaud@pobox.com-bugzilla503391-alt/bugzilla503391-alt-macosx.dmg
Comment 33•15 years ago
|
||
(In reply to comment #3) > it's different (simple null pointer). executing null is different than simply dereferencing, and often indicates something scarier is going on.
Group: core-security
Whiteboard: [sg:critical?]
Updated•15 years ago
|
Flags: wanted1.9.0.x-
Comment 34•15 years ago
|
||
Comment on attachment 389120 [details] [diff] [review] Alternative fix (reverse main part of bug 417124 patch) I'm lacking a 10.4 partition at the moment but undoing the fix for 417124 is fine so long as somebody with a 10.4 partition has tested for regressing 417124. Even if it does regress 417124 we're probably better off fixing the crash than the visual polish.
Attachment #389120 -
Flags: review?(joshmoz) → review+
Attachment #388927 -
Flags: review?(joshmoz)
Assignee | ||
Updated•15 years ago
|
Attachment #388927 -
Attachment is obsolete: true
Reporter | ||
Comment 35•15 years ago
|
||
Using the build in Comment 32, I did a little testing and did not see any issues with the following: 1. Testcase in the bug 2. Original STR 3. Issue with focus reported in Bug 417124
Assignee | ||
Comment 36•15 years ago
|
||
I've tested my alternative patch (attachment 389120 [details] [diff] [review]) with the STR for bug 417124 (on OS X 10.4.11) on the trunk and both branches. It doesn't appear to regress bug 417124 ... at least in my tests. Here are three tryserver builds (for the trunk and each branch): https://build.mozilla.org/tryserver-builds/smichaud@pobox.com-bugzilla503391-alt/bugzilla503391-alt-macosx.dmg https://build.mozilla.org/tryserver-builds/smichaud@pobox.com-bugzilla503391-alt-191branch/bugzilla503391-alt-191branch-macosx.dmg https://build.mozilla.org/tryserver-builds/smichaud@pobox.com-bugzilla503391-alt-190branch/smichaud@pobox.com-bugzilla503391-alt-190branch-firefox-try-macosx.dmg Marcia and Jesse, please test all three against the STR for bug 417124 (as well as this bug's STR). Jesse, I'm asking you because you're the one who originally reported bug 417124. I'm not sure if you still have an OS X 10.4.11 partition to test on.
Comment 37•15 years ago
|
||
I don't have 10.4 any more, but I think Waldo does.
Assignee | ||
Comment 38•15 years ago
|
||
> but I think Waldo does
Where's Waldo? :-)
Assignee | ||
Comment 39•15 years ago
|
||
>> but I think Waldo does > > Where's Waldo? :-) Actually, I think Marcia's results are enough. I'm not good at evaluating UI bugs, so I didn't trust mine alone.
Assignee | ||
Comment 40•15 years ago
|
||
Marcia -- the trunk build from comment #36 is the same as that from comment #32, so you don't need to test that over again.
Reporter | ||
Comment 41•15 years ago
|
||
I tested the Shiretoko build listed in Comment 36, and all three items listed in Comment 35 pass as well. Now on to the last build - 3 branches is exhausting!
Reporter | ||
Comment 42•15 years ago
|
||
Using Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.0.12pre) Gecko/2009072009 GranParadiso/3.0.12pre GTB5 (the tryserver build listed in Comment 36), the three items also pass. The trifecta is complete.
Assignee | ||
Comment 43•15 years ago
|
||
Thanks, Marcia! I'll land this on the trunk.
Assignee | ||
Comment 44•15 years ago
|
||
Landed on trunk (mozilla-central): http://hg.mozilla.org/mozilla-central/rev/92678c089fb6
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Assignee | ||
Updated•15 years ago
|
Attachment #389120 -
Flags: approval1.9.1.2?
Assignee | ||
Comment 45•15 years ago
|
||
Comment on attachment 389120 [details] [diff] [review] Alternative fix (reverse main part of bug 417124 patch) This bug's crash is the #11 topcrasher on the 1.9.1 branch (with 372 instances in the last week). It also makes Google Toolbar completely unusable on OS X 10.4.11.
Assignee | ||
Comment 46•15 years ago
|
||
Comment on attachment 389120 [details] [diff] [review] Alternative fix (reverse main part of bug 417124 patch) For some reason this bug's crash doesn't appear in the 1.9.0 branch topcrasher list, though it can easily be reproduced on the 1.9.0 branch using this bug's testcase (attachment 387797 [details]). Possibly it's more difficult to reproduce there in the Google Toolbar. So it's less urgent to backport this patch to the 1.9.0 branch. We should probably wait until we see whether or not this patch causes any UI regressions on the trunk or the 1.9.1 branch.
Attachment #389120 -
Flags: approval1.9.0.13?
Reporter | ||
Comment 47•15 years ago
|
||
Verified fixed on the trunk using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.2a1pre) Gecko/20090721 Minefield/3.6a1pre GTB5. I verified by checking the testcase, installing the Google toolbar and doing several searches, and checking the focus issue in Bug 417124.
Status: RESOLVED → VERIFIED
Comment 48•15 years ago
|
||
I do indeed run 10.4 still, as I think I shouldn't have to upgrade my OS for at least the length of time it's under warranty (still have another year left, but Gecko development may force my hand, unfortunately). I'm not sure if I'm needed based on the comments here, assuming no for the moment; if so, please clearly point out the STR in a comment, because they're not obvious at a glance.
Comment 49•15 years ago
|
||
Comment on attachment 389120 [details] [diff] [review] Alternative fix (reverse main part of bug 417124 patch) a1912=beltzner
Attachment #389120 -
Flags: approval1.9.1.2? → approval1.9.1.2+
Assignee | ||
Comment 50•15 years ago
|
||
Comment on attachment 389120 [details] [diff] [review] Alternative fix (reverse main part of bug 417124 patch) Landed on the 1.9.1 branch: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/e6221c785010
Assignee | ||
Comment 51•15 years ago
|
||
Oddly, I'm not able to add the 'fixed1.9.1.2' keyword.
![]() |
||
Comment 52•15 years ago
|
||
(In reply to comment #51) > Oddly, I'm not able to add the 'fixed1.9.1.2' keyword. It's now a flag at status1.9.1.
Assignee | ||
Comment 53•15 years ago
|
||
> It's now a flag at status1.9.1.
Oh, OK. Sorry. I'll eventually learn the new flags :-)
Updated•15 years ago
|
Whiteboard: [sg:critical?] → [sg:critical?] not a crash in 1.9.0
Comment 54•15 years ago
|
||
Comment on attachment 389120 [details] [diff] [review] Alternative fix (reverse main part of bug 417124 patch) Approved for 1.9.0.13, a=dveditz for release-drivers
Attachment #389120 -
Flags: approval1.9.0.13? → approval1.9.0.13+
Assignee | ||
Comment 55•15 years ago
|
||
Comment on attachment 389120 [details] [diff] [review] Alternative fix (reverse main part of bug 417124 patch) Someone else will need to land this on the 1.9.0 branch. I'm away for the next week.
Assignee | ||
Updated•15 years ago
|
Whiteboard: [sg:critical?] not a crash in 1.9.0 → [sg:critical?] not a crash in 1.9.0 [needs landing on 1.9.0.13]
Updated•15 years ago
|
Keywords: checkin-needed
Reporter | ||
Comment 56•15 years ago
|
||
Verified on the 1.9.1 branch using Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.1.2) Gecko/20090729 Firefox/3.5.2 GTB5. Verified using the steps in Comment 47. Adding keyword.
Keywords: verified1.9.1
Assignee | ||
Comment 57•15 years ago
|
||
Comment on attachment 389120 [details] [diff] [review] Alternative fix (reverse main part of bug 417124 patch) Landed on the 1.9.0 branch: Checking in widget/src/cocoa/nsCocoaWindow.mm; /cvsroot/mozilla/widget/src/cocoa/nsCocoaWindow.mm,v <-- nsCocoaWindow.mm new revision: 1.153; previous revision: 1.152 done
Assignee | ||
Updated•15 years ago
|
Keywords: checkin-needed → fixed1.9.0.13
Whiteboard: [sg:critical?] not a crash in 1.9.0 [needs landing on 1.9.0.13] → [sg:critical?] not a crash in 1.9.0
Comment 58•15 years ago
|
||
Please use the appropriate fixed keyword for the approval flag showing. In this case, we firedrilled 1.9.0.13, so we renamed the flag.
Keywords: fixed1.9.0.13 → fixed1.9.0.14
Assignee | ||
Comment 59•15 years ago
|
||
> we firedrilled 1.9.0.13
Oops, I missed that.
Comment 60•15 years ago
|
||
Marcia, can you verify that this is fixed in a 1.9.0 nightly on 10.4? I don't have 10.4 available.
Updated•14 years ago
|
Group: core-security
Updated•14 years ago
|
Updated•14 years ago
|
blocking1.9.1: needed → ---
Updated•13 years ago
|
Crash Signature: [@ nsViewManager::IsViewInserted(nsView*)]
You need to log in
before you can comment on or make changes to this bug.
Description
•