[memory-infra] Make MemoryDumpScheduler non-thread safe singleton

[memory-infra] Make MemoryDumpScheduler non-thread safe singleton

The MemoryDumpScheduler thread restrictions and lifetime were
complicated to follow. So, make it singleton with thread safety handled
by MemoryDumpManager. MDM always accesses the schduler holding a lock.

BUG=607533
Review-Url: https://codereview.chromium.org/2743993004
Cr-Commit-Position: refs/heads/master@{#459515}
Committed: https://chromium.googlesource.com/chromium/src/+/ea9b45ede358c7d903340c2a40709094152d3e2c

[ssid, 2017-03-11 03:08:25]

ssid@chromium.org changed reviewers:
+ primiano@chromium.org

[ssid, 2017-03-11 03:08:25]

I am not sure if this is a good pattern. Do you think it looks better now or any
suggestions?

[ssid, 2017-03-11 03:08:47]

Patchset #1 (id:1) has been deleted

[ssid, 2017-03-11 03:08:56]

Patchset #1 (id:20001) has been deleted

[Primiano Tucci (use gerrit), 2017-03-16 19:01:36]

LGTM with minor comments

https://codereview.chromium.org/2743993004/diff/40001/base/trace_event/memory...
File base/trace_event/memory_dump_scheduler.cc (right):

https://codereview.chromium.org/2743993004/diff/40001/base/trace_event/memory...
base/trace_event/memory_dump_scheduler.cc:140: polling_task_runner_ = nullptr;
.reset() might be slightly more readable, as it becomes clear that this is a
managed ptr (And also consistent with the reset() below)

https://codereview.chromium.org/2743993004/diff/40001/base/trace_event/memory...
File base/trace_event/memory_dump_scheduler.h (right):

https://codereview.chromium.org/2743993004/diff/40001/base/trace_event/memory...
base/trace_event/memory_dump_scheduler.h:13: #include "base/memory/singleton.h"
these days you don't need this boiler plate anymore. Due to C++11 thread-safe
statics you can just write, in the .cc file:

MDM* GetInstance()  {
  static MDM* instance = new MDM();
  return instance;
}

and that's all :)

See for instance
https://cs.chromium.org/chromium/src/media/base/media.cc?rcl=c2c59733fe5acfba...

[ssid, 2017-03-22 00:40:23]

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

[commit-bot: I haz the power, 2017-03-22 00:41:12]

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, 2017-03-22 00:44:41]

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

[commit-bot: I haz the power, 2017-03-22 00:44:42]

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

[ssid, 2017-03-22 01:32:59]

Thanks

https://codereview.chromium.org/2743993004/diff/40001/base/trace_event/memory...
File base/trace_event/memory_dump_scheduler.cc (right):

https://codereview.chromium.org/2743993004/diff/40001/base/trace_event/memory...
base/trace_event/memory_dump_scheduler.cc:140: polling_task_runner_ = nullptr;
On 2017/03/16 19:01:36, Primiano Tucci wrote:
> .reset() might be slightly more readable, as it becomes clear that this is a
> managed ptr (And also consistent with the reset() below)

scoped_refptr doesn't have a "reset()" method. The comment in the header says
use "= nullptr". So leaving it as it is.

https://codereview.chromium.org/2743993004/diff/40001/base/trace_event/memory...
File base/trace_event/memory_dump_scheduler.h (right):

https://codereview.chromium.org/2743993004/diff/40001/base/trace_event/memory...
base/trace_event/memory_dump_scheduler.h:13: #include "base/memory/singleton.h"
On 2017/03/16 19:01:36, Primiano Tucci wrote:
> these days you don't need this boiler plate anymore. Due to C++11 thread-safe
> statics you can just write, in the .cc file:
> 
> MDM* GetInstance()  {
>   static MDM* instance = new MDM();
>   return instance;
> }
> 
> and that's all :)
> 
> See for instance
>
https://cs.chromium.org/chromium/src/media/base/media.cc?rcl=c2c59733fe5acfba...

Thanks! fixed.

[ssid, 2017-03-23 00:51:14]

Description was changed from

==========
[memory-infra] Make MemoryDumpScheduler non-thread safe singleton

The MemoryDumpScheduler thread restrictions and lifetime were
complicated to follow. So, make it singleton with thread safety handled
by MemoryDumpManager. MDM always accesses the schduler holding a lock.

BUG=607533
==========

to

==========
[memory-infra] Make MemoryDumpScheduler non-thread safe singleton

The MemoryDumpScheduler thread restrictions and lifetime were
complicated to follow. So, make it singleton with thread safety handled
by MemoryDumpManager. MDM always accesses the schduler holding a lock.

BUG=607533
==========

[ssid, 2017-03-24 01:03:20]

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

[commit-bot: I haz the power, 2017-03-24 01:03:50]

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

[ssid, 2017-03-24 01:04:59]

The CQ bit was unchecked by ssid@chromium.org

[ssid, 2017-03-24 17:17:18]

Patchset #3 (id:80001) has been deleted

[ssid, 2017-03-24 17:21:29]

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

[commit-bot: I haz the power, 2017-03-24 17:21:58]

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

[ssid, 2017-03-24 18:05:43]

Patchset #4 (id:120001) has been deleted

[ssid, 2017-03-24 18:28:12]

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

[commit-bot: I haz the power, 2017-03-24 18:29:09]

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, 2017-03-24 19:10:57]

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

[commit-bot: I haz the power, 2017-03-24 19:10:58]

Dry run: This issue passed the CQ dry run.

[ssid, 2017-03-24 19:12:59]

The CQ bit was checked by ssid@chromium.org

[ssid, 2017-03-24 19:12:59]

The patchset sent to the CQ was uploaded after l-g-t-m from
primiano@chromium.org
Link to the patchset: https://codereview.chromium.org/2743993004/#ps100001
(title: "rebase.")

[commit-bot: I haz the power, 2017-03-24 19:13:42]

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-03-24 19:21:37]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1490382779352470,
"parent_rev": "087035f0efd4da20592c02b64fddaa7bfee761c6", "commit_rev":
"ea9b45ede358c7d903340c2a40709094152d3e2c"}

[commit-bot: I haz the power, 2017-03-24 19:22:33]

Description was changed from

==========
[memory-infra] Make MemoryDumpScheduler non-thread safe singleton

The MemoryDumpScheduler thread restrictions and lifetime were
complicated to follow. So, make it singleton with thread safety handled
by MemoryDumpManager. MDM always accesses the schduler holding a lock.

BUG=607533
==========

to

==========
[memory-infra] Make MemoryDumpScheduler non-thread safe singleton

The MemoryDumpScheduler thread restrictions and lifetime were
complicated to follow. So, make it singleton with thread safety handled
by MemoryDumpManager. MDM always accesses the schduler holding a lock.

BUG=607533

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

[commit-bot: I haz the power, 2017-03-24 19:22:34]

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