Resume WebKit shared timers only in previously suspended processes.

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 0fb08964dea3514f57c01e6fe384a81a52edd89d..7f83c8df5e75fc74a06b91620ecb7d1b8138b483 100644
--- a/content/browser/android/content_view_statics.cc
+++ b/content/browser/android/content_view_statics.cc
@@ -3,25 +3,60 @@
 // found in the LICENSE file.
 
 #include <jni.h>
+#include <vector>
 
 #include "base/android/jni_android.h"
 #include "base/android/jni_string.h"
 #include "base/android/scoped_java_ref.h"
 #include "base/basictypes.h"
+#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "content/browser/android/content_view_statics.h"
 #include "content/common/android/address_parser.h"
 #include "content/common/view_messages.h"
 #include "content/public/browser/render_process_host.h"
-
 #include "jni/ContentViewStatics_jni.h"
 
 using base::android::ConvertJavaStringToUTF16;
 using base::android::ConvertUTF16ToJavaString;
-using base::android::ScopedJavaLocalRef;
 
+namespace {
+
+// TODO(pliard): Move WebKit shared timer toggling functionnality out of
+// ContentViewStatistics.

[joth, 2013/04/26 17:57:10]

and perhaps add "and not be build on top of
WebKit::Platform::SuspendSharedTimer"
- ISTM that this whole approach is misusing an interface that is intended for
webkit to control the platform, not for the embedder to modify webkit policy,
hence the fragility of it.
(Writing a test maybe much easier if the two concepts are separated too)

[Philippe, 2013/04/26 18:03:07]

Very good point.

+// TODO(pliard): Add unit tests for WebKit shared timer toggling.
+
+// This tracks the renderer processes that received a suspend request. It's
+// important on resume to only resume the renderer processes that were actually
+// suspended as opposed to all the current renderer processes.

[joth, 2013/04/26 17:57:10]

nit: append why it's important: "because the suspend calls are refcounted within
WebKitPlatformSupport and it expects a perfectly matched number of resume calls"

[Philippe, 2013/04/26 18:03:07]

Done.

+// Note that this vector is only accessed from the UI thread.
+base::LazyInstance<std::vector<int /* process id */> > g_suspended_processes =
+    LAZY_INSTANCE_INITIALIZER;
+
+// 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));
+  }
+}
+
+// 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.
+      host->Send(new ViewMsg_SetWebKitSharedTimersSuspended(false));
+  }
+}
+
+}  // namespace
 
-// Return the first substring consisting of the address of a physical location.
+// Returns the first substring consisting of the address of a physical location.
 static jstring FindAddress(JNIEnv* env, jclass clazz, jstring addr) {
   string16 content_16 = ConvertJavaStringToUTF16(env, addr);
   string16 result_16;
@@ -33,12 +68,13 @@ static jstring FindAddress(JNIEnv* env, jclass clazz, jstring addr) {
 static void SetWebKitSharedTimersSuspended(JNIEnv* env,
                                            jclass obj,
                                            jboolean suspend) {
-  for (content::RenderProcessHost::iterator i =
-           content::RenderProcessHost::AllHostsIterator();
-       !i.IsAtEnd();
-       i.Advance()) {
-    content::RenderProcessHost* host = i.GetCurrentValue();
-    host->Send(new ViewMsg_SetWebKitSharedTimersSuspended(suspend));
+  std::vector<int>* suspended_processes = &g_suspended_processes.Get();

[joth, 2013/04/26 17:57:10]

foo = g_suspended_processes.Pointer()

[Philippe, 2013/04/26 18:03:07]

Ah, I always wondered why LazyInstance::Get() was returning a forbidden
non-const reference but I see now :)

+  if (suspend) {
+    DCHECK(suspended_processes->empty());
+    SuspendWebKitSharedTimers(suspended_processes);
+  } else {
+    ResumeWebkitSharedTimers(*suspended_processes);
+    suspended_processes->clear();
   }
 }