Delivering the FIRST_NONEMPTY_PAINT phase changing event to base/

content/browser/profiler_controller_impl.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/profiler_controller_impl.h"
 
 #include "base/bind.h"
 #include "base/process/process_handle.h"
 #include "base/tracked_objects.h"
 #include "content/common/child_process_messages.h"
@@ skipping 54 matching lines @@
   DCHECK(!subscriber_);
   subscriber_ = subscriber;
 }
 
 void ProfilerControllerImpl::Unregister(const ProfilerSubscriber* subscriber) {
   DCHECK_EQ(subscriber_, subscriber);
   subscriber_ = NULL;
 }
 
 void ProfilerControllerImpl::GetProfilerDataFromChildProcesses(
-    int sequence_number) {
+    int sequence_number,
+    int current_profiling_phase) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   int pending_processes = 0;
   for (BrowserChildProcessHostIterator iter; !iter.Done(); ++iter) {
     // In some cases, there may be no child process of the given type (for
     // example, the GPU process may not exist and there may instead just be a
     // GPU thread in the browser process). If that's the case, then the process
     // handle will be base::kNullProcessHandle and we shouldn't ask it for data.
     if (iter.GetData().handle == base::kNullProcessHandle)
       continue;
 
     ++pending_processes;
-    if (!iter.Send(new ChildProcessMsg_GetChildProfilerData(sequence_number)))
+    if (!iter.Send(new ChildProcessMsg_GetChildProfilerData(
+            sequence_number, current_profiling_phase))) {

[Ilya Sherman, 2015/04/07 01:15:30]

Do you understand why |sequence_number| is used in this IPC message?  If so,
could you please explain to me why it's needed here, and not needed for the new
code path?  If not, let's try to figure that out.

[vadimt, 2015/04/07 21:44:14]

See RequestContextMap. Sequence numbers are necessary to properly route
responses from child processes to corresponding callbacks.

Since the phase change broadcast doesn't take a callback, there is no need in
unique numbers.

       --pending_processes;
+    }
   }
 
   BrowserThread::PostTask(
       BrowserThread::UI,
       FROM_HERE,
       base::Bind(
           &ProfilerControllerImpl::OnPendingProcesses,
           base::Unretained(this),
           sequence_number,
           pending_processes,
           true));
 }
 
-void ProfilerControllerImpl::GetProfilerData(int sequence_number) {
+void ProfilerControllerImpl::NotifyChildProcessesOfProfilingPhaseCompletion(
+    int profiling_phase) {
   DCHECK_CURRENTLY_ON(BrowserThread::UI);

[Ilya Sherman, 2015/04/07 01:15:30]

Hmm, shouldn't this be the IO thread?  Are you not running with DCHECKS?

[vadimt, 2015/04/07 21:44:14]

Must have sneaked in since I ran this last time with UMA and phasing enabled;
thanks!
(I'd catch it later anyways, since I was planning another end-to-end test at the
end anyways.)

 
+  for (BrowserChildProcessHostIterator iter; !iter.Done(); ++iter) {
+    // In some cases, there may be no child process of the given type (for
+    // example, the GPU process may not exist and there may instead just be a
+    // GPU thread in the browser process). If that's the case, then the process
+    // handle will be base::kNullProcessHandle and we shouldn't send it a
+    // message.
+    if (iter.GetData().handle == base::kNullProcessHandle)
+      continue;
+
+    iter.Send(new ChildProcessMsg_OnProfilingPhase(profiling_phase));
+  }
+}
+
+void ProfilerControllerImpl::GetProfilerData(int sequence_number,
+                                             int current_profiling_phase) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[Ilya Sherman, 2015/04/07 01:15:30]

Please use DCHECK_CURRENTLY_ON

[vadimt, 2015/04/07 21:44:14]

Done.

+
+  // Iterates through renderers in UI thread, and through other child processes
+  // in IO thread, and send them GetChildProfilerData message. Renderers have to
+  // be contacted from UI thread, and other processes - from IO thread.
   int pending_processes = 0;
   for (RenderProcessHost::iterator it(RenderProcessHost::AllHostsIterator());
        !it.IsAtEnd(); it.Advance()) {
     ++pending_processes;
-    if (!it.GetCurrentValue()->Send(
-            new ChildProcessMsg_GetChildProfilerData(sequence_number))) {
+    if (!it.GetCurrentValue()->Send(new ChildProcessMsg_GetChildProfilerData(
+            sequence_number, current_profiling_phase))) {
       --pending_processes;
     }
   }
   OnPendingProcesses(sequence_number, pending_processes, false);
 
   BrowserThread::PostTask(
-      BrowserThread::IO,
-      FROM_HERE,
+      BrowserThread::IO, FROM_HERE,
       base::Bind(&ProfilerControllerImpl::GetProfilerDataFromChildProcesses,
-                 base::Unretained(this),
-                 sequence_number));
+                 base::Unretained(this), sequence_number,
+                 current_profiling_phase));
+}
+
+void ProfilerControllerImpl::OnProfilingPhaseCompletion(int profiling_phase) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

[Ilya Sherman, 2015/04/07 01:15:30]

Please use DCHECK_CURRENTLY_ON

[vadimt, 2015/04/07 21:44:14]

Done.

+
+  // Iterates through renderers in UI thread, and through other child processes
+  // in IO thread, and send them OnProfilingPhase message. Renderers have to be
+  // contacted from UI thread, and other processes - from IO thread.
+  for (RenderProcessHost::iterator it(RenderProcessHost::AllHostsIterator());
+       !it.IsAtEnd(); it.Advance()) {
+    it.GetCurrentValue()->Send(
+        new ChildProcessMsg_OnProfilingPhase(profiling_phase));
+  }
+
+  BrowserThread::PostTask(
+      BrowserThread::IO, FROM_HERE,
+      base::Bind(&ProfilerControllerImpl::
+                     NotifyChildProcessesOfProfilingPhaseCompletion,
+                 base::Unretained(this), profiling_phase));
 }
 
 }  // namespace content