Fix a crash due to UserScriptListener unregistering notifications on the wrong

Fix a crash due to UserScriptListener unregistering notifications on the wrong
thread, then being sent a notification after it was deleted.

BUG=38374
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=42120

[Paweł Hajdan Jr., 2010-03-18 07:47:54]

http://codereview.chromium.org/995007/diff/1/3
File chrome/browser/extensions/user_script_listener.h (right):

http://codereview.chromium.org/995007/diff/1/3#newcode35
chrome/browser/extensions/user_script_listener.h:35: void ShutdownMainThread();
The object is constructed on the UI thread, and currently the dtor is empty. How
about using ChromeThread::DeleteOnUIThread, and doing the cleanup in the dtor?

[Matt Perry, 2010-03-18 18:00:55]

http://codereview.chromium.org/995007/diff/1/3
File chrome/browser/extensions/user_script_listener.h (right):

http://codereview.chromium.org/995007/diff/1/3#newcode35
chrome/browser/extensions/user_script_listener.h:35: void ShutdownMainThread();
On 2010/03/18 07:47:54, Paweł Hajdan Jr. wrote:
> The object is constructed on the UI thread, and currently the dtor is empty.
How
> about using ChromeThread::DeleteOnUIThread, and doing the cleanup in the dtor?

That doesn't work, because the object is deleted so late in the shutdown process
that the UI loop never has a chance to pump messages. We would just leak the
object.

[Paweł Hajdan Jr., 2010-03-19 07:42:50]

I think I don't know this code enough to do a good review then. Changing
reviewers.

[darin (slow to review), 2010-03-19 07:52:40]

On Thu, Mar 18, 2010 at 11:00 AM, <mpcomplete@chromium.org> wrote:

>
> http://codereview.chromium.org/995007/diff/1/3
> File chrome/browser/extensions/user_script_listener.h (right):
>
> http://codereview.chromium.org/995007/diff/1/3#newcode35
> chrome/browser/extensions/user_script_listener.h:35: void
> ShutdownMainThread();
> On 2010/03/18 07:47:54, Paweł Hajdan Jr. wrote:
>
>> The object is constructed on the UI thread, and currently the dtor is
>>
> empty. How
>
>> about using ChromeThread::DeleteOnUIThread, and doing the cleanup in
>>
> the dtor?
>
> That doesn't work, because the object is deleted so late in the shutdown
> process that the UI loop never has a chance to pump messages. We would
> just leak the object.


All background threads should be cleaned up before the UI thread, and maybe
we should call RunAllPending on the main thread at some opportune time.  It
seems a bit unfortunate that you cannot count on cycles on the UI thread.
 Of
course that could also lead to shutdown taking longer (if the queue is
long).

This change LGTM.

-Darin



>
>
> http://codereview.chromium.org/995007
>