Disable PurgeAndSuspend when MemoryCoordinator is enabled.

content/renderer/render_thread_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/renderer/render_thread_impl.h"
 
 #include <algorithm>
 #include <limits>
 #include <map>
 #include <utility>
@@ skipping 1760 matching lines @@
     record_purge_suspend_metric_closure_.Reset(
         base::Bind(&RenderThreadImpl::RecordPurgeAndSuspendMetrics,
                    base::Unretained(this)));
   }
 }
 
 void RenderThreadImpl::OnProcessPurgeAndSuspend() {
   ChildThreadImpl::OnProcessPurgeAndSuspend();
   if (!RendererIsHidden())
     return;
+
+  // Disable PurgeAndSuspend and not record UMA to avoid conflicting
+  // MemoryCoordinator finch experiment when MemoryCoordinator is enabled.

[haraken, 2016/12/21 04:03:23]

Just say:

TODO(bashi): Enable the tab suspension when MemoryCoordinator is enabled.

[tasak, 2016/12/22 04:49:52]

Done.

+  if (base::FeatureList::IsEnabled(features::kMemoryCoordinator))

[bashi, 2016/12/21 08:44:02]

Why do we need this guard? Isn't having a guard in SuspendRenderer() enough?

[tasak, 2016/12/21 08:49:31]

To avoid invoking record_purge_suspend_metric_closure_.
If we invoke this, PurgeAndSuspend finch experiment has wrong data (i.e.
PurgeAndSuspend is enabled but no purge / suspend renderer is invoked).

This means, PurgeAndSuspend finch experiment runs with 25% control and 25%
experiment.

[bashi, 2016/12/21 08:55:32]

How about calling PostDelayedTask() in SuspendRenderer then?

[tasak, 2016/12/21 08:58:28]

To enable A/B testing, the PostDelayedTask is also needed to invoke when
PurgeAndSuspend is disabled (=control of PurgeAndSuspend finch experiment).

[bashi, 2016/12/21 09:02:41]

OK

+    return;
+
   if (base::FeatureList::IsEnabled(features::kPurgeAndSuspend)) {
-    // TODO(tasak,bashi): After enabling MemoryCoordinator, stop calling
-    // SuspendRenderer() here.
-    SuspendRenderer();
+    // TODO(tasak): After enabling MemoryCoordinator, remove this Notify
+    // and follow MemoryCoordinator's request.
+    base::MemoryCoordinatorClientRegistry::GetInstance()->Notify(
+        base::MemoryState::SUSPENDED);
   }
   // Since purging is not a synchronous task (e.g. v8 GC, oilpan GC, ...),
   // we need to wait until the task is finished. So wait 15 seconds and
   // update purge+suspend UMA histogram.
   // TODO(tasak): use MemoryCoordinator's callback to report purge+suspend
   // UMA when MemoryCoordinator is available.
   GetRendererScheduler()->DefaultTaskRunner()->PostDelayedTask(
       FROM_HERE, record_purge_suspend_metric_closure_.callback(),
       base::TimeDelta::FromSeconds(15));
 }
