Fix crash in WindowEventRouter::OnActiveWindowChanged when closing an incognito window.

Fix crash in WindowEventRouter::OnActiveWindowChanged when closing an incognito window.

When closing an incongito window, it is possible that the previously focused profile is already
destroyed. Since profile_->IsSameProfile(previous_focused_profile) is always true in this case, we
can just skip the check and dispatch the events against profile_.

BUG=139237
TEST=open an incognito window and close it and hope it doesn't crash.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=150656

[Yoyo Zhou, 2012-08-08 19:36:58]

http://codereview.chromium.org/10837170/diff/1/chrome/browser/extensions/wind...
File chrome/browser/extensions/window_event_router.cc (right):

http://codereview.chromium.org/10837170/diff/1/chrome/browser/extensions/wind...
chrome/browser/extensions/window_event_router.cc:154: if ((window_profile &&
!profile_->IsSameProfile(window_profile)))
excessive parentheses

http://codereview.chromium.org/10837170/diff/1/chrome/browser/extensions/wind...
chrome/browser/extensions/window_event_router.cc:157:
ExtensionSystem::Get(profile_)->event_router()->
This seems to be the important fix.

http://codereview.chromium.org/10837170/diff/1/chrome/browser/extensions/wind...
chrome/browser/extensions/window_event_router.cc:161: window_profile,
Are you sure this should be NULL in some cases?

[dcheng, 2012-08-08 20:05:56]

http://codereview.chromium.org/10837170/diff/1/chrome/browser/extensions/wind...
File chrome/browser/extensions/window_event_router.cc (right):

http://codereview.chromium.org/10837170/diff/1/chrome/browser/extensions/wind...
chrome/browser/extensions/window_event_router.cc:154: if ((window_profile &&
!profile_->IsSameProfile(window_profile)))
On 2012/08/08 19:36:58, Yoyo Zhou wrote:
> excessive parentheses

Already removed in my latest snapshot.

http://codereview.chromium.org/10837170/diff/1/chrome/browser/extensions/wind...
chrome/browser/extensions/window_event_router.cc:161: window_profile,
On 2012/08/08 19:36:58, Yoyo Zhou wrote:
> Are you sure this should be NULL in some cases?

Oops. While I think it's technically OK to pass NULL, it probably doesn't do
what we want...

I'm going to have to think about how to fix this properly...

[dcheng, 2012-08-08 22:08:33]

I've updated the CL a bit to trim out an if () check as well. I've confirmed
that this code still passes the test case you used in crbug.com/138901.

http://codereview.chromium.org/10837170/diff/1/chrome/browser/extensions/wind...
File chrome/browser/extensions/window_event_router.cc (right):

http://codereview.chromium.org/10837170/diff/1/chrome/browser/extensions/wind...
chrome/browser/extensions/window_event_router.cc:161: window_profile,
On 2012/08/08 20:05:56, dcheng wrote:
> On 2012/08/08 19:36:58, Yoyo Zhou wrote:
> > Are you sure this should be NULL in some cases?
> 
> Oops. While I think it's technically OK to pass NULL, it probably doesn't do
> what we want...
> 
> I'm going to have to think about how to fix this properly...

Passing NULL won't cause any crashes. However, I think it may cause a few extra
callbacks with window_id = WINDOW_ID_NONE across incognito boundaries. This may
be a slight regression, but I don't have a test case to prove my theory one way
or another. Theoretically, the proper fix for crbug.com/141474 will address this
case too...

[Yoyo Zhou, 2012-08-08 22:11:38]

LGTM, fingers crossed

[commit-bot: I haz the power, 2012-08-08 22:12:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dcheng@chromium.org/10837170/1002

[commit-bot: I haz the power, 2012-08-08 22:40:09]

Try job failure for 10837170-1002 (retry) on android for steps "compile, build"
(clobber build).
It's a second try, previously, steps "compile, build" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android&nu...

[dcheng, 2012-08-09 00:39:11]

On 2012/08/08 22:40:09, I haz the power (commit-bot) wrote:
> Try job failure for 10837170-1002 (retry) on android for steps "compile,
build"
> (clobber build).
> It's a second try, previously, steps "compile, build" failed.
>
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android&nu...

I tested locally and the failing tests on mac_rel/linux_rel seem to be flakes. I
will be landing this once the tree opens.