Plugin Power Saver: Remove Size Recheck hack in plugin placeholders.

components/plugins/renderer/loadable_plugin_placeholder.cc

 // Copyright 2015 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 "components/plugins/renderer/loadable_plugin_placeholder.h"
 
-#include "base/auto_reset.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/json/string_escape.h"
 #include "base/strings/string_piece.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/thread_task_runner_handle.h"
 #include "base/values.h"
 #include "content/public/child/v8_value_converter.h"
 #include "content/public/renderer/render_frame.h"
@@ skipping 10 matching lines @@
 #include "third_party/WebKit/public/web/WebView.h"
 #include "url/gurl.h"
 #include "url/origin.h"
 
 using base::UserMetricsAction;
 using content::PluginInstanceThrottler;
 using content::RenderThread;
 
 namespace plugins {
 
-// TODO(tommycli): After a size update, re-check the size after this delay, as
-// Blink can report incorrect sizes to plugins while the compositing state is

[groby-ooo-7-16, 2015/12/15 01:28:13]

Do we not have the "incorrect sizes" issue any more? Just because it's not
reproing on your machine... :)

Did we have a test case/page for this?

[tommycli, 2015/12/15 02:09:23]

The incorrect size report bug has been mostly, but not completely, fixed:
https://code.google.com/p/chromium/issues/detail?id=343769

Most of the cases with incorrect size reports have been eliminated because of
https://codereview.chromium.org/1413523007/

The remaining cases I've filed as a bug here, and don't affect the PPS use case.
https://code.google.com/p/chromium/issues/detail?id=569172

I'm not 100% sure about the above. There are two browser tests that should start
heavily flaking if I'm wrong:

PluginPowerSaverBrowserTest.LargeCrossOriginObscured
PluginPowerSaverBrowserTest.PosterTests

But yes, I've mostly only tested on my local machine so far.

-// dirty. Chosen because it seems to work.
-const int kSizeChangeRecheckDelayMilliseconds = 100;
-
 void LoadablePluginPlaceholder::BlockForPowerSaverPoster() {
   DCHECK(!is_blocked_for_power_saver_poster_);
   is_blocked_for_power_saver_poster_ = true;
 
   render_frame()->RegisterPeripheralPlugin(
       url::Origin(GURL(GetPluginParams().url)),
       base::Bind(&LoadablePluginPlaceholder::MarkPluginEssential,
                  weak_factory_.GetWeakPtr(),
                  PluginInstanceThrottler::UNTHROTTLE_METHOD_BY_WHITELIST));
 }
@@ skipping 11 matching lines @@
     const blink::WebPluginParams& params,
     const std::string& html_data)
     : PluginPlaceholderBase(render_frame, frame, params, html_data),
       is_blocked_for_background_tab_(false),
       is_blocked_for_prerendering_(false),
       is_blocked_for_power_saver_poster_(false),
       power_saver_enabled_(false),
       premade_throttler_(nullptr),
       allow_loading_(false),
       finished_loading_(false),
