Fix bug causing blank pages on resume of Chrome for Android.

content/browser/android/content_view_statics.cc

Index: content/browser/android/content_view_statics.cc
diff --git a/content/browser/android/content_view_statics.cc b/content/browser/android/content_view_statics.cc
index e176564c4a0f5a430aad40adaddc2d7aef806c05..cafcd606194f7593c05e611e23ebc3dbf9d49b3c 100644
--- a/content/browser/android/content_view_statics.cc
+++ b/content/browser/android/content_view_statics.cc
@@ -15,6 +15,7 @@
 #include "content/common/android/address_parser.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_process_host_observer.h"
 #include "jni/ContentViewStatics_jni.h"
 
 using base::android::ConvertJavaStringToUTF16;
@@ -37,6 +38,28 @@ namespace {
 base::LazyInstance<std::vector<int /* process id */> > g_suspended_processes =

[Philippe, 2014/02/25 09:41:12]

Nit: s/process id/RPH ID (see my comment below).

[Yaron, 2014/02/25 18:14:48]

Done.

     LAZY_INSTANCE_INITIALIZER;
 
+// Listens for RenderProcessHosts who's renderers have crashed. This is

[Philippe, 2014/02/25 09:41:12]

You mean in your CL description that the same RenderProcessHost instance is kept
alive when a renderer process gets killed and re-spawned, right?

It looks to me that we may have had this in mind when landing this CL. We did
completely screw up though by using RenderProcessHost::GetID(). By looking at
RenderProcessHostImpl the ID is constant (and set to the initial renderer
process PID) as opposed to the current renderer process' PID backing the RPH (as
I/we were expecting). Having a non-constant ID would not be sane either though
:) I suspect that we should have used RPH::GetHandle() in the first place as
opposed to RPH::GetID() but there is no RPH::FromHandle() method...

So removing the RPH IDs from the vector as the renderer processes get killed
like you did looks like the best/only solution IMO (modulo doing the refactoring
mentioned line 26 of course :).

[Philippe, 2014/02/25 09:41:12]

Nit: s/who's/whose I believe although my knowledge in English is quite limited
:)

[Yaron, 2014/02/25 18:14:48]

Done.

[Yaron, 2014/02/25 18:14:48]

It's not actually the renderer process pid. It's just a monotonically increasing
id. Ya, I came to the same conclusion about GetHandle when looking for an
alternate way.

+// necessary to ensure that we don't over-resume a RenderProcess as described
+// above.
+class SuspendedProcessWatcher : public content::RenderProcessHostObserver {
+  virtual void RenderProcessExited(content::RenderProcessHost* host,
+                                    base::ProcessHandle handle,

[Philippe, 2014/02/25 09:41:12]

Nit: extra leading space here and on the two lines below.

[Yaron, 2014/02/25 18:14:48]

Done.

+                                    base::TerminationStatus status,
+                                    int exit_code) OVERRIDE {
+    std::vector<int>* suspended_processes = g_suspended_processes.Pointer();
+    std::vector<int>::iterator pos = std::find(suspended_processes->begin(),
+                                               suspended_processes->end(),
+                                               host->GetID());
+    if (pos != suspended_processes->end()) {
+      host->RemoveObserver(this);
+      suspended_processes->erase(pos);
+    }
+  }
+};
+
+base::LazyInstance<SuspendedProcessWatcher > g_suspended_processes_watcher =

[Philippe, 2014/02/25 09:41:12]

Nit: extra space before '>'.

[Yaron, 2014/02/25 18:14:48]

Done.

+    LAZY_INSTANCE_INITIALIZER;
+
 // Suspends timers in all current render processes.
 void SuspendWebKitSharedTimers(std::vector<int>* suspended_processes) {
   for (content::RenderProcessHost::iterator i(
@@ -45,6 +68,7 @@ void SuspendWebKitSharedTimers(std::vector<int>* suspended_processes) {
     content::RenderProcessHost* host = i.GetCurrentValue();
     suspended_processes->push_back(host->GetID());
     host->Send(new ViewMsg_SetWebKitSharedTimersSuspended(true));
+    host->AddObserver(g_suspended_processes_watcher.Pointer());
   }
 }