Purge memory via MemoryCoordinatorClientRegistry when a backgrounded renderer is suspended.

Purge memory via MemoryCoordinatorClientRegistry when a backgrounded renderer is suspended.

- Added kPurgeAndSuspend to content feature list.
- If kPurgeAndSuspend is enabled, purge memory and suspend.
  Otherwise, do nothing except recording UMA.

- intent-to-implement-and-ship of background renderer's purge +
  suspend is
  https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
  https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJvI/edit?usp=sharing
- The feature is not enabled by default because
  purge-and-suspend-time is 0.

BUG=635419
Committed: https://crrev.com/6ff44039ad107d09e4abd791dbb2d61358657600
Cr-Commit-Position: refs/heads/master@{#430225}

[tasak, 2016-10-28 05:31:41]

The CQ bit was checked by tasak@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-28 05:31:48]

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

[tasak, 2016-10-28 05:34:27]

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

[tasak, 2016-10-28 05:34:28]

Would you review this CL?

[bashi, 2016-10-28 05:45:05]

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

https://codereview.chromium.org/2456073003/diff/1/content/renderer/render_thr...
content/renderer/render_thread_impl.cc:850:
base::FeatureList::IsEnabled(features::kPurgeAndSuspend)) {
Let's discuss offline how to enable MemoryCoordinatorClients for purge+suspend
experiment. I think that enabling ChildMemoryCoordinator without enabling
MemoryCoordinator in browser side may break many things.

[tasak, 2016-10-28 06:29:35]

Thank you for review.

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

https://codereview.chromium.org/2456073003/diff/1/content/renderer/render_thr...
content/renderer/render_thread_impl.cc:850:
base::FeatureList::IsEnabled(features::kPurgeAndSuspend)) {
On 2016/10/28 05:45:05, bashi1 wrote:
> Let's discuss offline how to enable MemoryCoordinatorClients for purge+suspend
> experiment. I think that enabling ChildMemoryCoordinator without enabling
> MemoryCoordinator in browser side may break many things.

According to the discussion, I removed
base::FeatureList::IsEnabled(kPurgeAndSuspend), because
MemoryCoordinatorClientRegistry::Notify works without MemoryCoordinator.
Instead, I added DCHECK(!base::FeatuerList::IsEnabled(kPurgeAndSuspsned) inside
the "if".

[tasak, 2016-10-28 06:31:32]

tasak@google.com changed reviewers:
+ avi@chromium.org, chrisha@chromium.org

[tasak, 2016-10-28 06:31:33]

Would you review this CL?

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

https://codereview.chromium.org/2456073003/diff/20001/content/renderer/render...
content/renderer/render_thread_impl.cc:1779: DCHECK(!is_renderer_suspended_);
This needs https://codereview.chromium.org/2462513002/ 's patch.

[bashi, 2016-10-28 06:35:29]

lgtm on my side.

BTW, do we still need kPurgeAndSuspendTime? Or should we use
kPurgeAndSuspendTime with this feature flag?

[tasak, 2016-10-28 06:55:51]

On 2016/10/28 06:35:29, bashi1 wrote:
> lgtm on my side.
> 
> BTW, do we still need kPurgeAndSuspendTime? Or should we use
> kPurgeAndSuspendTime with this feature flag?

I will remove kPurgeAndSuspendTime in another CL.
Instead, I will add "static constexpr base::TimeDelta kPurgeAndSuspendDuration =
base::TimeDelta::FromMinutes(30);".

[bashi, 2016-10-28 06:58:24]

On 2016/10/28 06:55:51, tasak wrote:
> On 2016/10/28 06:35:29, bashi1 wrote:
> > lgtm on my side.
> > 
> > BTW, do we still need kPurgeAndSuspendTime? Or should we use
> > kPurgeAndSuspendTime with this feature flag?
> 
> I will remove kPurgeAndSuspendTime in another CL.
> Instead, I will add "static constexpr base::TimeDelta kPurgeAndSuspendDuration
=
> base::TimeDelta::FromMinutes(30);".

I see. Thanks!

[haraken, 2016-10-28 07:48:42]

LGTM

[haraken, 2016-10-28 07:49:41]

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

https://codereview.chromium.org/2456073003/diff/20001/content/renderer/render...
content/renderer/render_thread_impl.cc:1905: base::MemoryState::NORMAL);

I'm not sure if this should be NORMAL. It could be THROTTLE but we can adjust
the heuristics in a follow-up.

[Avi (use Gerrit), 2016-10-28 14:03:59]

lgtm

stamp

[chrisha, 2016-11-01 21:24:38]

lgtm

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

https://codereview.chromium.org/2456073003/diff/20001/content/renderer/render...
content/renderer/render_thread_impl.cc:1905: base::MemoryState::NORMAL);
On 2016/10/28 07:49:41, haraken wrote:
> 
> I'm not sure if this should be NORMAL. It could be THROTTLE but we can adjust
> the heuristics in a follow-up.

Ditto. This seems fine for now.

[tasak, 2016-11-07 05:51:59]

The CQ bit was checked by tasak@google.com

[tasak, 2016-11-07 05:52:00]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, avi@chromium.org, chrisha@chromium.org, bashi@chromium.org
Link to the patchset: https://codereview.chromium.org/2456073003/#ps40001
(title: "Patch for landing")

[commit-bot: I haz the power, 2016-11-07 05:52:09]

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-11-07 06:45:52]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-07 06:48:11]

Description was changed from

==========
Purge memory via MemoryCoordinatorClientRegistry when a backgrounded renderer is
suspended.

- Added kPurgeAndSuspend to content feature list.
- If kPurgeAndSuspend is enabled, purge memory and suspend.
  Otherwise, do nothing except recording UMA.

- intent-to-implement-and-ship of background renderer's purge +
  suspend is
  https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
 
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...
- The feature is not enabled by default because
  purge-and-suspend-time is 0.

BUG=635419
==========

to

==========
Purge memory via MemoryCoordinatorClientRegistry when a backgrounded renderer is
suspended.

- Added kPurgeAndSuspend to content feature list.
- If kPurgeAndSuspend is enabled, purge memory and suspend.
  Otherwise, do nothing except recording UMA.

- intent-to-implement-and-ship of background renderer's purge +
  suspend is
  https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/DK189tnM8l4
- one of the documents attached in the above intent is
 
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...
- The feature is not enabled by default because
  purge-and-suspend-time is 0.

BUG=635419

Committed: https://crrev.com/6ff44039ad107d09e4abd791dbb2d61358657600
Cr-Commit-Position: refs/heads/master@{#430225}
==========

[commit-bot: I haz the power, 2016-11-07 06:48:12]

Patchset 3 (id:??) landed as
https://crrev.com/6ff44039ad107d09e4abd791dbb2d61358657600
Cr-Commit-Position: refs/heads/master@{#430225}