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..aa7ebff397f0efbe4ab33b6500eb57f5312dc5a6 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;
@@ -33,30 +34,59 @@ namespace {
 // suspended as opposed to all the current renderer processes because the
 // suspend calls are refcounted within WebKitPlatformSupport and it expects a
 // perfectly matched number of resume calls.
-// Note that this vector is only accessed from the UI thread.
-base::LazyInstance<std::vector<int /* process id */> > g_suspended_processes =
-    LAZY_INSTANCE_INITIALIZER;
+// Note that this class is only accessed from the UI thread.
+class SuspendedProcessWatcher : public content::RenderProcessHostObserver {
+ public:
 
-// Suspends timers in all current render processes.
-void SuspendWebKitSharedTimers(std::vector<int>* suspended_processes) {
-  for (content::RenderProcessHost::iterator i(
-          content::RenderProcessHost::AllHostsIterator());
-       !i.IsAtEnd(); i.Advance()) {
-    content::RenderProcessHost* host = i.GetCurrentValue();
-    suspended_processes->push_back(host->GetID());
-    host->Send(new ViewMsg_SetWebKitSharedTimersSuspended(true));
+  // If the process crashes, stop watching the corresponding RenderProcessHost
+  // and ensure it doesn't get over-resumed.
+  virtual void RenderProcessExited(content::RenderProcessHost* host,
+                                   base::ProcessHandle handle,
+                                   base::TerminationStatus status,
+                                   int exit_code) OVERRIDE {
+    std::vector<int>::iterator pos = std::find(suspended_processes_.begin(),
+                                               suspended_processes_.end(),
+                                               host->GetID());
+    DCHECK_NE(pos, suspended_processes_.end());
+    host->RemoveObserver(this);
+    suspended_processes_.erase(pos);
+  }
+
+  // Suspends timers in all current render processes.
+  void SuspendWebKitSharedTimers() {
+    DCHECK(suspended_processes_.empty());
+
+    for (content::RenderProcessHost::iterator i(
+            content::RenderProcessHost::AllHostsIterator());
+         !i.IsAtEnd(); i.Advance()) {
+      content::RenderProcessHost* host = i.GetCurrentValue();
+      if (host->HasConnection()) {

[benm (inactive), 2014/06/17 17:03:02]

Why do we add this check here?

This seems to be causing a compatibility break for WebView where an app calls
pauseTimers before navigating to the initial document and expects that page to
be loaded with the shared timer paused. Do you think that removing it would
cause an issue for Chrome?

+        host->AddObserver(this);
+        host->Send(new ViewMsg_SetWebKitSharedTimersSuspended(true));
+        suspended_processes_.push_back(host->GetID());
+      }
+    }
   }
-}
 
-// Resumes timers in processes that were previously stopped.
-void ResumeWebkitSharedTimers(const std::vector<int>& suspended_processes) {
-  for (std::vector<int>::const_iterator it = suspended_processes.begin();
-       it != suspended_processes.end(); ++it) {
-    content::RenderProcessHost* host = content::RenderProcessHost::FromID(*it);
-    if (host)  // The process might have been killed since it was suspended.
+  // Resumes timers in processes that were previously stopped.
+  void ResumeWebkitSharedTimers() {
+    for (std::vector<int>::const_iterator it = suspended_processes_.begin();
+         it != suspended_processes_.end(); ++it) {
+      content::RenderProcessHost* host =
+          content::RenderProcessHost::FromID(*it);
+      DCHECK(host);
+      host->RemoveObserver(this);
       host->Send(new ViewMsg_SetWebKitSharedTimersSuspended(false));
+    }
+    suspended_processes_.clear();
   }
-}
+
+ private:
+  std::vector<int /* RenderProcessHost id */> suspended_processes_;
+};
+
+base::LazyInstance<SuspendedProcessWatcher> g_suspended_processes_watcher =
+    LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
@@ -72,20 +102,17 @@ static jstring FindAddress(JNIEnv* env, jclass clazz, jstring addr) {
 static void SetWebKitSharedTimersSuspended(JNIEnv* env,
                                            jclass obj,
                                            jboolean suspend) {
-  std::vector<int>* suspended_processes = g_suspended_processes.Pointer();
   if (suspend) {
-    DCHECK(suspended_processes->empty());
-    SuspendWebKitSharedTimers(suspended_processes);
+    g_suspended_processes_watcher.Pointer()->SuspendWebKitSharedTimers();
   } else {
-    ResumeWebkitSharedTimers(*suspended_processes);
-    suspended_processes->clear();
+    g_suspended_processes_watcher.Pointer()->ResumeWebkitSharedTimers();
   }
 }
 
 namespace content {
 
 bool RegisterWebViewStatics(JNIEnv* env) {
-  return RegisterNativesImpl(env) >= 0;
+  return RegisterNativesImpl(env);
 }
 
 }  // namespace content