memory coordinator: Resume suspended renderer on NORMAL state change

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 1761 matching lines @@
     // TODO(tasak): after enabling MemoryCoordinator, remove this Notify
     // and follow MemoryCoordinator's request.
     if (base::FeatureList::IsEnabled(features::kPurgeAndSuspend))
       base::MemoryCoordinatorClientRegistry::GetInstance()->Notify(
           base::MemoryState::NORMAL);
 
     record_purge_suspend_metric_closure_.Cancel();
     record_purge_suspend_metric_closure_.Reset(
         base::Bind(&RenderThreadImpl::RecordPurgeAndSuspendMetrics,
                    base::Unretained(this)));
-    is_renderer_suspended_ = false;
   }
 }
 
 void RenderThreadImpl::OnProcessPurgeAndSuspend() {
   ChildThreadImpl::OnProcessPurgeAndSuspend();
   DCHECK(!is_renderer_suspended_);
   if (!RendererIsHidden())
     return;
-  is_renderer_suspended_ = true;
   if (base::FeatureList::IsEnabled(features::kPurgeAndSuspend)) {
     // 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
@@ skipping 90 matching lines @@
                           (blink_stats.partitionAllocTotalAllocatedBytes +
                            blink_stats.blinkGCTotalAllocatedBytes +
                            malloc_usage + v8_usage + discardable_usage) /
                               1024 / 1024);
 }
 
 void RenderThreadImpl::OnProcessResume() {
   ChildThreadImpl::OnProcessResume();
 
   DCHECK(is_renderer_suspended_);
-  is_renderer_suspended_ = false;
   if (base::FeatureList::IsEnabled(features::kPurgeAndSuspend)) {
     // TODO(tasak): after enabling MemoryCoordinator, remove this Notify
     // and follow MemoryCoordinator's request.
     base::MemoryCoordinatorClientRegistry::GetInstance()->Notify(
         base::MemoryState::NORMAL);
-    renderer_scheduler_->ResumeRenderer();
   }
 }
 
 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 371 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:
+      ResumeRendererIfNeeded();
       break;
     case base::MemoryState::THROTTLED:
+      ResumeRendererIfNeeded();
       ReleaseFreeMemory();
       break;
     case base::MemoryState::SUSPENDED:
       SuspendRenderer();
       break;
     case base::MemoryState::UNKNOWN:
       NOTREACHED();
       break;
   }
 }
 
 void RenderThreadImpl::SuspendRenderer() {
+  DCHECK(IsMainThread());
+  // |is_renderer_suspende_| shouldn't be true here, but it could due to IPC
+  // message lag.
+  if (is_renderer_suspended_)

[bashi, 2016/12/16 07:50:39]

Chatted with tasak@ offline and he said we can't DCHECK here due to IPC lag.
Changed to early return.

[tasak, 2016/12/16 07:55:15]

I was trying to remove DCHECK related to is_renderer_suspended. c.f.
https://codereview.chromium.org/2564833002/
I missed checking whether the patch was successfully committed or not...

... the patch removes is_renderer_suspended_, but this CL needs. I will remove
only DCHECK in OnProcessResume and OnProcessPurgeAndSuspend.

[bashi, 2016/12/16 08:03:13]

Thanks for the info. Then we should do early return in
RendererSchedulerImpl::ResumeRenderer(). I'll wait for your CL to land.

+    return;
   OnTrimMemoryImmediately();
   ReleaseFreeMemory();
   ClearMemory();
+  is_renderer_suspended_ = true;
   renderer_scheduler_->SuspendRenderer();
 }
 
+void RenderThreadImpl::ResumeRendererIfNeeded() {
+  DCHECK(IsMainThread());
+  if (!is_renderer_suspended_)
+    return;
+  renderer_scheduler_->ResumeRenderer();
+  is_renderer_suspended_ = false;
+}
+
 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 177 matching lines @@
   }
 }
 
 void RenderThreadImpl::OnRendererInterfaceRequest(
     mojom::RendererAssociatedRequest request) {
   DCHECK(!renderer_binding_.is_bound());
   renderer_binding_.Bind(std::move(request));
 }
 
 }  // namespace content