Mozilla/5.0 (Windows NT 6.0; rv:2.0b10pre) Gecko/20110114 Firefox/4.0b10pre http://crash-stats.mozilla.com/report/index/bp-b5c26952-00b0-452a-a143-b59a82110114 http://crash-stats.mozilla.com/report/index/bp-1fb5be51-1765-4cde-b4bf-2ab162110114 http://crash-stats.mozilla.com/report/index/bp-9dd28713-3e08-4f54-9a98-aed002110114 I have URL and search bar on Menubar Now when I hide Menubar it crashes See my layout on nomenu_crash.png If I reset layout and try to hide Menubar there is no problem.

probably a dupe of bug 625771 0 xul.dll nsTextControlFrame::SetSelectionInternal layout/forms/nsTextControlFrame.cpp:859 1 xul.dll nsTextControlFrame::SetSelectionEndPoints layout/forms/nsTextControlFrame.cpp:942 2 xul.dll nsTextControlFrame::SetSelectionRange layout/forms/nsTextControlFrame.cpp:958 3 xul.dll RestoreSelectionState::Run content/html/content/src/nsTextEditorState.cpp:95 4 xul.dll nsContentUtils::AddScriptRunner 5 xul.dll PrepareEditorEvent::Run content/html/content/src/nsTextEditorState.cpp:1040 6 xul.dll nsContentUtils::RemoveScriptBlocker content/base/src/nsContentUtils.cpp:4722 7 xul.dll nsDocument::EndUpdate content/base/src/nsDocument.cpp:3969 8 xul.dll nsXULDocument::EndUpdate content/xul/document/src/nsXULDocument.cpp:3330 9 xul.dll mozAutoDocUpdate::~mozAutoDocUpdate content/base/src/mozAutoDocUpdate.h:66 10 xul.dll nsGenericElement::SetAttrAndNotify content/base/src/nsGenericElement.cpp:4725 11 xul.dll nsGenericElement::SetAttr content/base/src/nsGenericElement.cpp:4623 12 xul.dll nsGenericElement::SetAttribute content/base/src/nsGenericElement.cpp:2386 13 xul.dll nsIDOMElement_SetAttribute obj-firefox/js/src/xpconnect/src/dom_quickstubs.cpp:4853 ...

Confirmed on Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b10pre) Gecko/20110114 Firefox/4.0b10pre ID:20110114030359 Browser crashes with crash reports bp-ba3664d7-c94d-4ea4-9135-8e28a2110115 [str] 1. Start Minefield with clean profile 2. Alt > View > Toolbars > Check Menu Bar(disable auto hide) 3. Alt > View > Toolbars > Customize… 4. Drag and drop Location Bar to Menu Bar 5. Alt > View > Toolbars > Un-check Menu Bar(enable auto hide) [actual] Browser crashes with crash reports Regression window(cached hourly): Works: http://hg.mozilla.org/mozilla-central/rev/2894886924fc Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b10pre) Gecko/20110113 Firefox/4.0b10pre ID:20110113161101 Fails: http://hg.mozilla.org/mozilla-central/rev/5efe5f98ac13 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b10pre) Gecko/20110113 Firefox/4.0b10pre ID:20110113162644 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=2894886924fc&tochange=5efe5f98ac13

Bug 602331 added a flush in nsTypedSelection::AddRange so consumers needs to check for frame destruction etc. This should fix the crash in nsTextControlFrame::SetSelectionInternal. We used to flush at an even lower level, in nsTypedSelection::selectFrames, but bug 527306 removed that 2009-11-15. I'm checking the other call sites...

How does this patch fix the crash? We're holding a strong reference to selCon so we couldn't crash dereferencing it.

'selCon' in SetSelectionInternal is just a raw pointer. It's owned by nsTextEditorState::mSelCon which is owned by nsHTMLInputElement, but destroying the frame calls nsTextEditorState::UnbindFromFrame which sets mSelCon to null. http://mxr.mozilla.org/mozilla-central/source/content/html/content/src/nsTextEditorState.cpp#1451 So I think we could fix it by just making 'selCon' a strong ref locally in SetSelectionInternal and trusting its ScrollSelectionIntoView to do the right thing when the frame is gone (which it looks like it's doing)...

Let's do that!

FTR, I did verify the above in GDB just to be sure. After making 'selCon' strong, the refcount is 1 in ScrollSelectionIntoView. It does an early return NS_ERROR_FAILURE when 'mFrameSelection' is null. mFrameSelection is set to null in nsTextInputSelectionImpl::SetScrollableFrame (called from UnbindFromFrame, see link in comment above). So this all looks good, except maybe that SetSelectionInternal returns what ScrollSelectionIntoView returns (NS_ERROR_FAILURE).

Additionally, adding a frame destruction check to SetFormProperty. SetFormProperty SelectAllOrCollapseToEndOfText SetSelectionInternal

Comment on attachment 504355 [details] [diff] [review] Patch v2 I don't like this patch because of two reasons: 1. We're introducing failures stemming from SetSelectionInternal which is a web visible API, which means that our APIs might start to fail in cases which used to not fail before. 2. We're relying on ScrollSelectionIntoView for a detached selection controller object to do the right thing, always. I think a better fix would be to just regrab the selection controller after the AddRange call. BTW, I think we can use the testcase in bug 626288 as a crashtest here. It would be useful to run the testcase under valgrind to make sure that we're catching all of the loose ends here.

Agreed, this is a better fix. I'll land the test in bug 626288 together with the fix (not included in the patch to protect our nightly testers). The test passed valgrind testing on Linux64 (all crashtests passed btw).

Comment on attachment 504603 [details] [diff] [review] Patch v3 This is great! But please add a comment explaining why we're grabbing the selection controller again. Thanks!

(In reply to comment #12) > I'll land the test in bug 626288 together with the fix (not included in the > patch to protect our nightly testers). Good choice. > The test passed valgrind testing on Linux64 (all crashtests passed btw). Thanks for testing it under valgrind. This makes me feel better! :-)

Landed with the additional code comment and crashtest: http://hg.mozilla.org/mozilla-central/rev/65844bb7d0c5 http://hg.mozilla.org/mozilla-central/rev/8b5fb8ac2125