Move discardable memory to //components from //content

Move discardable memory  to //components from //content

BUG=654678
Committed: https://crrev.com/d81c1a6966c6e4d1f5e49ae7b1d08a29faa1944d
Cr-Commit-Position: refs/heads/master@{#429380}

[Peng, 2016-10-27 20:41:27]

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

[commit-bot: I haz the power, 2016-10-27 20:42:31]

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-27 20:47:48]

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

[commit-bot: I haz the power, 2016-10-27 20:47:49]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Peng, 2016-10-27 21:08:32]

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

[commit-bot: I haz the power, 2016-10-27 21:09:37]

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-27 21:14:48]

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

[commit-bot: I haz the power, 2016-10-27 21:14:49]

Dry run: Try jobs failed on following builders:
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_clan...)
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Peng, 2016-10-28 15:10:06]

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

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

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-28 15:14:38]

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

[commit-bot: I haz the power, 2016-10-28 15:14:38]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Peng, 2016-10-28 15:22:13]

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

[commit-bot: I haz the power, 2016-10-28 15:22:34]

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-28 15:32:06]

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

[commit-bot: I haz the power, 2016-10-28 15:32:07]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Peng, 2016-10-28 18:19:31]

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

[commit-bot: I haz the power, 2016-10-28 18:20:06]

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-28 18:30:13]

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

[commit-bot: I haz the power, 2016-10-28 18:30:14]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Peng, 2016-10-28 21:39:35]

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

[commit-bot: I haz the power, 2016-10-28 21:40:06]

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-28 21:51:52]

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

[commit-bot: I haz the power, 2016-10-28 21:51:53]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Peng, 2016-10-31 15:08:38]

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

[commit-bot: I haz the power, 2016-10-31 15:08:50]

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-31 15:17:48]

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

[commit-bot: I haz the power, 2016-10-31 15:17:49]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[Peng, 2016-10-31 18:05:44]

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

[commit-bot: I haz the power, 2016-10-31 18:06:28]

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-31 18:47:27]

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

[commit-bot: I haz the power, 2016-10-31 18:47:28]

Dry run: Try jobs failed on following builders:
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)

[Peng, 2016-10-31 20:25:40]

Description was changed from

==========
Move discardable memory out of content to components

BUG=654678
==========

to

==========
Move discardable memory out of content to components.



BUG=654678
==========

[Peng, 2016-10-31 20:27:24]

penghuang@chromium.org changed reviewers:
+ reveman@chromium.org

[Peng, 2016-10-31 20:27:25]

Hi David, PTAL. Thanks.

[Peng, 2016-10-31 20:27:36]

Description was changed from

==========
Move discardable memory out of content to components.



BUG=654678
==========

to

==========
Move discardable memory  to //components from //content



BUG=654678
==========

[reveman, 2016-10-31 22:46:02]

lg, just a few questions and minor nits

also please add a components/discardable_memory/OWNERS file with my chromium
email

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
File
components/discardable_memory/client/child_discardable_shared_memory_manager.h
(right):

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/client/child_discardable_shared_memory_manager.h:29:
virtual ~Delegate() {}
nit: can this be protected?

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/client/child_discardable_shared_memory_manager.h:77:
Delegate* delegate_;
nit: Delegate* const delegate_

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
File components/discardable_memory/common/discardable_shared_memory_heap.cc
(right):

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_heap.cc:281:
const size_t extra = serving->length_ - blocks;
did you need this to fix the build? just curious

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
File components/discardable_memory/common/discardable_shared_memory_manager.h
(right):

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:35: //
discardable memory segments for the browser process and child processes.
Can you update this comment to make sense in more generic host/client terms?

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:37:
class DISCARDABLE_MEMORY_EXPORT DiscardableSharedMemoryManager
what's the reason 'Host' was removed from the name here?

[Peng, 2016-11-01 14:26:18]

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
File
components/discardable_memory/client/child_discardable_shared_memory_manager.h
(right):

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/client/child_discardable_shared_memory_manager.h:29:
virtual ~Delegate() {}
On 2016/10/31 22:46:02, reveman wrote:
> nit: can this be protected?

Done.

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/client/child_discardable_shared_memory_manager.h:77:
Delegate* delegate_;
On 2016/10/31 22:46:02, reveman wrote:
> nit: Delegate* const delegate_

