athena: Expose the list of activities from the ActivityManager.

athena: Expose the list of activities from the ActivityManager.

Notable implementation details of the change:
. Have ActivityManagerImpl observer each aura::Window of the activities, and
  update order of the activities when their windows are restacked.
. Introduce ActivityManagerObserver::OnActivityOrderChanged().
. Remove the code from ResourceManagerImpl that tracks the activity order, and
  replace it to get the list from ActivityManager instead.
  - Make sure the code traverses the activities in the correct order.

BUG=413928
R=oshima@chromium.org, skuhne@chromium.org

Committed: https://chromium.googlesource.com/chromium/src/+/0167b4868e070224bc1a0ff10039ce9df9107340

[sadrul, 2014-11-03 22:39:54]

sadrul@chromium.org changed reviewers:
+ oshima@chromium.org, skuhne@chromium.org

[sadrul, 2014-11-03 22:39:55]

+skuhne@ mostly for the change in resource_manager_impl.cc

+oshima@ for all of the changes.

[Mr4D (OOO till 08-26), 2014-11-03 23:48:00]

lgtm!

Note: The internal state changes will need to change once more logic will get
added, but for the time being this looks good!

https://codereview.chromium.org/694353004/diff/1/athena/resource_manager/reso...
File athena/resource_manager/resource_manager_impl.cc (right):

https://codereview.chromium.org/694353004/diff/1/athena/resource_manager/reso...
athena/resource_manager/resource_manager_impl.cc:348:
++unloadable_activity_count;
For the time being this is ok.
However - note that we might consider a more aggressive release once we can test
this and see a real "resource crunch" happening. Also - note that as soon as we
add the other application states this will also have to come back.

[Mr4D (OOO till 08-26), 2014-11-03 23:49:36]

[I have replied to the resource manager change and I looked at the core change
in activity_manager_impl.cc which creates the list itself.]

[oshima, 2014-11-04 00:26:03]

https://codereview.chromium.org/694353004/diff/1/athena/activity/activity_man...
File athena/activity/activity_manager_impl.cc (right):

https://codereview.chromium.org/694353004/diff/1/athena/activity/activity_man...
athena/activity/activity_manager_impl.cc:132: aura::Window* lost_active) {
can you add test for this?

https://codereview.chromium.org/694353004/diff/1/athena/activity/public/activ...
File athena/activity/public/activity_manager.h (right):

https://codereview.chromium.org/694353004/diff/1/athena/activity/public/activ...
athena/activity/public/activity_manager.h:37: virtual void
UpdateActivity(Activity* activity) = 0;
off topic: I think we should get rid of this and replace with better mechanism.
WDTY?

https://codereview.chromium.org/694353004/diff/1/athena/activity/public/activ...
athena/activity/public/activity_manager.h:41: virtual const ActivityList&
GetActivities() = 0;
I was just thinking something similiar for different purpose, thanks!

Would you mind using GetActivityList to emphasize it's ordered and also it
matches the type.

[sadrul, 2014-11-04 03:43:46]

https://codereview.chromium.org/694353004/diff/1/athena/activity/activity_man...
File athena/activity/activity_manager_impl.cc (right):

https://codereview.chromium.org/694353004/diff/1/athena/activity/activity_man...
athena/activity/activity_manager_impl.cc:132: aura::Window* lost_active) {
On 2014/11/04 00:26:03, oshima wrote:
> can you add test for this?

Done.

https://codereview.chromium.org/694353004/diff/1/athena/activity/public/activ...
File athena/activity/public/activity_manager.h (right):

https://codereview.chromium.org/694353004/diff/1/athena/activity/public/activ...
athena/activity/public/activity_manager.h:37: virtual void
UpdateActivity(Activity* activity) = 0;
On 2014/11/04 00:26:03, oshima wrote:
> off topic: I think we should get rid of this and replace with better
mechanism.
> WDTY?

One option is to introduce an ActivityViewModelObserver interface that notifies
of title/icon/color changes, and the ActivityManagerImpl installs itself as an
observer to all the activity-models in the activity-list.

https://codereview.chromium.org/694353004/diff/1/athena/activity/public/activ...
athena/activity/public/activity_manager.h:41: virtual const ActivityList&
GetActivities() = 0;
On 2014/11/04 00:26:03, oshima wrote:
> I was just thinking something similiar for different purpose, thanks!
> 
> Would you mind using GetActivityList to emphasize it's ordered and also it
> matches the type.

Done.

https://codereview.chromium.org/694353004/diff/1/athena/resource_manager/reso...
File athena/resource_manager/resource_manager_impl.cc (right):

https://codereview.chromium.org/694353004/diff/1/athena/resource_manager/reso...
athena/resource_manager/resource_manager_impl.cc:348:
++unloadable_activity_count;
On 2014/11/03 23:48:00, Mr4D wrote:
> For the time being this is ok.
> However - note that we might consider a more aggressive release once we can
test
> this and see a real "resource crunch" happening. Also - note that as soon as
we
> add the other application states this will also have to come back.

Acknowledged.

[oshima, 2014-11-04 04:25:11]

lgtm

[commit-bot: I haz the power, 2014-11-04 04:52:41]

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

[sadrul, 2014-11-04 04:52:46]

Committed patchset #2 (id:20001) manually as
0167b4868e070224bc1a0ff10039ce9df9107340 (presubmit successful).

[Mr4D (OOO till 08-26), 2014-11-11 00:44:25]

Since our Trybots are not in the main waterfall you should run your patch at
least locally against the browser / unit tests.

After debugging now for hours I think that this must be the cause of the crash
which removed the trybots from the CQ again.

Reason: The ActivityProxy is after the creation in front of the
ActivityManager's list - even though the window is behind the still existing
AppWindow.

Will create a patch to fix this.