Add ChildMemoryCoordinator::PurgeMemory()

Add ChildMemoryCoordinator::PurgeMemory()

In crrev.com/2655083003 we added OnPurgeMemory() to
MemoryCoordinatorClient and we are moving purging logic from
OnMemoryStateChange() to OnPurgeMemory(). To make this transition easy,
this CL adds PurgeMemory() to ChildMemoryCoordinator. This method calls
both OnPurgeMemory() and OnMemoryStateChange(SUSPENDED). After purging
logic in all clients move to OnPurgeMemory(), we can remove
OnMemoryStateChange() call.

BUG=684287
Review-Url: https://codereview.chromium.org/2669323002
Cr-Commit-Position: refs/heads/master@{#448475}
Committed: https://chromium.googlesource.com/chromium/src/+/110841cafec3a76ab7446dc7c59107572a2fc68f

[bashi, 2017-02-02 19:43:59]

The CQ bit was checked by bashi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-02-02 19:44:29]

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

[bashi, 2017-02-02 19:46:59]

bashi@chromium.org changed reviewers:
+ tasak@google.com

[bashi, 2017-02-02 19:47:00]

tasak@: PTAL before asking owners review?

https://codereview.chromium.org/2669323002/diff/1/content/renderer/render_thr...
File content/renderer/render_thread_impl.cc (left):

https://codereview.chromium.org/2669323002/diff/1/content/renderer/render_thr...
content/renderer/render_thread_impl.cc:1673: // TODO(tasak): After enabling
MemoryCoordinator, remove this Notify
I think we can remove this comment because we could enable kPurgeAndSuspended by
default without stabilizing memory coordinator.

[commit-bot: I haz the power, 2017-02-02 20:48:54]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-02-02 20:48:55]

Dry run: This issue passed the CQ dry run.

[tasak, 2017-02-03 04:09:33]

lgtm

https://codereview.chromium.org/2669323002/diff/1/content/renderer/render_thr...
File content/renderer/render_thread_impl.cc (left):

https://codereview.chromium.org/2669323002/diff/1/content/renderer/render_thr...
content/renderer/render_thread_impl.cc:1673: // TODO(tasak): After enabling
MemoryCoordinator, remove this Notify
On 2017/02/02 19:47:00, bashi wrote:
> I think we can remove this comment because we could enable kPurgeAndSuspended
by
> default without stabilizing memory coordinator.

I agree with you.

[bashi, 2017-02-03 04:30:28]

bashi@chromium.org changed reviewers:
+ haraken@chromium.org, jam@chromium.org

[bashi, 2017-02-03 04:30:29]

jam: could you review content/renderer/ ?
haraken: could you review content/child/memory ?

[haraken, 2017-02-03 04:32:26]

Do we have a test for ChildMemoryCoordinator::PurgeMemory()?

[bashi, 2017-02-03 04:37:24]

On 2017/02/03 04:32:26, haraken wrote:
> Do we have a test for ChildMemoryCoordinator::PurgeMemory()?

No. Should I add it even MemoryCoordinatorClientRegistry::PurgeMemory() and
MemoryCoordinatorClientRegistry::Notify() have tests? I'm happy to add tests.
What kind of test do you want to have? (I won't be able to ssh until next week
so I'll add tests next week).

[haraken, 2017-02-03 04:38:34]

On 2017/02/03 04:37:24, bashi wrote:
> On 2017/02/03 04:32:26, haraken wrote:
> > Do we have a test for ChildMemoryCoordinator::PurgeMemory()?
> 
> No. Should I add it even MemoryCoordinatorClientRegistry::PurgeMemory() and
> MemoryCoordinatorClientRegistry::Notify() have tests? I'm happy to add tests.
> What kind of test do you want to have? (I won't be able to ssh until next week
> so I'll add tests next week).

Ah, then fine. LGTM.

[jam, 2017-02-06 18:48:15]

lgtm

[bashi, 2017-02-06 23:16:20]

The CQ bit was checked by bashi@chromium.org

[bashi, 2017-02-06 23:16:20]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, tasak@google.com, jam@chromium.org
Link to the patchset: https://codereview.chromium.org/2669323002/#ps20001
(title: "rebase")

[commit-bot: I haz the power, 2017-02-06 23:17:21]

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, 2017-02-07 00:50:54]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1486422980086130,
"parent_rev": "6ce2738ac15a657f8716f6e837dfce3f9cdb8ee3", "commit_rev":
"110841cafec3a76ab7446dc7c59107572a2fc68f"}

[commit-bot: I haz the power, 2017-02-07 00:51:56]

Description was changed from

==========
Add ChildMemoryCoordinator::PurgeMemory()

In crrev.com/2655083003 we added OnPurgeMemory() to
MemoryCoordinatorClient and we are moving purging logic from
OnMemoryStateChange() to OnPurgeMemory(). To make this transition easy,
this CL adds PurgeMemory() to ChildMemoryCoordinator. This method calls
both OnPurgeMemory() and OnMemoryStateChange(SUSPENDED). After purging
logic in all clients move to OnPurgeMemory(), we can remove
OnMemoryStateChange() call.

BUG=684287
==========

to

==========
Add ChildMemoryCoordinator::PurgeMemory()

In crrev.com/2655083003 we added OnPurgeMemory() to
MemoryCoordinatorClient and we are moving purging logic from
OnMemoryStateChange() to OnPurgeMemory(). To make this transition easy,
this CL adds PurgeMemory() to ChildMemoryCoordinator. This method calls
both OnPurgeMemory() and OnMemoryStateChange(SUSPENDED). After purging
logic in all clients move to OnPurgeMemory(), we can remove
OnMemoryStateChange() call.

BUG=684287

Review-Url: https://codereview.chromium.org/2669323002
Cr-Commit-Position: refs/heads/master@{#448475}
Committed:
https://chromium.googlesource.com/chromium/src/+/110841cafec3a76ab7446dc7c591...
==========

[commit-bot: I haz the power, 2017-02-07 00:51:57]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/110841cafec3a76ab7446dc7c591...

[Wez, 2017-02-15 19:51:57]

wez@chromium.org changed reviewers:
+ wez@chromium.org

[Wez, 2017-02-15 19:51:58]

https://codereview.chromium.org/2669323002/diff/20001/content/renderer/render...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/2669323002/diff/20001/content/renderer/render...
content/renderer/render_thread_impl.cc:1673: memory_coordinator_->PurgeMemory();
Isn't |memory_coordinator_| only initialized if MemoryCoordinator is enabled,
which is mutually exclusive with PurgeAndSuspend, according to the DCHECK at
line 826?

[Wez, 2017-02-15 21:41:03]

https://codereview.chromium.org/2669323002/diff/20001/content/renderer/render...
File content/renderer/render_thread_impl.cc (right):

https://codereview.chromium.org/2669323002/diff/20001/content/renderer/render...
content/renderer/render_thread_impl.cc:826:
DCHECK(!base::FeatureList::IsEnabled(features::kPurgeAndSuspend));
We have both MemoryCoordinator and PurgeAndSuspend enabled in experiments at 50%
- what prevents an instance getting both enabled at the same time?

[Wez, 2017-02-15 22:48:06]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/2693223003/ by wez@chromium.org.

The reason for reverting is: Appears to be the root cause of crbug.com/692509
(renderer crashes in RenderThreadImpl::OnProcessPurgeAndSuspend) - although the
timeline doesn't match up, the change to RenderThreadImpl appears incorrect by
inspection (see my comment in the original CL)..