Add a public interface for content::MemoryCoordinator

Add a public interface for content::MemoryCoordinator

This public interface will be used to query memory state of
each process from the task manager.

BUG=673216
Committed: https://crrev.com/690a7220b424d9624650d007c296aba7a1fe29b0
Cr-Commit-Position: refs/heads/master@{#440688}

[bashi, 2016-12-20 04:20:33]

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

[commit-bot: I haz the power, 2016-12-20 04:20:55]

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-12-20 05:12:25]

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

[commit-bot: I haz the power, 2016-12-20 05:12:26]

Dry run: This issue passed the CQ dry run.

[bashi, 2016-12-20 06:29:51]

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

[bashi, 2016-12-20 06:29:52]

What do you think about this CL? I tried some approaches to expose memory state
of each process outside content and I think having an interface in
content/public would be a better choice. (This approaches what Chris suggested
first. I wasn't aware of the benefit of having interface in content) Task
manager will use the interface to show memory state of each process.

I'll add another unittest if this looks fine.

[haraken, 2016-12-20 10:13:52]

On 2016/12/20 06:29:52, bashi1 wrote:
> What do you think about this CL? I tried some approaches to expose memory
state
> of each process outside content and I think having an interface in
> content/public would be a better choice. (This approaches what Chris suggested
> first. I wasn't aware of the benefit of having interface in content) Task
> manager will use the interface to show memory state of each process.
> 
> I'll add another unittest if this looks fine.

The class structure looks good, though it looks a bit confusing to have
"MemoryCoordinator" in both base/ and content/public/.

[bashi, 2016-12-20 10:34:55]

On 2016/12/20 10:13:52, haraken wrote:
> On 2016/12/20 06:29:52, bashi1 wrote:
> > What do you think about this CL? I tried some approaches to expose memory
> state
> > of each process outside content and I think having an interface in
> > content/public would be a better choice. (This approaches what Chris
suggested
> > first. I wasn't aware of the benefit of having interface in content) Task
> > manager will use the interface to show memory state of each process.
> > 
> > I'll add another unittest if this looks fine.
> 
> The class structure looks good, though it looks a bit confusing to have
> "MemoryCoordinator" in both base/ and content/public/.

I agree. Any suggestion for naming?

I think we can defer adding base::MemoryCoordinator. GetStateForProcess() is
what we want to expose now so maybe we just add content::MemoryCoordinator to
avoid confusing naming.

[haraken, 2016-12-20 10:39:34]

On 2016/12/20 10:34:55, bashi1 wrote:
> On 2016/12/20 10:13:52, haraken wrote:
> > On 2016/12/20 06:29:52, bashi1 wrote:
> > > What do you think about this CL? I tried some approaches to expose memory
> > state
> > > of each process outside content and I think having an interface in
> > > content/public would be a better choice. (This approaches what Chris
> suggested
> > > first. I wasn't aware of the benefit of having interface in content) Task
> > > manager will use the interface to show memory state of each process.
> > > 
> > > I'll add another unittest if this looks fine.
> > 
> > The class structure looks good, though it looks a bit confusing to have
> > "MemoryCoordinator" in both base/ and content/public/.
> 
> I agree. Any suggestion for naming?
> 
> I think we can defer adding base::MemoryCoordinator. GetStateForProcess() is
> what we want to expose now so maybe we just add content::MemoryCoordinator to
> avoid confusing naming.

Sounds reasonable to me.

[bashi, 2016-12-22 00:05:30]

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

[commit-bot: I haz the power, 2016-12-22 00:05:53]

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

[bashi, 2016-12-22 00:12:13]

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

[commit-bot: I haz the power, 2016-12-22 00:12:46]

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-12-22 00:52:30]

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

[commit-bot: I haz the power, 2016-12-22 00:52:31]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[bashi, 2016-12-22 01:23:09]

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

[commit-bot: I haz the power, 2016-12-22 01:23:32]

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

[bashi, 2016-12-22 01:29:45]

Added a unittest. I think this CL is ready for review. PTAL.

[haraken, 2016-12-22 01:35:47]

LGTM

[bashi, 2016-12-22 01:39:39]

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

[bashi, 2016-12-22 01:39:39]

+pfeldman@ for OWNERS review

[commit-bot: I haz the power, 2016-12-22 02:28:53]

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

[commit-bot: I haz the power, 2016-12-22 02:28:53]

Dry run: This issue passed the CQ dry run.

[pfeldman, 2016-12-22 17:22:54]

lgtm

[bashi, 2016-12-25 21:53:04]

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

[commit-bot: I haz the power, 2016-12-25 21:53:11]

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-12-25 22:46:05]

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

[commit-bot: I haz the power, 2016-12-25 22:46:06]

Dry run: This issue passed the CQ dry run.

[bashi, 2016-12-25 23:25:49]

The CQ bit was checked by bashi@chromium.org

[bashi, 2016-12-25 23:25:50]

The patchset sent to the CQ was uploaded after l-g-t-m from
haraken@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/2590773002/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2016-12-25 23:25:59]

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-12-25 23:29:43]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1482708349644830,
"parent_rev": "0b826886d9902053400e0d43cf68f27cabf06e70", "commit_rev":
"1c0ec1d1047ff09a6fb1b71ea7a2dcf1e3e87234"}

[commit-bot: I haz the power, 2016-12-25 23:30:12]

Description was changed from

==========
Add a public interface for content::MemoryCoordinator

This public interface will be used to query memory state of
each process from the task manager.

BUG=673216
==========

to

==========
Add a public interface for content::MemoryCoordinator

This public interface will be used to query memory state of
each process from the task manager.

BUG=673216

Review-Url: https://codereview.chromium.org/2590773002
==========

[commit-bot: I haz the power, 2016-12-25 23:30:13]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-12-25 23:33:07]

Description was changed from

==========
Add a public interface for content::MemoryCoordinator

This public interface will be used to query memory state of
each process from the task manager.

BUG=673216

Review-Url: https://codereview.chromium.org/2590773002
==========

to

==========
Add a public interface for content::MemoryCoordinator

This public interface will be used to query memory state of
each process from the task manager.

BUG=673216

Committed: https://crrev.com/690a7220b424d9624650d007c296aba7a1fe29b0
Cr-Commit-Position: refs/heads/master@{#440688}
==========

[commit-bot: I haz the power, 2016-12-25 23:33:08]

Patchset 5 (id:??) landed as
https://crrev.com/690a7220b424d9624650d007c296aba7a1fe29b0
Cr-Commit-Position: refs/heads/master@{#440688}