@@ skipping 86 matching lines @@
                            blink_stats.blinkGCTotalAllocatedBytes +
                            malloc_usage + v8_usage + discardable_usage) /
                               1024 / 1024);
 }
 
 void RenderThreadImpl::OnProcessResume() {
   ChildThreadImpl::OnProcessResume();
 
   if (!RendererIsHidden())
     return;
+
+  // Disable PurgeAndSuspend and not record UMA to avoid conflicting
+  // MemoryCoordinator finch experiment when MemoryCoordinator is enabled.

[haraken, 2016/12/21 04:03:23]

Ditto.

[tasak, 2016/12/22 04:49:52]

Done.

+  if (base::FeatureList::IsEnabled(features::kMemoryCoordinator))
+    return;
+
   if (base::FeatureList::IsEnabled(features::kPurgeAndSuspend)) {
-    // TODO(tasak,bashi): After enabling MemoryCoordinator, stop calling
-    // ResumeRenderer() here.
-    ResumeRenderer();
+    // TODO(tasak): after enabling MemoryCoordinator, remove this Notify
+    // and follow MemoryCoordinator's request.
+    base::MemoryCoordinatorClientRegistry::GetInstance()->Notify(
+        base::MemoryState::NORMAL);
   }
 }
 
 scoped_refptr<gpu::GpuChannelHost> RenderThreadImpl::EstablishGpuChannelSync() {
   TRACE_EVENT0("gpu", "RenderThreadImpl::EstablishGpuChannelSync");
 
   if (gpu_channel_) {
     // Do nothing if we already have a GPU channel or are already
     // establishing one.
     if (!gpu_channel_->IsLost())
@@ skipping 346 matching lines @@
 void RenderThreadImpl::OnMemoryStateChange(base::MemoryState state) {
   // TODO(hajimehoshi): Adjust the size of this memory usage according to
   // |state|. RenderThreadImpl doesn't have a feature to limit memory usage at
   // present.
   if (blink_platform_impl_) {
     blink::WebMemoryCoordinator::onMemoryStateChange(
         static_cast<blink::MemoryState>(state));
   }
   switch (state) {
     case base::MemoryState::NORMAL:
-      // TODO(bashi): When the renderer is suspended, resume it.
+      ResumeRenderer();
       break;
     case base::MemoryState::THROTTLED:
-      // TODO(bashi): When the renderer is suspended, resume it.
+      ResumeRenderer();
       // TODO(bashi): Figure out what kind of strategy is suitable on
       // THROTTLED state. crbug.com/674815
 #if defined(OS_ANDROID)
       OnTrimMemoryImmediately();
 #else
       OnSyncMemoryPressure(
           base::MemoryPressureListener::MEMORY_PRESSURE_LEVEL_MODERATE);
 #endif
       ReleaseFreeMemory();
       break;
     case base::MemoryState::SUSPENDED:
-      // TODO(bashi): Suspend the renderer.
-      OnTrimMemoryImmediately();
-      ReleaseFreeMemory();
-      ClearMemory();
+      SuspendRenderer();
       break;
     case base::MemoryState::UNKNOWN:
       NOTREACHED();
       break;
   }
 }
 
 void RenderThreadImpl::SuspendRenderer() {
   DCHECK(IsMainThread());
   OnTrimMemoryImmediately();
   ReleaseFreeMemory();
   ClearMemory();
-  renderer_scheduler_->SuspendRenderer();
+  // When only PurgeAndSuspend is enabled, suspend blink scheduler.

[haraken, 2016/12/21 04:03:23]

Ditto.

[tasak, 2016/12/22 04:49:52]

Done.

+  if (!base::FeatureList::IsEnabled(features::kMemoryCoordinator) &&
+      base::FeatureList::IsEnabled(features::kPurgeAndSuspend))
+    renderer_scheduler_->SuspendRenderer();
 }
 
 void RenderThreadImpl::ResumeRenderer() {
   DCHECK(IsMainThread());
-  renderer_scheduler_->ResumeRenderer();
+  // When only PurgeAndSuspend is enabled, resume blink scheduler.

[haraken, 2016/12/21 04:03:23]

Ditto.

[tasak, 2016/12/22 04:49:52]

Done.

+  if (!base::FeatureList::IsEnabled(features::kMemoryCoordinator) &&
+      base::FeatureList::IsEnabled(features::kPurgeAndSuspend))
+    renderer_scheduler_->ResumeRenderer();
 }
 
 void RenderThreadImpl::ClearMemory() {
   // Do not call into blink if it is not initialized.
   if (blink_platform_impl_) {
     // Purge Skia font cache, by setting it to 0 and then again to the
     // previous limit.
     size_t font_cache_limit = SkGraphics::SetFontCacheLimit(0);
     SkGraphics::SetFontCacheLimit(font_cache_limit);
   }
@@ skipping 179 matching lines @@
   }
 }
 
 void RenderThreadImpl::OnRendererInterfaceRequest(
     mojom::RendererAssociatedRequest request) {
   DCHECK(!renderer_binding_.is_bound());
   renderer_binding_.Bind(std::move(request));
 }
 
 }  // namespace content