Done.

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
File components/discardable_memory/common/discardable_shared_memory_heap.cc
(right):

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_heap.cc:281:
const size_t extra = serving->length_ - blocks;
On 2016/10/31 22:46:02, reveman wrote:
> did you need this to fix the build? just curious

Yes. It causes a warning on WIN build. Not sure why it doesn't happen when this
file is in content/.

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
File components/discardable_memory/common/discardable_shared_memory_manager.h
(right):

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:35: //
discardable memory segments for the browser process and child processes.
On 2016/10/31 22:46:02, reveman wrote:
> Can you update this comment to make sense in more generic host/client terms?

Done.

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:37:
class DISCARDABLE_MEMORY_EXPORT DiscardableSharedMemoryManager
On 2016/10/31 22:46:02, reveman wrote:
> what's the reason 'Host' was removed from the name here?

I just think the term Host is more like old chromium IPC concept, in mojo, this
class will be a mojo service. So I remove Host. WDYT?

[reveman, 2016-11-01 14:48:46]

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
File components/discardable_memory/common/discardable_shared_memory_manager.h
(right):

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:37:
class DISCARDABLE_MEMORY_EXPORT DiscardableSharedMemoryManager
On 2016/11/01 at 14:26:18, Peng wrote:
> On 2016/10/31 22:46:02, reveman wrote:
> > what's the reason 'Host' was removed from the name here?
> 
> I just think the term Host is more like old chromium IPC concept, in mojo,
this class will be a mojo service. So I remove Host. WDYT?

Sgtm. Should we change the name of child manager to client manager at the same
time? Child is also very content specific..

[Peng, 2016-11-01 15:59:48]

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

[commit-bot: I haz the power, 2016-11-01 16:00:07]

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

[Peng, 2016-11-01 16:00:23]

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
File components/discardable_memory/common/discardable_shared_memory_manager.h
(right):

https://codereview.chromium.org/2459733002/diff/160001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:37:
class DISCARDABLE_MEMORY_EXPORT DiscardableSharedMemoryManager
On 2016/11/01 14:48:45, reveman wrote:
> On 2016/11/01 at 14:26:18, Peng wrote:
> > On 2016/10/31 22:46:02, reveman wrote:
> > > what's the reason 'Host' was removed from the name here?
> > 
> > I just think the term Host is more like old chromium IPC concept, in mojo,
> this class will be a mojo service. So I remove Host. WDYT?
> 
> Sgtm. Should we change the name of child manager to client manager at the same
> time? Child is also very content specific..

Done.

[commit-bot: I haz the power, 2016-11-01 16:17:50]

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

[commit-bot: I haz the power, 2016-11-01 16:17:51]

Dry run: Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[Peng, 2016-11-01 16:21:26]

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

[commit-bot: I haz the power, 2016-11-01 16:21:53]

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-11-01 16:36:00]

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

[commit-bot: I haz the power, 2016-11-01 16:36:01]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[reveman, 2016-11-01 17:18:39]

