Delivering the FIRST_NONEMPTY_PAINT phase changing event to base/

content/browser/profiler_controller_impl.cc

Index: content/browser/profiler_controller_impl.cc
diff --git a/content/browser/profiler_controller_impl.cc b/content/browser/profiler_controller_impl.cc
index 98fd2c068ac377ccde4b6025ab2758721c1fa738..ab2c23639d382e49249b8e1a51e8086a97b54e71 100644
--- a/content/browser/profiler_controller_impl.cc
+++ b/content/browser/profiler_controller_impl.cc
@@ -72,7 +72,8 @@ void ProfilerControllerImpl::Unregister(const ProfilerSubscriber* subscriber) {
 }
 
 void ProfilerControllerImpl::GetProfilerDataFromChildProcesses(
-    int sequence_number) {
+    int sequence_number,
+    int current_profiling_phase) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   int pending_processes = 0;
@@ -85,8 +86,10 @@ void ProfilerControllerImpl::GetProfilerDataFromChildProcesses(
       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(
@@ -100,26 +103,65 @@ void ProfilerControllerImpl::GetProfilerDataFromChildProcesses(
           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