Revert of memory-infra: Finish moving to Mojo (2nd attempt)

Revert of memory-infra: Finish moving to Mojo (2nd attempt) (patchset #8 id:140001 of https://codereview.chromium.org/2734193002/ )

Reason for revert:
Causing assertion failures on the GPU FYI waterfall:
https://build.chromium.org/p/chromium.gpu.fyi/builders/Mac%20Debug%20%28Intel%29/builds/1049

Original issue's description:
> memory-infra: Finish moving to Mojo (2nd attempt)
> 
> This was originally landed in https://codereview.chromium.org/2694083005.
> But, it broke webview perf tests, and so, it was reverted. It turned out that
> in some scenarios, the memory dump manager is not initialized early enough (in
> the browser process). So, now, I initialize it as soon as the UI thread is
> created.
> 
> Modifications since the original CL are:
> 
> - The Coordinator service is created when the UI thread is created
>   (browser_main_loop.cc).
> - When Coordinator is created, it initializes memory dump manager's TPID
>   (memory_dump_manager_delegate_impl.cc).
> - To avoid dependency from //services/resource_coordinator to //content the
>   definition of the service TPID is moved from child_process_host.h to
>   constants.mojom.
> 
> I tested the CL by building for Android and running the following with an
> android device attached:
> 
> run_benchmark memory.top_10_mobile --browser=android-webview
> --also-run-disabled-tests
> 
> In the original reverted CL, the above command results in
> 'Unable to obtain memory dump' exceptions. With this CL the test runs
> successfully.
> 
> An example interaction diagram (to see the large image, click on it):
> https://docs.google.com/document/d/1Mz64egjuZ4WsYw9AKKWdTvl0Il706EWdCy24V87R_F4/edit#heading=h.1ku5wcoxqifr
> 
> BUG=679830, 697773, 697384, 697062
> 
> Review-Url: https://codereview.chromium.org/2734193002
> Cr-Commit-Position: refs/heads/master@{#455866}
> Committed: https://chromium.googlesource.com/chromium/src/+/0a3a302b6df6458b21b3ce0951c5251413cadc48

TBR=primiano@chromium.org,ssid@chromium.org,kenrb@chromium.org,jam@chromium.org,chiniforooshan@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=679830, 697773, 697384, 697062

[Geoff Lang, 2017-03-09 23:32:54]

The CQ bit was checked by geofflang@chromium.org

[Geoff Lang, 2017-03-09 23:32:55]

Created Revert of memory-infra: Finish moving to Mojo (2nd attempt)

[commit-bot: I haz the power, 2017-03-09 23:34:27]

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-09 23:37:30]

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

[commit-bot: I haz the power, 2017-03-09 23:37:32]

Failed to apply patch for base/trace_event/memory_dump_manager.cc:
While running git apply --index -p1;
  error: patch failed: base/trace_event/memory_dump_manager.cc:156
  error: base/trace_event/memory_dump_manager.cc: patch does not apply

Patch:       base/trace_event/memory_dump_manager.cc
Index: base/trace_event/memory_dump_manager.cc
diff --git a/base/trace_event/memory_dump_manager.cc
b/base/trace_event/memory_dump_manager.cc
index
f86b9e97d2c426fb5fff39bbe929db08c100316d..31016f970e3155cd18d749595d5dc432d38c5486
100644
--- a/base/trace_event/memory_dump_manager.cc
+++ b/base/trace_event/memory_dump_manager.cc
@@ -156,6 +156,7 @@
 
 MemoryDumpManager::MemoryDumpManager()
     : delegate_(nullptr),
+      is_coordinator_(false),
       memory_tracing_enabled_(0),
       tracing_process_id_(kInvalidTracingProcessId),
       dumper_registrations_ignored_for_testing_(false),
@@ -213,12 +214,14 @@
   heap_profiling_enabled_ = true;
 }
 
-void MemoryDumpManager::Initialize(MemoryDumpManagerDelegate* delegate) {
+void MemoryDumpManager::Initialize(MemoryDumpManagerDelegate* delegate,
+                                   bool is_coordinator) {
   {
     AutoLock lock(lock_);
     DCHECK(delegate);
     DCHECK(!delegate_);
     delegate_ = delegate;
+    is_coordinator_ = is_coordinator;
     EnableHeapProfilingIfNeeded();
   }
 
@@ -457,10 +460,21 @@
                                     TRACE_ID_MANGLE(guid));
   MemoryDumpCallback wrapped_callback = Bind(&OnGlobalDumpDone, callback);
 
+  // Technically there is no need to grab the |lock_| here as the delegate is
+  // long-lived and can only be set by Initialize(), which is locked and
+  // necessarily happens before memory_tracing_enabled_ == true.
+  // Not taking the |lock_|, though, is lakely make TSan barf and, at this
point
+  // (memory-infra is enabled) we're not in the fast-path anymore.
+  MemoryDumpManagerDelegate* delegate;
+  {
+    AutoLock lock(lock_);
+    delegate = delegate_;
+  }
+
   // The delegate will coordinate the IPC broadcast and at some point invoke
   // CreateProcessDump() to get a dump for the current process.
   MemoryDumpRequestArgs args = {guid, dump_type, level_of_detail};
-  delegate_->RequestGlobalMemoryDump(args, wrapped_callback);
+  delegate->RequestGlobalMemoryDump(args, wrapped_callback);
 }
 
 void MemoryDumpManager::RequestGlobalDump(
@@ -848,7 +862,7 @@
       dump_scheduler_->NotifyPollingSupported();
 
     // Only coordinator process triggers periodic global memory dumps.
-    if (delegate_->IsCoordinator())
+    if (is_coordinator_)
       dump_scheduler_->NotifyPeriodicTriggerSupported();
   }
 
