athena: Use ActivityList where appropriate, and fix AppActivity tests.

athena: Use ActivityList where appropriate, and fix AppActivity tests.

. Introduce ActivityManager::ReplaceActivity to swap out an activity with
  another activity (e.g. when replacing an AppActivity with a proxy).

. Have AppActivityRegistry use the ActivityList to get the MRU activity
  belonging to the same app (instead of WindowListProvider).

. TestAppActivity was overriding all of AppActivity, so the tests weren't
  actually testing AppActivity. So update TestAppActivity to override only the
  bare minimum of AppActivity.

BUG=430030

[sadrul, 2014-11-05 22:15:50]

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

[Mr4D (OOO till 08-26), 2014-11-06 04:33:00]

My general comment to this CL is that this was my initial idea of how to do
this.
However after several long discussions with Oshima we decided to go with the
WindowListProvider approach.
As such we have spent a lot of time to get that working and I wonder now why we
are rewriting this again.
Furthermore I do not quite understand what that issue has to do with this CL.

Oshima - what is your opinion about this?

[sadrul, 2014-11-06 15:13:50]

> My general comment to this CL is that this was my initial idea of how to do
> this.

Yes. I think it's most natural for activities/activity-registry to work in terms
of just activities, instead of having to manipulate both activities and windows
(which makes this more complex than necessary). There should be a separate
entity that is responsible for keeping the activity-list and the window-list
in-sync.

> However after several long discussions with Oshima we decided to go with the
> WindowListProvider approach.
> As such we have spent a lot of time to get that working and I wonder now why
we
> are rewriting this again.

I can't comment on the discussion, but this is a relatively small amount of
code. Fixing/Rewriting it to do the simpler/better thing is good.

> Furthermore I do not quite understand what that issue has to do with this CL.

The app-activity etc. code takes care to update the ordering of the windows, but
not the ordering of the activities. So code that operates on the list of
activities do the wrong thing (see hashimoto@'s analysis:
https://code.google.com/p/chromium/issues/detail?id=430030#c3). The right thing
to do here is to have the relevant code worry only about the activities.

[oshima, 2014-11-06 15:47:28]

On 2014/11/06 15:13:50, sadrul wrote:
> > My general comment to this CL is that this was my initial idea of how to do
> > this.
> 
> Yes. I think it's most natural for activities/activity-registry to work in
terms
> of just activities, instead of having to manipulate both activities and
windows
> (which makes this more complex than necessary). There should be a separate
> entity that is responsible for keeping the activity-list and the window-list
> in-sync.

I haven't looked through the change yet, but I understand that this *can* makes
sense.
It really depends on if it makes the design cleaner.

> 
> > However after several long discussions with Oshima we decided to go with the
> > WindowListProvider approach.
> > As such we have spent a lot of time to get that working and I wonder now why
> we
> > are rewriting this again.
> 
> I can't comment on the discussion, but this is a relatively small amount of
> code. Fixing/Rewriting it to do the simpler/better thing is good.

What I wanted to change in skuhen@'s original CL was that it was
that it using window->parent()->children(), which is fragile.

I'll review this when I'm in.

> 
> > Furthermore I do not quite understand what that issue has to do with this
CL.
> 
> The app-activity etc. code takes care to update the ordering of the windows,
but
> not the ordering of the activities. So code that operates on the list of
> activities do the wrong thing (see hashimoto@'s analysis:
> https://code.google.com/p/chromium/issues/detail?id=430030#c3). The right
thing
> to do here is to have the relevant code worry only about the activities.

[Mr4D (OOO till 08-26), 2014-11-17 14:54:20]

I think we do not need this anymore now anyways.

[Mr4D (OOO till 08-26), 2017-01-30 16:32:27]

Athena is obsolete. Could you please close this bug?

[sadrul, 2017-01-30 16:59:05]

Description was changed from

==========
athena: Use ActivityList where appropriate, and fix AppActivity tests.

. Introduce ActivityManager::ReplaceActivity to swap out an activity with
  another activity (e.g. when replacing an AppActivity with a proxy).

. Have AppActivityRegistry use the ActivityList to get the MRU activity
  belonging to the same app (instead of WindowListProvider).

. TestAppActivity was overriding all of AppActivity, so the tests weren't
  actually testing AppActivity. So update TestAppActivity to override only the
  bare minimum of AppActivity.

BUG=430030
==========

to

==========
athena: Use ActivityList where appropriate, and fix AppActivity tests.

. Introduce ActivityManager::ReplaceActivity to swap out an activity with
  another activity (e.g. when replacing an AppActivity with a proxy).

. Have AppActivityRegistry use the ActivityList to get the MRU activity
  belonging to the same app (instead of WindowListProvider).

. TestAppActivity was overriding all of AppActivity, so the tests weren't
  actually testing AppActivity. So update TestAppActivity to override only the
  bare minimum of AppActivity.

BUG=430030
==========

[sadrul, 2017-01-30 16:59:06]

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