Make RenderThreadImpl a client of memory coordinator

Make RenderThreadImpl a client of memory coordinator

When the memory coordinator is enabled, RenderThreadImpl becomes a
client of the memory coordinator instead of installing
MemoryPressureListener.

In the implmentation of this CL, both MemoryPressureListener and
MemoryCoordinatorClient are installed regardless of the flag, but as
default MemoryCoordinator is not enabled and when MemoryCoordinator is
enabled by a flag, it is planed to suppress MemoryPressureMonitor
features.

Design Doc: https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8Uo/edit?ts=57d7968b#

BUG=639700
Committed: https://crrev.com/7bb39585012ce3c383e880bc43b710226326a026
Cr-Commit-Position: refs/heads/master@{#424668}

[hajimehoshi, 2016-10-07 11:28:18]

Patchset #1 (id:1) has been deleted

[hajimehoshi, 2016-10-07 11:28:57]

Description was changed from

==========
Make RenderThreadImpl a client of memory coordinator

When the memory coordinator is enabled, RenderThreadImpl becomes a
client of the memory coordinator instead of installing
MemoryPressureListener.

In the implmentation of this CL, both MemoryPressureListener and
MemoryCoordinatorClient are installed regardless of the flag, but as
default MemoryCoordinator is not enabled and when MemoryCoordinator is
enabled by a flag, it is planed to suppress MemoryPressureMonitor
features.

Design Doc:
https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8...

BUG=639700
==========

to

==========
Make RenderThreadImpl a client of memory coordinator

When the memory coordinator is enabled, RenderThreadImpl becomes a
client of the memory coordinator instead of installing
MemoryPressureListener.

In the implmentation of this CL, both MemoryPressureListener and
MemoryCoordinatorClient are installed regardless of the flag, but as
default MemoryCoordinator is not enabled and when MemoryCoordinator is
enabled by a flag, it is planed to suppress MemoryPressureMonitor
features.

Design Doc:
https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8...

BUG=639700
==========

[hajimehoshi, 2016-10-07 11:28:57]

hajimehoshi@chromium.org changed reviewers:
+ avi@chromium.org, bashi@chromium.org, chrisha@chromium.org,
haraken@chromium.org

[hajimehoshi, 2016-10-07 11:29:07]

PTAL

[hajimehoshi, 2016-10-07 11:37:38]

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

[commit-bot: I haz the power, 2016-10-07 11:37:50]

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

[haraken, 2016-10-07 11:45:30]

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

https://codereview.chromium.org/2402643002/diff/20001/content/renderer/render...
content/renderer/render_thread_impl.cc:905:
base::MemoryCoordinatorClientRegistry::GetInstance()->Unregister(this);

I'd call this in Shutdown().

https://codereview.chromium.org/2402643002/diff/20001/content/renderer/render...
content/renderer/render_thread_impl.cc:2170: void
RenderThreadImpl::ClearMemory(bool critical) {

I'd avoid introducing the boolean flag. Alternately, you can move
ReleaseFreeMemory() to the caller side; i.e.,

  case base::MemoryState::THROTTLED:
    ReleaseFreeMemory();
    break;
  case base::MemoryState::SUSPENDED:
    ReleaseFreeMemory();
    ClearMemory();
    break;

https://codereview.chromium.org/2402643002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/MemoryCoordinator.cpp (right):

https://codereview.chromium.org/2402643002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/MemoryCoordinator.cpp:54: void
MemoryCoordinator::clearMemory(bool critical) {

I'd avoid introducing a boolean flag. See my below comment.

https://codereview.chromium.org/2402643002/diff/20001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebMemoryState.h (right):

https://codereview.chromium.org/2402643002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebMemoryState.h:10: // These number must
correspond to base::MemoryState.

numbers

[Avi (use Gerrit), 2016-10-07 14:43:02]

I'm liking what I see so far, but ping me when haraken is satisfied.

[commit-bot: I haz the power, 2016-10-07 14:50:37]

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

[commit-bot: I haz the power, 2016-10-07 14:50:38]

Dry run: This issue passed the CQ dry run.

[hajimehoshi, 2016-10-11 07:05:32]

Thank you!

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

https://codereview.chromium.org/2402643002/diff/20001/content/renderer/render...
content/renderer/render_thread_impl.cc:905:
base::MemoryCoordinatorClientRegistry::GetInstance()->Unregister(this);
On 2016/10/07 11:45:30, haraken wrote:
> 
> I'd call this in Shutdown().

Done.

https://codereview.chromium.org/2402643002/diff/20001/content/renderer/render...
content/renderer/render_thread_impl.cc:2170: void
RenderThreadImpl::ClearMemory(bool critical) {
On 2016/10/07 11:45:30, haraken wrote:
> 
> I'd avoid introducing the boolean flag. Alternately, you can move
> ReleaseFreeMemory() to the caller side; i.e.,
> 
>   case base::MemoryState::THROTTLED:
>     ReleaseFreeMemory();
>     break;
>   case base::MemoryState::SUSPENDED:
>     ReleaseFreeMemory();
>     ClearMemory();
>     break;

Done.

https://codereview.chromium.org/2402643002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/MemoryCoordinator.cpp (right):

https://codereview.chromium.org/2402643002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/MemoryCoordinator.cpp:54: void
MemoryCoordinator::clearMemory(bool critical) {
On 2016/10/07 11:45:30, haraken wrote:
> 
> I'd avoid introducing a boolean flag. See my below comment.

Done.

https://codereview.chromium.org/2402643002/diff/20001/third_party/WebKit/publ...
File third_party/WebKit/public/platform/WebMemoryState.h (right):

https://codereview.chromium.org/2402643002/diff/20001/third_party/WebKit/publ...
third_party/WebKit/public/platform/WebMemoryState.h:10: // These number must
correspond to base::MemoryState.
On 2016/10/07 11:45:30, haraken wrote:
> 
> numbers

Done.

[hajimehoshi, 2016-10-11 07:05:50]

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

[commit-bot: I haz the power, 2016-10-11 07:05:56]

Dry run: 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-10-11 08:38:40]

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

[commit-bot: I haz the power, 2016-10-11 08:38:41]

Dry run: This issue passed the CQ dry run.

[haraken, 2016-10-11 08:38:55]

LGTM

[Avi (use Gerrit), 2016-10-11 15:04:57]

stampity stamp lgtm

[hajimehoshi, 2016-10-12 04:25:50]

The CQ bit was checked by hajimehoshi@chromium.org

[commit-bot: I haz the power, 2016-10-12 04:26:04]

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-10-12 04:31:20]

Description was changed from

==========
Make RenderThreadImpl a client of memory coordinator

When the memory coordinator is enabled, RenderThreadImpl becomes a
client of the memory coordinator instead of installing
MemoryPressureListener.

In the implmentation of this CL, both MemoryPressureListener and
MemoryCoordinatorClient are installed regardless of the flag, but as
default MemoryCoordinator is not enabled and when MemoryCoordinator is
enabled by a flag, it is planed to suppress MemoryPressureMonitor
features.

Design Doc:
https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8...

BUG=639700
==========

to

==========
Make RenderThreadImpl a client of memory coordinator

When the memory coordinator is enabled, RenderThreadImpl becomes a
client of the memory coordinator instead of installing
MemoryPressureListener.

In the implmentation of this CL, both MemoryPressureListener and
MemoryCoordinatorClient are installed regardless of the flag, but as
default MemoryCoordinator is not enabled and when MemoryCoordinator is
enabled by a flag, it is planed to suppress MemoryPressureMonitor
features.

Design Doc:
https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8...

BUG=639700
==========

[commit-bot: I haz the power, 2016-10-12 04:31:21]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-10-12 04:34:37]

Description was changed from

==========
Make RenderThreadImpl a client of memory coordinator

When the memory coordinator is enabled, RenderThreadImpl becomes a
client of the memory coordinator instead of installing
MemoryPressureListener.

In the implmentation of this CL, both MemoryPressureListener and
MemoryCoordinatorClient are installed regardless of the flag, but as
default MemoryCoordinator is not enabled and when MemoryCoordinator is
enabled by a flag, it is planed to suppress MemoryPressureMonitor
features.

Design Doc:
https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8...

BUG=639700
==========

to

==========
Make RenderThreadImpl a client of memory coordinator

When the memory coordinator is enabled, RenderThreadImpl becomes a
client of the memory coordinator instead of installing
MemoryPressureListener.

In the implmentation of this CL, both MemoryPressureListener and
MemoryCoordinatorClient are installed regardless of the flag, but as
default MemoryCoordinator is not enabled and when MemoryCoordinator is
enabled by a flag, it is planed to suppress MemoryPressureMonitor
features.

Design Doc:
https://docs.google.com/document/d/1a69mMr7jI7qK0OfKNlrZ350xiXizVMCCe8orGX7K8...

BUG=639700

Committed: https://crrev.com/7bb39585012ce3c383e880bc43b710226326a026
Cr-Commit-Position: refs/heads/master@{#424668}
==========

[commit-bot: I haz the power, 2016-10-12 04:34:38]

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