@@ -893,6 +907,10 @@
   return session_state_->IsDumpModeAllowed(dump_mode);
 }
 
+uint64_t MemoryDumpManager::GetTracingProcessId() const {
+  return delegate_->GetTracingProcessId();
+}
+
 MemoryDumpManager::MemoryDumpProviderInfo::MemoryDumpProviderInfo(
     MemoryDumpProvider* dump_provider,
     const char* name,

[ssid, 2017-03-10 00:11:58]

already reverted in https://codereview.chromium.org/2741723003/

[ssid, 2017-03-10 00:14:00]

On 2017/03/10 00:11:58, ssid wrote:
> already reverted in https://codereview.chromium.org/2741723003/

Meh, I mean this https://codereview.chromium.org/2738093003/

[Geoff Lang, 2017-03-10 00:19:07]

On 2017/03/10 00:14:00, ssid wrote:
> On 2017/03/10 00:11:58, ssid wrote:
> > already reverted in https://codereview.chromium.org/2741723003/
> 
> Meh, I mean this https://codereview.chromium.org/2738093003/

Thanks!

[Geoff Lang, 2017-03-10 00:19:19]

Description was changed from

==========
Revert of memory-infra: Finish moving to Mojo (2nd attempt) (patchset #8
id:140001 of https://codereview.chromium.org/2734193002/ )

Reason for revert:
Causing assertion failures on the GPU FYI waterfall:
https://build.chromium.org/p/chromium.gpu.fyi/builders/Mac%20Debug%20%28Intel...

Original issue's description:
> memory-infra: Finish moving to Mojo (2nd attempt)
> 
> This was originally landed in https://codereview.chromium.org/2694083005.
> But, it broke webview perf tests, and so, it was reverted. It turned out that
> in some scenarios, the memory dump manager is not initialized early enough (in
> the browser process). So, now, I initialize it as soon as the UI thread is
> created.
> 
> Modifications since the original CL are:
> 
> - The Coordinator service is created when the UI thread is created
>   (browser_main_loop.cc).
> - When Coordinator is created, it initializes memory dump manager's TPID
>   (memory_dump_manager_delegate_impl.cc).
> - To avoid dependency from //services/resource_coordinator to //content the
>   definition of the service TPID is moved from child_process_host.h to
>   constants.mojom.
> 
> I tested the CL by building for Android and running the following with an
> android device attached:
> 
> run_benchmark memory.top_10_mobile --browser=android-webview
> --also-run-disabled-tests
> 
> In the original reverted CL, the above command results in
> 'Unable to obtain memory dump' exceptions. With this CL the test runs
> successfully.
> 
> An example interaction diagram (to see the large image, click on it):
>
https://docs.google.com/document/d/1Mz64egjuZ4WsYw9AKKWdTvl0Il706EWdCy24V87R_...
> 
> BUG=679830, 697773, 697384, 697062
> 
> Review-Url: https://codereview.chromium.org/2734193002
> Cr-Commit-Position: refs/heads/master@{#455866}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0a3a302b6df6458b21b3ce0951c5...

TBR=primiano@chromium.org,ssid@chromium.org,kenrb@chromium.org,jam@chromium.o...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=679830, 697773, 697384, 697062
==========

to

==========
Revert of memory-infra: Finish moving to Mojo (2nd attempt) (patchset #8
id:140001 of https://codereview.chromium.org/2734193002/ )

Reason for revert:
Causing assertion failures on the GPU FYI waterfall:
https://build.chromium.org/p/chromium.gpu.fyi/builders/Mac%20Debug%20%28Intel...

Original issue's description:
> memory-infra: Finish moving to Mojo (2nd attempt)
> 
> This was originally landed in https://codereview.chromium.org/2694083005.
> But, it broke webview perf tests, and so, it was reverted. It turned out that
> in some scenarios, the memory dump manager is not initialized early enough (in
> the browser process). So, now, I initialize it as soon as the UI thread is
> created.
> 
> Modifications since the original CL are:
> 
> - The Coordinator service is created when the UI thread is created
>   (browser_main_loop.cc).
> - When Coordinator is created, it initializes memory dump manager's TPID
>   (memory_dump_manager_delegate_impl.cc).
> - To avoid dependency from //services/resource_coordinator to //content the
>   definition of the service TPID is moved from child_process_host.h to
>   constants.mojom.
> 
> I tested the CL by building for Android and running the following with an
> android device attached:
> 
> run_benchmark memory.top_10_mobile --browser=android-webview
> --also-run-disabled-tests
> 
> In the original reverted CL, the above command results in
> 'Unable to obtain memory dump' exceptions. With this CL the test runs
> successfully.
> 
> An example interaction diagram (to see the large image, click on it):
>
https://docs.google.com/document/d/1Mz64egjuZ4WsYw9AKKWdTvl0Il706EWdCy24V87R_...
> 
> BUG=679830, 697773, 697384, 697062
> 
> Review-Url: https://codereview.chromium.org/2734193002
> Cr-Commit-Position: refs/heads/master@{#455866}
> Committed:
https://chromium.googlesource.com/chromium/src/+/0a3a302b6df6458b21b3ce0951c5...

TBR=primiano@chromium.org,ssid@chromium.org,kenrb@chromium.org,jam@chromium.o...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=679830, 697773, 697384, 697062
==========