Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(62)

Issue 2413643002: Add MemoryCoordinator::SetClientMemoryState and GetClientMemoryState (Closed)

Created:
4 years, 2 months ago by hajimehoshi
Modified:
4 years, 2 months ago
Reviewers:
haraken, chrisha, bashi
CC:
chromium-reviews, jam, darin-cc_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add MemoryCoordinator::SetClientMemoryState and GetClientMemoryState This CL adds methods SetClientMemoryState and GetClientMemoryState to MemoryCoordinator. The setter dispatch memory state change to MemoryCoordinatorClients registerd by MemoryCoordinatorClientRegistry in the browser process. The getter returns the current memory state of the clients. BUG=639700

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -1 line) Patch
M content/browser/memory/memory_coordinator.h View 2 chunks +8 lines, -0 lines 0 comments Download
M content/browser/memory/memory_coordinator.cc View 3 chunks +11 lines, -1 line 1 comment Download

Depends on Patchset:

Messages

Total messages: 5 (2 generated)
hajimehoshi
PTAL
4 years, 2 months ago (2016-10-12 07:59:41 UTC) #2
haraken
https://codereview.chromium.org/2413643002/diff/1/content/browser/memory/memory_coordinator.cc File content/browser/memory/memory_coordinator.cc (right): https://codereview.chromium.org/2413643002/diff/1/content/browser/memory/memory_coordinator.cc#newcode85 content/browser/memory/memory_coordinator.cc:85: void MemoryCoordinator::SetClientMemoryState(base::MemoryState memory_state) { Who are you assuming calls ...
4 years, 2 months ago (2016-10-12 08:11:45 UTC) #3
hajimehoshi
4 years, 2 months ago (2016-10-12 08:31:55 UTC) #4
On 2016/10/12 08:11:45, haraken wrote:
>
https://codereview.chromium.org/2413643002/diff/1/content/browser/memory/memo...
> File content/browser/memory/memory_coordinator.cc (right):
> 
>
https://codereview.chromium.org/2413643002/diff/1/content/browser/memory/memo...
> content/browser/memory/memory_coordinator.cc:85: void
> MemoryCoordinator::SetClientMemoryState(base::MemoryState memory_state) {
> 
> Who are you assuming calls this method?
> 
> Also Chris' proposal is to introduce MemoryCoordinatorProxy, right?
> 
> I might want to see a proof-of-concept CL that implements MCP and changes MC
and
> CMC using MCP. Also we need to add tests.

As we discussed offline, we confirmed the necessity of the current memory state
management (and its getter) before creating the proxy but this is being
implemented at MemoryCoordinatorImpl. I've not noticed
https://codereview.chromium.org/2374343002/. Let me close this.

Powered by Google App Engine
This is Rietveld 408576698