arc: Fix crash on window close (on close race condition).

arc: Fix crash on window close (on close race condition).

This handles race condition on window closing.

TEST=Extended browser_tests
TEST=Manually on device
TEST=test_that -b veyron_minnie <IP> cheets_Notification
BUG=638656
BUG=638261
Committed: https://crrev.com/800987eb69027f6f34dff840b19c68a83065c6ae
Cr-Commit-Position: refs/heads/master@{#412940}

[khmel, 2016-08-18 16:09:28]

khmel@chromium.org changed reviewers:
+ skuhne@chromium.org

[khmel, 2016-08-18 16:09:29]

Hi Stefan,

PTAL

https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
File chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
(right):

https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc:336:
OnTaskDestroyed(it.second->task_id());
Fix race condition on Close (Close event before OnTaskDestroyed)

https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc
(right):

https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc:849:
exo::ShellSurface::SetApplicationId(widget->GetNativeWindow(),
We use visibility event to activate Arc window binding.

[khmel, 2016-08-18 16:17:49]

Description was changed from

==========
arc: Fix crash on window close.

This handles race condition on window closing.

TEST=Extended browser_tests
TEST=Manually on device
TEST=test_that -b veyron_minnie <IP> cheets_Notification
BUG=638656
BUG=638261
==========

to

==========
arc: Fix crash on window close (on close race condition).

This handles race condition on window closing.

TEST=Extended browser_tests
TEST=Manually on device
TEST=test_that -b veyron_minnie <IP> cheets_Notification
BUG=638656
BUG=638261
==========

[James Cook, 2016-08-18 20:43:09]

On 2016/08/18 16:09:29, khmel wrote:
> Hi Stefan,
> 
> PTAL
> 
>
https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
> File chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc
> (right):
> 
>
https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
> chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc:336:
> OnTaskDestroyed(it.second->task_id());
> Fix race condition on Close (Close event before OnTaskDestroyed)
> 
>
https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
> File
chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc
> (right):
> 
>
https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
>
chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc:849:
> exo::ShellSurface::SetApplicationId(widget->GetNativeWindow(),
> We use visibility event to activate Arc window binding.

Ping skuhne -- I'd like to get a fix for this landed before 5 PM, since that's
when our Chrome build for the PFQ kicks off.

[Mr4D (OOO till 08-26), 2016-08-18 21:00:12]

lgtm

https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc
(right):

https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc:849:
exo::ShellSurface::SetApplicationId(widget->GetNativeWindow(),
You might want to add a comment for that as well? Just to make sure that no one
sorts these calls in a different way. ( ;) )

[khmel, 2016-08-18 21:05:12]

Thanks Stefan

https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
File chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc
(right):

https://codereview.chromium.org/2256273003/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/chrome_launcher_controller_impl_unittest.cc:849:
exo::ShellSurface::SetApplicationId(widget->GetNativeWindow(),
On 2016/08/18 21:00:12, Mr4D wrote:
> You might want to add a comment for that as well? Just to make sure that no
one
> sorts these calls in a different way. ( ;) )

Agree, this is not clear :)

[khmel, 2016-08-18 21:05:28]

The CQ bit was checked by khmel@chromium.org

[khmel, 2016-08-18 21:05:29]

The patchset sent to the CQ was uploaded after l-g-t-m from skuhne@chromium.org
Link to the patchset: https://codereview.chromium.org/2256273003/#ps20001
(title: "comment added")

[commit-bot: I haz the power, 2016-08-18 21:06:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-18 21:47:04]

Description was changed from

==========
arc: Fix crash on window close (on close race condition).

This handles race condition on window closing.

TEST=Extended browser_tests
TEST=Manually on device
TEST=test_that -b veyron_minnie <IP> cheets_Notification
BUG=638656
BUG=638261
==========

to

==========
arc: Fix crash on window close (on close race condition).

This handles race condition on window closing.

TEST=Extended browser_tests
TEST=Manually on device
TEST=test_that -b veyron_minnie <IP> cheets_Notification
BUG=638656
BUG=638261
==========

[commit-bot: I haz the power, 2016-08-18 21:47:05]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-08-18 21:54:02]

Description was changed from

==========
arc: Fix crash on window close (on close race condition).

This handles race condition on window closing.

TEST=Extended browser_tests
TEST=Manually on device
TEST=test_that -b veyron_minnie <IP> cheets_Notification
BUG=638656
BUG=638261
==========

to

==========
arc: Fix crash on window close (on close race condition).

This handles race condition on window closing.

TEST=Extended browser_tests
TEST=Manually on device
TEST=test_that -b veyron_minnie <IP> cheets_Notification
BUG=638656
BUG=638261

Committed: https://crrev.com/800987eb69027f6f34dff840b19c68a83065c6ae
Cr-Commit-Position: refs/heads/master@{#412940}
==========

[commit-bot: I haz the power, 2016-08-18 21:54:04]

Patchset 2 (id:??) landed as
https://crrev.com/800987eb69027f6f34dff840b19c68a83065c6ae
Cr-Commit-Position: refs/heads/master@{#412940}

[James Cook, 2016-08-18 22:22:59]

On 2016/08/18 21:54:04, commit-bot: I haz the power wrote:
> Patchset 2 (id:??) landed as
> https://crrev.com/800987eb69027f6f34dff840b19c68a83065c6ae
> Cr-Commit-Position: refs/heads/master@{#412940}

Thanks, guys.