memory coordinator: Introduce MemoryCoordinatorImpl::Policy

memory coordinator: Introduce MemoryCoordinatorImpl::Policy

Proper actions under memory pressure can vary on different
systems. Use a strategy pattern to make policies interchangeable.

This CL just moves the current heuristics to
MemoryCoordinatorDefaultPolicy.
 
Design doc: https://docs.google.com/document/d/1rGeUzhfzIKwmBzUcqUEwxEpLQQRq3PulRuHwFBlVIYI/edit#heading=h.12ebay6zfsdw

BUG=696844
Review-Url: https://codereview.chromium.org/2867353002
Cr-Commit-Position: refs/heads/master@{#471260}
Committed: https://chromium.googlesource.com/chromium/src/+/c5106fdfb45ca1c85f5018dfbf0989d3c97db323

[bashi, 2017-05-09 09:20:08]

Description was changed from

==========
memory coordinator: Introduce MemoryCoordinatorImpl::Policy

BUG=696844
==========

to

==========
memory coordinator: Introduce MemoryCoordinatorImpl::Policy

Appropriate actions under memory pressure can vary on different
systems. Use a strategy pattern to make policies interchangeable.

This CL just moves the current heuristics to
MemoryCoordinatorDefaultPolicy.
 
Design doc:
https://docs.google.com/document/d/1rGeUzhfzIKwmBzUcqUEwxEpLQQRq3PulRuHwFBlVI...

BUG=696844
==========

[bashi, 2017-05-09 09:29:57]

Description was changed from

==========
memory coordinator: Introduce MemoryCoordinatorImpl::Policy

Appropriate actions under memory pressure can vary on different
systems. Use a strategy pattern to make policies interchangeable.

This CL just moves the current heuristics to
MemoryCoordinatorDefaultPolicy.
 
Design doc:
https://docs.google.com/document/d/1rGeUzhfzIKwmBzUcqUEwxEpLQQRq3PulRuHwFBlVI...

BUG=696844
==========

to

==========
memory coordinator: Introduce MemoryCoordinatorImpl::Policy

Proper actions under memory pressure can vary on different
systems. Use a strategy pattern to make policies interchangeable.

This CL just moves the current heuristics to
MemoryCoordinatorDefaultPolicy.
 
Design doc:
https://docs.google.com/document/d/1rGeUzhfzIKwmBzUcqUEwxEpLQQRq3PulRuHwFBlVI...

BUG=696844
==========

[bashi, 2017-05-09 09:30:13]

bashi@chromium.org changed reviewers:
+ chrome-grc-reviews+memory@chromium.org, haraken@chromium.org

[bashi, 2017-05-09 09:30:14]

I'd like to get some feedback. WDYT? Any other good separation/abstraction?

[haraken, 2017-05-09 11:47:30]

The approach looks good. Is this ready for full review?

https://codereview.chromium.org/2867353002/diff/1/content/browser/memory/memo...
File content/browser/memory/memory_coordinator_default_policy.h (right):

https://codereview.chromium.org/2867353002/diff/1/content/browser/memory/memo...
content/browser/memory/memory_coordinator_default_policy.h:15:
MemoryCoordinatorDefaultPolicy(MemoryCoordinatorImpl* coordinator);

Add explicit.

[chrisha, 2017-05-09 18:53:00]

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

[chrisha, 2017-05-09 18:53:01]

+1 to the approach, seems like a good abstraction point.

[bashi, 2017-05-12 01:39:18]

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

[commit-bot: I haz the power, 2017-05-12 01:40:16]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[bashi, 2017-05-12 01:53:16]

Thanks haraken-san and Chris. PS#2 is ready for review.

(I was trying to tweak some method signatures so that we don't need to lookup
children() more than once but it involved many changes. I'll try to tweak
signatures in follow-up CLs)

https://codereview.chromium.org/2867353002/diff/1/content/browser/memory/memo...
File content/browser/memory/memory_coordinator_default_policy.h (right):

https://codereview.chromium.org/2867353002/diff/1/content/browser/memory/memo...
content/browser/memory/memory_coordinator_default_policy.h:15:
MemoryCoordinatorDefaultPolicy(MemoryCoordinatorImpl* coordinator);
On 2017/05/09 11:47:30, haraken wrote:
> 
> Add explicit.

Done.

https://codereview.chromium.org/2867353002/diff/40001/content/browser/memory/...
File content/browser/memory/memory_coordinator_impl.h (left):

https://codereview.chromium.org/2867353002/diff/40001/content/browser/memory/...
content/browser/memory/memory_coordinator_impl.h:82: MemoryState
GetChildMemoryState(int render_process_id) const;
This is dead code. Removed.

[commit-bot: I haz the power, 2017-05-12 03:30:57]

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

[commit-bot: I haz the power, 2017-05-12 03:30:58]

Dry run: This issue passed the CQ dry run.

[haraken, 2017-05-12 04:06:08]

LGTM. I'm assuming that you're not making any functional change.

[bashi, 2017-05-12 07:09:14]

bashi@chromium.org changed reviewers:
+ kinuko@chromium.org

[bashi, 2017-05-12 07:09:14]

+kinuko@: could you review content/browser/ ?

[kinuko, 2017-05-12 07:45:25]

lgtm

[bashi, 2017-05-12 09:32:00]

Thank you!

[bashi, 2017-05-12 09:32:05]

The CQ bit was checked by bashi@chromium.org

[commit-bot: I haz the power, 2017-05-12 09:32:25]

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-05-12 09:38:35]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1494581525338370,
"parent_rev": "3a0258864f393c913a30cdd2665d56f213395b1c", "commit_rev":
"c5106fdfb45ca1c85f5018dfbf0989d3c97db323"}

[commit-bot: I haz the power, 2017-05-12 09:38:49]

Description was changed from

==========
memory coordinator: Introduce MemoryCoordinatorImpl::Policy

Proper actions under memory pressure can vary on different
systems. Use a strategy pattern to make policies interchangeable.

This CL just moves the current heuristics to
MemoryCoordinatorDefaultPolicy.
 
Design doc:
https://docs.google.com/document/d/1rGeUzhfzIKwmBzUcqUEwxEpLQQRq3PulRuHwFBlVI...

BUG=696844
==========

to

==========
memory coordinator: Introduce MemoryCoordinatorImpl::Policy

Proper actions under memory pressure can vary on different
systems. Use a strategy pattern to make policies interchangeable.

This CL just moves the current heuristics to
MemoryCoordinatorDefaultPolicy.
 
Design doc:
https://docs.google.com/document/d/1rGeUzhfzIKwmBzUcqUEwxEpLQQRq3PulRuHwFBlVI...

BUG=696844

Review-Url: https://codereview.chromium.org/2867353002
Cr-Commit-Position: refs/heads/master@{#471260}
Committed:
https://chromium.googlesource.com/chromium/src/+/c5106fdfb45ca1c85f5018dfbf09...
==========

[commit-bot: I haz the power, 2017-05-12 09:38:52]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/c5106fdfb45ca1c85f5018dfbf09...