395344 - Topcrash typing in the address bar [@ nsAutoCompleteController::HandleText]

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Jesse Ruderman]

http://crash-stats.mozilla.com/report/list?range_unit=weeks&query_search=signature&query_type=contains&version=Firefox%3A3.0a8pre&signature=nsAutoCompleteController%3A%3AHandleText(int)&range_value=1

Comment 1

[Jesse Ruderman]

CCing Seth, who has been hacking on autocomplete lately.

Comment 2

[(not reading, please use [e-mail] instead)]

will start by applying some wallpaper, but will work on figuring out how to make this crash.

Comment 3

[(not reading, please use [e-mail] instead)]

everything is going boom here:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp&rev=1.64&mark=225#225

mInput is appears to be null, but not sure why yet.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #3)
> http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cpp&rev=1.64&mark=225#225

> mInput is appears to be null, but not sure why yet.

Are you sure? We null check mInput at the top of that function...

Comment 5

[(not reading, please use [e-mail] instead)]

> Are you sure? We null check mInput at the top of that function...

From the crash-stats, if I'm reading the reports right, the windows crashes look like we are crashing at 0x0, but mac and linux look non-zero.

I'm thinking about some wallpaper to at least turn the crash (if mInput is null?) to at least turn crash into an assertion.

Comment 6

[(not reading, please use [e-mail] instead)]

the crash started showing on 8/26 (I love breakpad!).

I made a change to the autocomplete code on 8/24 that could be related, see bug #393244

we only have one caller to HandleText() from script, here:

http://lxr.mozilla.org/seamonkey/source/toolkit/content/widgets/autocomplete.xml#499

I'm going to call Components.utils.forceGC right before the call to handleText() to see if I can force a crash.

Comment 7

[(not reading, please use [e-mail] instead)]

> Components.utils.forceGC

no luck with that approach.

Comment 8

[(not reading, please use [e-mail] instead)]

> the crash started showing on 8/26

I checked in on 2007-08-24 11:40.

there was one crash on 2007082404 and then more on 2007082405

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

If I read the code correctly:
Before GetDisableAutoComplete() there is call to StopSearch() which 
may lead to call PostSearchCleanup() which again may call onSearchComplete, which
apparently can be implemented in JS/XBL too: 
http://lxr.mozilla.org/seamonkey/source/toolkit/content/widgets/autocomplete.xml#207
Which means anything can happen there; tearing down the document, modifying DOM ...
So null check in the beginning of the method is probably not enough, since
it can become null later.

Comment 10

[Marcia Knous [:marcia]]

I hit this today, but I believe I was typing in the search bar. http://crash-stats.mozilla.com/report/index/3cfdeecf-6612-11dc-bed1-001a4bd43ef6?date=2007-09-18-18 is my CR.

Comment 11

[(not reading, please use [e-mail] instead)]

didn't make m8, moving to m9.  will investigate smaug's comment #9.

Comment 12

[(not reading, please use [e-mail] instead)]

Thanks to a tip from marcia, I've been able to reproduce this on the mac in the search box.

I'm doing:  "command k, type something, hit enter" over and over until I crash.

It looks like our blur event handler is getting called during PostSearchCleanup().

our blur handler is calling detachController(), which sets the input to null).

Comment 13

[(not reading, please use [e-mail] instead)]

after more debugging, here's what I've got:

nsAutoCompleteController::HandleText() is called and mInput is not null.

StopSearch() stops all searches, and if our mSearchStatus is STATUS_SEARCHING, we will call PostSearchCleanup()

PostSearchCleanup() will call EnterMatch() which will call onTextEntered() (in search.xml)

onTextEntered() calls handleSearchCommand(), which calls doSearch() which calls openUILinkIn() which calls focusElement() which will blur the focus away from the search box.

our blur handler in autocomplete.xml will call detatchController() which will null the controllers input.

then, we continue on in HandleText() but mInput is null, so we'll derefence a null pointer.

for future reference, here's the stack (in js) from

