arc: Fix crash in multi-user env on task stop.

arc: Fix crash in multi-user env on task stop.

This CL makes it safe to handle Arc events for shelf in case secondary
user is active. It also restores active status when primary user is
activated again.

BUG=b/31316824
BUG=b/31313336
BUG=644053
TEST=Manually on device. When secondary user is active, stop one of
     the Android task. There is no more crash in this case. Switch
     back to primary account and active status is correctly restored
     for the latest Arc app.

Committed: https://crrev.com/78c98597ace74cec2375e39d5cbf8a8424bbc3ba
Cr-Commit-Position: refs/heads/master@{#417076}

[khmel, 2016-09-06 20:46:12]

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

[khmel, 2016-09-06 20:46:13]

Hi Stefan,

PTAL

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

https://codereview.chromium.org/2315903002/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc:277:
task_id_to_app_window_.clear();
It is not safe to keep mapping to AppWindow when controllers are destroyed.
Solution is to recreate shelf items from existing trackied windows.

[Mr4D (OOO till 08-26), 2016-09-07 20:17:34]

Please see comments. Then lgtm.

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

https://codereview.chromium.org/2315903002/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc:277:
task_id_to_app_window_.clear();
Can you please add a comment here to state this?

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

https://codereview.chromium.org/2315903002/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.h:102: void
MaybeAttachContollerToWindow(aura::Window* window);
Why rename "May" -> "Maybe"? That sounds really odd. Can you please keep the
name - or - maybe name it "attachControllerToWindowIfNeeded"?

[khmel, 2016-09-07 21:01:48]

Thanks for review!

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

https://codereview.chromium.org/2315903002/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.cc:277:
task_id_to_app_window_.clear();
On 2016/09/07 20:17:34, Mr4D wrote:
> Can you please add a comment here to state this?

Done.

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

https://codereview.chromium.org/2315903002/diff/1/chrome/browser/ui/ash/launc...
chrome/browser/ui/ash/launcher/arc_app_window_launcher_controller.h:102: void
MaybeAttachContollerToWindow(aura::Window* window);
On 2016/09/07 20:17:34, Mr4D wrote:
> Why rename "May" -> "Maybe"? That sounds really odd. Can you please keep the
> name - or - maybe name it "attachControllerToWindowIfNeeded"?

Based on my previous reviews :) There is a style to name like this. I don't mind
to rename to attachControllerToWindowIfNeeded in the code under your guidance :)

[khmel, 2016-09-07 21:02:14]

The CQ bit was checked by khmel@chromium.org

[khmel, 2016-09-07 21:02:14]

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/2315903002/#ps20001
(title: "rebase + comments")

[commit-bot: I haz the power, 2016-09-07 21:03:08]

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-09-07 22:07:47]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-07 22:09:54]

Description was changed from

==========
arc: Fix crash in multi-user env on task stop.

This CL makes it safe to handle Arc events for shelf in case secondary
user is active. It also restores active status when primary user is
activated again.

BUG=b/31316824
BUG=b/31313336
BUG=644053
TEST=Manually on device. When secondary user is active, stop one of
     the Android task. There is no more crash in this case. Switch
     back to primary account and active status is correctly restored
     for the latest Arc app.
==========

to

==========
arc: Fix crash in multi-user env on task stop.

This CL makes it safe to handle Arc events for shelf in case secondary
user is active. It also restores active status when primary user is
activated again.

BUG=b/31316824
BUG=b/31313336
BUG=644053
TEST=Manually on device. When secondary user is active, stop one of
     the Android task. There is no more crash in this case. Switch
     back to primary account and active status is correctly restored
     for the latest Arc app.

Committed: https://crrev.com/78c98597ace74cec2375e39d5cbf8a8424bbc3ba
Cr-Commit-Position: refs/heads/master@{#417076}
==========

[commit-bot: I haz the power, 2016-09-07 22:09:54]

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