-      in_size_recheck_(false),
       heuristic_run_before_(premade_throttler_ != nullptr),
       weak_factory_(this) {}
 
 LoadablePluginPlaceholder::~LoadablePluginPlaceholder() {
 }
 
 void LoadablePluginPlaceholder::MarkPluginEssential(
     PluginInstanceThrottler::PowerSaverUnthrottleMethod method) {
   if (!power_saver_enabled_)
     return;
@@ skipping 90 matching lines @@
   // Pass through JavaScript access to the underlying throttled plugin.
   if (premade_throttler_ && premade_throttler_->GetWebPlugin()) {
     return premade_throttler_->GetWebPlugin()->v8ScriptableObject(isolate);
   }
   return v8::Local<v8::Object>();
 }
 
 void LoadablePluginPlaceholder::OnUnobscuredRectUpdate(
     const gfx::Rect& unobscured_rect) {
   DCHECK(content::RenderThread::Get());
-  if (!power_saver_enabled_ || !finished_loading_)
+  if (!plugin() || !power_saver_enabled_ || !finished_loading_)
     return;
 
-  // Only update the unobscured rect during the recheck phase. Also early exit
-  // to prevent reentrancy issues.
-  if (in_size_recheck_) {
-    unobscured_rect_ = unobscured_rect;
+  if (unobscured_rect_ == unobscured_rect)
     return;
-  }
 
-  if (!size_update_timer_.IsRunning()) {
-    // TODO(tommycli): We have to post a delayed task to recheck the size, as
-    // Blink can report wrong sizes for partially obscured plugins while the
-    // compositing state is dirty. https://crbug.com/343769
-    size_update_timer_.Start(
-        FROM_HERE,
-        base::TimeDelta::FromMilliseconds(kSizeChangeRecheckDelayMilliseconds),
-        base::Bind(&LoadablePluginPlaceholder::RecheckSizeAndMaybeUnthrottle,
-                   weak_factory_.GetWeakPtr()));
+  unobscured_rect_ = unobscured_rect;
+
+  float zoom_factor = plugin()->container()->pageZoomFactor();
+  int width = roundf(unobscured_rect_.width() / zoom_factor);
+  int height = roundf(unobscured_rect_.height() / zoom_factor);
+
+  if (is_blocked_for_power_saver_poster_) {
+    // Adjust poster container padding and dimensions to center play button for
+    // plugins and plugin posters that have their top or left portions obscured.
+    int x = roundf(unobscured_rect_.x() / zoom_factor);
+    int y = roundf(unobscured_rect_.y() / zoom_factor);
+    std::string script = base::StringPrintf(
+        "window.resizePoster('%dpx', '%dpx', '%dpx', '%dpx')", x, y, width,
+        height);
+    plugin()->web_view()->mainFrame()->executeScript(
+        blink::WebScriptSource(base::UTF8ToUTF16(script)));
+
+    // On a size update check if we now qualify as a essential plugin.
+    url::Origin content_origin = url::Origin(GetPluginParams().url);
+    auto status = render_frame()->GetPeripheralContentStatus(
+        render_frame()->GetWebFrame()->top()->securityOrigin(), content_origin,
+        gfx::Size(width, height));
+    if (status != content::RenderFrame::CONTENT_STATUS_PERIPHERAL) {
+      MarkPluginEssential(
+          heuristic_run_before_
+              ? PluginInstanceThrottler::UNTHROTTLE_METHOD_BY_SIZE_CHANGE
+              : PluginInstanceThrottler::UNTHROTTLE_METHOD_DO_NOT_RECORD);
+
+      if (!heuristic_run_before_ &&
+          status ==
+              content::RenderFrame::CONTENT_STATUS_ESSENTIAL_CROSS_ORIGIN_BIG) {
+        render_frame()->WhitelistContentOrigin(content_origin);
+      }
+    }
+
+    heuristic_run_before_ = true;
   }
 }
 
 void LoadablePluginPlaceholder::WasShown() {
   if (is_blocked_for_background_tab_) {
     is_blocked_for_background_tab_ = false;
     if (!LoadingBlocked())
       LoadPlugin();
   }
 }
@@ skipping 95 matching lines @@
 const std::string& LoadablePluginPlaceholder::GetIdentifier() const {
   return identifier_;
 }
 
 bool LoadablePluginPlaceholder::LoadingBlocked() const {
   DCHECK(allow_loading_);
   return is_blocked_for_background_tab_ || is_blocked_for_power_saver_poster_ ||
          is_blocked_for_prerendering_;
 }
 
-void LoadablePluginPlaceholder::RecheckSizeAndMaybeUnthrottle() {
-  DCHECK(content::RenderThread::Get());
-  DCHECK(!in_size_recheck_);
-  DCHECK(finished_loading_);
-
-  base::AutoReset<bool> recheck_scope(&in_size_recheck_, true);
-
-  if (!plugin())
-    return;
-
-  gfx::Rect old_rect = unobscured_rect_;
-
-  // Re-check the size in case the reported size was incorrect.
-  plugin()->container()->reportGeometry();
-
-  if (old_rect == unobscured_rect_)
-    return;
-
-  float zoom_factor = plugin()->container()->pageZoomFactor();
-  int width = roundf(unobscured_rect_.width() / zoom_factor);
-  int height = roundf(unobscured_rect_.height() / zoom_factor);
-
-  if (is_blocked_for_power_saver_poster_) {
-    // Adjust poster container padding and dimensions to center play button for
-    // plugins and plugin posters that have their top or left portions obscured.
-    int x = roundf(unobscured_rect_.x() / zoom_factor);
-    int y = roundf(unobscured_rect_.y() / zoom_factor);
-    std::string script = base::StringPrintf(
-        "window.resizePoster('%dpx', '%dpx', '%dpx', '%dpx')", x, y, width,
-        height);
-    plugin()->web_view()->mainFrame()->executeScript(
-        blink::WebScriptSource(base::UTF8ToUTF16(script)));
-
-    // On a size update check if we now qualify as a essential plugin.
-    url::Origin content_origin = url::Origin(GetPluginParams().url);
-    auto status = render_frame()->GetPeripheralContentStatus(
-        render_frame()->GetWebFrame()->top()->securityOrigin(), content_origin,
-        gfx::Size(width, height));
-    if (status != content::RenderFrame::CONTENT_STATUS_PERIPHERAL) {
-      MarkPluginEssential(
-          heuristic_run_before_
-              ? PluginInstanceThrottler::UNTHROTTLE_METHOD_BY_SIZE_CHANGE
-              : PluginInstanceThrottler::UNTHROTTLE_METHOD_DO_NOT_RECORD);
-
-      if (!heuristic_run_before_ &&
-          status ==
-              content::RenderFrame::CONTENT_STATUS_ESSENTIAL_CROSS_ORIGIN_BIG) {
-        render_frame()->WhitelistContentOrigin(content_origin);
-      }
-    }
-
-    heuristic_run_before_ = true;
-  }
-}
-
 }  // namespace plugins