onxblblur([object Event])@chrome://global/content/bindings/autocomplete.xml:468
focus()@:0
XPCNativeWrapper function wrapper()@:0
focusElement([object XPCNativeWrapper])@chrome://browser/content/utilityOverlay.js:63
openUILinkIn("http://www.google.com/search?q=adfadsf+&ie=utf-8&oe=utf-8&aq=t&rls=org.mozilla:en-US:unofficial&client=firefox-a","current",null,null)@chrome://browser/content/utilityOverlay.js:196
doSearch("adfadsf ","current")@chrome://browser/content/search/search.xml:471
handleSearchCommand([object KeyboardEvent])@chrome://browser/content/search/search.xml:461
onTextEntered()@chrome://browser/content/search/search.xml:660
handleText(false)@:0
onxblinput([object Event])@chrome://global/content/bindings/autocomplete.xml:453

Comment 14

[(not reading, please use [e-mail] instead)]

note to gavin, I added the call to PostSearchCleanup() in StopSearch() for bug #393244

For this bug, I propose that we (at least) check mInput for after the call to StopSearch() in nsAutoCompleteController::HandleText(), like we are doing for bug #193544

I've tested this, and I no longer crash.

Comment 15

[(not reading, please use [e-mail] instead)]

Attachment: patch

Comment 16

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 281429 [details] [diff] [review]
patch

(In reply to comment #13)
> PostSearchCleanup() will call EnterMatch() which will call onTextEntered() (in
> search.xml)

PostSearchCleanup() will only call EnterMatch() if mEnterAfterSearch is true. I'm not sure I understand how mEnterAfterSearch can be true when HandleText() is called. I guess it could happen if you press enter in the textbox and then quickly type something else, before the result has been loaded? Does that match your steps to reproduce?

r=me because this is the right thing to do when we get into this situation (instead of crashing), but I'd like to try and understand how we get here in case something else is going wrong.

Comment 17

[(not reading, please use [e-mail] instead)]

> I guess it could happen if you press enter in the textbox and then
> quickly type something else, before the result has been loaded? Does that match
> your steps to reproduce?

That does match my steps my reproduce this, at least the pressing enter and quickly typing, which I was doing over and over.

Comment 18

[(not reading, please use [e-mail] instead)]

fixed.

Checking in nsAutoCompleteController.cpp;
/cvsroot/mozilla/toolkit/components/autocomplete/src/nsAutoCompleteController.cp
p,v  <--  nsAutoCompleteController.cpp
new revision: 1.65; previous revision: 1.64
done

I'll log a spin off bug on the related issue (in comment #16) that is not fully understood.

Comment 19

[(not reading, please use [e-mail] instead)]

> I'll log a spin off bug on the related issue

see bug #396780

thanks to smaug, gavin and marcia on this one.

Comment 21

[(not currently active) Ted Mielczarek]

I crashed with a slightly different signature, I'm not sure if it's fixed by this patch:
http://crash-stats.mozilla.com/report/index/6e4e486e-6797-11dc-9336-001a4bd43ed6?date=2007-09-20-16

[@ nsAutoCompleteController::CompleteDefaultIndex ]

Comment 22

[(not reading, please use [e-mail] instead)]

Ted, that looks like something else.  I'll start a new bug on that, with some stack info from crash-stats.

Comment 23

[(not reading, please use [e-mail] instead)]

> I'll start a new bug on that

see bug #396925

Comment 24

[(not reading, please use [e-mail] instead)]

note, if you look at http://crash-stats.mozilla.com/report/list?range_unit=months&query_search=signature&query_type=contains&version=Firefox%3A3.0a8pre&signature=nsAutoCompleteController%3A%3AHandleText(int)&range_value=1, we aren't seeing this crash from any builds after 9/19.  

(we are getting reports, but they are from old builds.)

Comment 25

[Stephen Donner [:stephend] Not actively reading bugmail]

Using the steps in comment 12, I can no longer crash using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a9pre) Gecko/2007100504 Minefield/3.0a9pre

Also, I double-checked Seth's handy URL in comment 24, and verified that there are no crashes in this stack starting from 9/20/2007 on.

Verified FIXED

Comment 26

[(not reading, please use [e-mail] instead)]

I queried again, just to make sure, and we're still good:

http://crash-stats.mozilla.com/report/list?range_unit=weeks&query_search=signature&query_type=contains&product=Firefox&signature=nsAutoCompleteController%3A%3AHandleText(int)&query=nsAutoCompleteController%3A%3AHandleText&range_value=1