Please make sure all references to child are replaced by client. I might have
missed a few in my comments below.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
File components/discardable_memory/common/BUILD.gn (right):

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/BUILD.gn:17:
"discardable_shared_memory_manager.h",
Please move the discardable_shared_memory_manager* files into a "service"
directory. It's confusing to have a "common" directory otherwise and it would be
nice to keep this clear separation between client, service and common code.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
File components/discardable_memory/common/discardable_shared_memory_manager.cc
(right):

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:43:
uint64_t ClientIdToClientTracingId(int client_id) {
nit: ClientProcessUniqueIdToTracingProcessId

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:45:
return client_id;
This needs to work properly for chrome as we rely on it for critical memory
measurements. Can we just have it do what ChildProcessUniqueIdToTracingProcessId
is currently doing and then leave the exercise of making it adjustable by the
embedder as a TODO?

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:241:
const int child_process_id = process_entry.first;
nit: s/child/client/

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:265:
// Create the cross-process ownership edge. If the child creates a
nit: s/child/client/

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:269:
const uint64_t child_tracing_process_id =
nit: s/child/client/

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:300:
int child_process_id,
nit: s/child/client/

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:310:
int child_process_id) {
nit: s/child/client/

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:314:
void DiscardableSharedMemoryManager::ProcessRemoved(int child_process_id) {
nit: s/child/client/

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
File components/discardable_memory/common/discardable_shared_memory_manager.h
(right):

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:5:
#ifndef
COMPONENTS_DISCARDABLE_MEMORY_COMMON_HOST_DISCARDABLE_SHARED_MEMORY_MANAGER_H_
remove HOST_

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:6:
#define
COMPONENTS_DISCARDABLE_MEMORY_COMMON_HOST_DISCARDABLE_SHARED_MEMORY_MANAGER_H_
remove HOST_

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:60:
void AllocateLockedDiscardableSharedMemoryForChild(
ForClient

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:67: //
Call this to notify the manager that child process associated with
client

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:68: //
|child_process_id| has deleted discardable memory segment with |id|.
client_process_id

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:69:
void ChildDeletedDiscardableSharedMemory(DiscardableSharedMemoryId id,
ClientDeleted..

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:72: //
Call this to notify the manager that child process associated with
client

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:73: //
|child_process_id| has been removed. The manager will use this to release
client_process_id

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:73: //
|child_process_id| has been removed. The manager will use this to release
client_process_id

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:74: //
memory segments allocated for child process to the OS.
client

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:75:
void ProcessRemoved(int child_process_id);
client_process_id

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:157:
#endif  //
COMPONENTS_DISCARDABLE_MEMORY_COMMON_HOST_DISCARDABLE_SHARED_MEMORY_MANAGER_H_
remove HOST_

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
File
components/discardable_memory/common/discardable_shared_memory_manager_unittest.cc
(right):

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager_unittest.cc:73:
TEST_F(DiscardableSharedMemoryManagerTest, AllocateForChild) {
nit: s/Child/Client/

[Peng, 2016-11-01 18:38:31]

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
File components/discardable_memory/common/BUILD.gn (right):

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/BUILD.gn:17:
"discardable_shared_memory_manager.h",
On 2016/11/01 17:18:38, reveman wrote:
> Please move the discardable_shared_memory_manager* files into a "service"
> directory. It's confusing to have a "common" directory otherwise and it would
be
> nice to keep this clear separation between client, service and common code. 

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
File components/discardable_memory/common/discardable_shared_memory_manager.cc
(right):

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:43:
uint64_t ClientIdToClientTracingId(int client_id) {
On 2016/11/01 17:18:38, reveman wrote:
> nit: ClientProcessUniqueIdToTracingProcessId

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:45:
return client_id;
On 2016/11/01 17:18:38, reveman wrote:
> This needs to work properly for chrome as we rely on it for critical memory
> measurements. Can we just have it do what
ChildProcessUniqueIdToTracingProcessId
> is currently doing and then leave the exercise of making it adjustable by the
> embedder as a TODO?

Done.

BTW for mojo, I don't think we can get process id of a client. So we need figure
out how to generate a tracing id for mojo IPC.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:241:
const int child_process_id = process_entry.first;
On 2016/11/01 17:18:38, reveman wrote:
> nit: s/child/client/

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:265:
// Create the cross-process ownership edge. If the child creates a
On 2016/11/01 17:18:38, reveman wrote:
> nit: s/child/client/

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:269:
const uint64_t child_tracing_process_id =
On 2016/11/01 17:18:38, reveman wrote:
> nit: s/child/client/

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:300:
int child_process_id,
On 2016/11/01 17:18:38, reveman wrote:
> nit: s/child/client/

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:310:
int child_process_id) {
On 2016/11/01 17:18:38, reveman wrote:
> nit: s/child/client/

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.cc:314:
void DiscardableSharedMemoryManager::ProcessRemoved(int child_process_id) {
On 2016/11/01 17:18:38, reveman wrote:
> nit: s/child/client/

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
File components/discardable_memory/common/discardable_shared_memory_manager.h
(right):

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:5:
#ifndef
COMPONENTS_DISCARDABLE_MEMORY_COMMON_HOST_DISCARDABLE_SHARED_MEMORY_MANAGER_H_
On 2016/11/01 17:18:39, reveman wrote:
> remove HOST_

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:6:
#define
COMPONENTS_DISCARDABLE_MEMORY_COMMON_HOST_DISCARDABLE_SHARED_MEMORY_MANAGER_H_
On 2016/11/01 17:18:39, reveman wrote:
> remove HOST_

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:60:
void AllocateLockedDiscardableSharedMemoryForChild(
On 2016/11/01 17:18:38, reveman wrote:
> ForClient

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:67: //
Call this to notify the manager that child process associated with
On 2016/11/01 17:18:39, reveman wrote:
> client

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:69:
void ChildDeletedDiscardableSharedMemory(DiscardableSharedMemoryId id,
On 2016/11/01 17:18:39, reveman wrote:
> ClientDeleted..

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:72: //
Call this to notify the manager that child process associated with
On 2016/11/01 17:18:39, reveman wrote:
> client

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:73: //
|child_process_id| has been removed. The manager will use this to release
On 2016/11/01 17:18:39, reveman wrote:
> client_process_id

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:74: //
memory segments allocated for child process to the OS.
On 2016/11/01 17:18:39, reveman wrote:
> client

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:75:
void ProcessRemoved(int child_process_id);
On 2016/11/01 17:18:39, reveman wrote:
> client_process_id

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager.h:157:
#endif  //
COMPONENTS_DISCARDABLE_MEMORY_COMMON_HOST_DISCARDABLE_SHARED_MEMORY_MANAGER_H_
On 2016/11/01 17:18:39, reveman wrote:
> remove HOST_

Done.

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
File
components/discardable_memory/common/discardable_shared_memory_manager_unittest.cc
(right):

https://codereview.chromium.org/2459733002/diff/220001/components/discardable...
components/discardable_memory/common/discardable_shared_memory_manager_unittest.cc:73:
TEST_F(DiscardableSharedMemoryManagerTest, AllocateForChild) {
On 2016/11/01 17:18:39, reveman wrote:
> nit: s/Child/Client/

Done.

[Peng, 2016-11-01 18:40:28]

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

[commit-bot: I haz the power, 2016-11-01 18:40:51]

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-11-01 18:45:22]

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

[commit-bot: I haz the power, 2016-11-01 18:45:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[reveman, 2016-11-01 18:49:23]

Thanks! LGTM after filing a bug and adding a TODO regarding the tracing id.

https://codereview.chromium.org/2459733002/diff/240001/components/discardable...
File components/discardable_memory/service/discardable_shared_memory_manager.cc
(right):

https://codereview.chromium.org/2459733002/diff/240001/components/discardable...
components/discardable_memory/service/discardable_shared_memory_manager.cc:48:
uint64_t ClientProcessUniqueIdToTracingProcessId(int client_id) {
nit: please file a bug about moving this to the right place and add a TODO here
with a link to the bug.

[Peng, 2016-11-01 19:10:22]

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

[Peng, 2016-11-01 19:10:36]

https://codereview.chromium.org/2459733002/diff/240001/components/discardable...
File components/discardable_memory/service/discardable_shared_memory_manager.cc
(right):

https://codereview.chromium.org/2459733002/diff/240001/components/discardable...
components/discardable_memory/service/discardable_shared_memory_manager.cc:48:
uint64_t ClientProcessUniqueIdToTracingProcessId(int client_id) {
On 2016/11/01 18:49:23, reveman wrote:
> nit: please file a bug about moving this to the right place and add a TODO
here
> with a link to the bug.

Done.

[commit-bot: I haz the power, 2016-11-01 19:11:03]

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

[Peng, 2016-11-01 19:11:29]

penghuang@chromium.org changed reviewers:
+ jochen@chromium.org

[Peng, 2016-11-01 19:11:30]

Hi jochen, PTAL. Thanks.

[Peng, 2016-11-01 19:13:24]

penghuang@chromium.org changed reviewers:
+ tsepez@chromium.org

[Peng, 2016-11-01 19:13:25]

+tsepez for content/common/child_process_messages.h
Hi Thomas, PTAL. Thanks.

[Tom Sepez, 2016-11-01 19:15:10]

RS LGTM on moving/renaming type.

[commit-bot: I haz the power, 2016-11-01 19:24:06]

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

[commit-bot: I haz the power, 2016-11-01 19:24:07]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Peng, 2016-11-01 19:59:35]

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

[commit-bot: I haz the power, 2016-11-01 20:00:10]

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-11-01 20:04:30]

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

[commit-bot: I haz the power, 2016-11-01 20:04:31]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jochen (gone - plz use gerrit), 2016-11-02 10:58:55]

jochen@chromium.org changed reviewers:
+ jam@chromium.org

[jochen (gone - plz use gerrit), 2016-11-02 10:58:56]

+jam

wasn't the plan to only put new code shared with ios in components?

[Fady Samuel, 2016-11-02 11:32:50]

On 2016/11/02 10:58:56, jochen wrote:
> +jam
> 
> wasn't the plan to only put new code shared with ios in components?

We would like to use this in both mus and content.

[jochen (gone - plz use gerrit), 2016-11-02 11:36:49]

On 2016/11/02 at 11:32:50, fsamuel wrote:
> On 2016/11/02 10:58:56, jochen wrote:
> > +jam
> > 
> > wasn't the plan to only put new code shared with ios in components?
> 
> We would like to use this in both mus and content.

yeah, I understood that. However, John compiled a list of use cases for
components (in components/README) and mus doesn't naturally fit into this, so
I'd like him to chime in

[Peng, 2016-11-02 14:38:16]

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

[commit-bot: I haz the power, 2016-11-02 14:38:36]

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-11-02 16:15:34]

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

[commit-bot: I haz the power, 2016-11-02 16:15:36]

Dry run: This issue passed the CQ dry run.

[Peng, 2016-11-02 17:52:04]

Patchset #3 (id:40001) has been deleted

[jam, 2016-11-02 19:49:37]

On 2016/11/02 11:36:49, jochen wrote:
> On 2016/11/02 at 11:32:50, fsamuel wrote:
> > On 2016/11/02 10:58:56, jochen wrote:
> > > +jam
> > > 
> > > wasn't the plan to only put new code shared with ios in components?
> > 
> > We would like to use this in both mus and content.
> 
> yeah, I understood that. However, John compiled a list of use cases for
> components (in components/README) and mus doesn't naturally fit into this, so
> I'd like him to chime in

This is fine; sorry if the readme isn't very clear on this. it's not fully
exhaustive; my main intention was to list scenarios where we shouldn't be
creating a new component.

in this case, since mus and content are peers and they want to share code,
components is the right place (i.e. we wouldn't want either of them to depend on
the other to get this code)

(lgtm)

[Peng, 2016-11-02 19:59:35]

On 2016/11/02 19:49:37, jam wrote:
> On 2016/11/02 11:36:49, jochen wrote:
> > On 2016/11/02 at 11:32:50, fsamuel wrote:
> > > On 2016/11/02 10:58:56, jochen wrote:
> > > > +jam
> > > > 
> > > > wasn't the plan to only put new code shared with ios in components?
> > > 
> > > We would like to use this in both mus and content.
> > 
> > yeah, I understood that. However, John compiled a list of use cases for
> > components (in components/README) and mus doesn't naturally fit into this,
so
> > I'd like him to chime in
> 
> This is fine; sorry if the readme isn't very clear on this. it's not fully
> exhaustive; my main intention was to list scenarios where we shouldn't be
> creating a new component.
> 
> in this case, since mus and content are peers and they want to share code,
> components is the right place (i.e. we wouldn't want either of them to depend
on
> the other to get this code)
> 
> (lgtm)

Thanks for the explanation.

[Peng, 2016-11-02 19:59:44]

The CQ bit was checked by penghuang@chromium.org

[Peng, 2016-11-02 19:59:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
reveman@chromium.org, tsepez@chromium.org
Link to the patchset: https://codereview.chromium.org/2459733002/#ps300001
(title: "Fix build error")

[commit-bot: I haz the power, 2016-11-02 20:00:15]

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-02 20:06:28]

Description was changed from

==========
Move discardable memory  to //components from //content



BUG=654678
==========

to

==========
Move discardable memory  to //components from //content



BUG=654678
==========

[commit-bot: I haz the power, 2016-11-02 20:06:31]

Committed patchset #15 (id:300001)

[commit-bot: I haz the power, 2016-11-02 20:10:05]

Description was changed from

==========
Move discardable memory  to //components from //content



BUG=654678
==========

to

==========
Move discardable memory  to //components from //content

BUG=654678

Committed: https://crrev.com/d81c1a6966c6e4d1f5e49ae7b1d08a29faa1944d
Cr-Commit-Position: refs/heads/master@{#429380}
==========

[commit-bot: I haz the power, 2016-11-02 20:10:07]

Patchset 15 (id:??) landed as
https://crrev.com/d81c1a6966c6e4d1f5e49ae7b1d08a29faa1944d
Cr-Commit-Position: refs/heads/master@{#429380}