Plugin Power Saver: Fix double-counting NEVER case in LoadablePluginPlaceholder.

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/bind.h"
 #include "base/bind_helpers.h"
 #include "base/json/string_escape.h"
 #include "base/strings/string_piece.h"
@@ skipping 29 matching lines @@
 void LoadablePluginPlaceholder::BlockForPowerSaverPoster() {
   DCHECK(!is_blocked_for_power_saver_poster_);
   is_blocked_for_power_saver_poster_ = true;
 
   render_frame()->RegisterPeripheralPlugin(
       GURL(GetPluginParams().url).GetOrigin(),
       base::Bind(&LoadablePluginPlaceholder::MarkPluginEssential,
                  weak_factory_.GetWeakPtr(),
                  PluginInstanceThrottler::UNTHROTTLE_METHOD_BY_WHITELIST));
 }
-#endif
 
 void LoadablePluginPlaceholder::SetPremadePlugin(
     content::PluginInstanceThrottler* throttler) {
   DCHECK(throttler);
   DCHECK(!premade_throttler_);
   premade_throttler_ = throttler;
-
-  premade_throttler_->AddObserver(this);
 }
+#endif
 
 LoadablePluginPlaceholder::LoadablePluginPlaceholder(
     content::RenderFrame* render_frame,
     WebLocalFrame* frame,
     const WebPluginParams& params,
     const std::string& html_data,
     GURL placeholderDataUrl)
     : PluginPlaceholder(render_frame,
                         frame,
                         params,
                         html_data,
                         placeholderDataUrl),
       is_blocked_for_background_tab_(false),
       is_blocked_for_prerendering_(false),
       is_blocked_for_power_saver_poster_(false),
       power_saver_enabled_(false),
-      plugin_marked_essential_(false),
       premade_throttler_(nullptr),
       allow_loading_(false),
-      placeholder_was_replaced_(false),
       hidden_(false),
       finished_loading_(false),
       weak_factory_(this) {
 }
 
 LoadablePluginPlaceholder::~LoadablePluginPlaceholder() {
-#if defined(ENABLE_PLUGINS)
-  DCHECK(!premade_throttler_);
-
-  if (!plugin_marked_essential_ && !placeholder_was_replaced_ &&
-      !is_blocked_for_prerendering_ && is_blocked_for_power_saver_poster_) {
-    PluginInstanceThrottler::RecordUnthrottleMethodMetric(
-        PluginInstanceThrottler::UNTHROTTLE_METHOD_NEVER);
-  }
-#endif
 }
 
 #if defined(ENABLE_PLUGINS)
 void LoadablePluginPlaceholder::MarkPluginEssential(
     PluginInstanceThrottler::PowerSaverUnthrottleMethod method) {
-  if (plugin_marked_essential_)
+  if (!power_saver_enabled_)
     return;
 
-  plugin_marked_essential_ = true;
+  power_saver_enabled_ = false;
+
   if (premade_throttler_)
     premade_throttler_->MarkPluginEssential(method);
   else
     PluginInstanceThrottler::RecordUnthrottleMethodMetric(method);
 
   if (is_blocked_for_power_saver_poster_) {
     is_blocked_for_power_saver_poster_ = false;
     if (!LoadingBlocked())
       LoadPlugin();
   }
@@ skipping 27 matching lines @@
     return;
   }
 
   // The plugin has been removed from the page. Destroy the old plugin. We
   // will be destroyed as soon as V8 garbage collects us.
   if (!element.pluginContainer()) {
     plugin()->destroy();
     return;
   }
 
-  placeholder_was_replaced_ = true;
-
   // During initialization, the new plugin might have replaced itself in turn
   // with another plugin. Make sure not to use the passed in |new_plugin| after
   // this point.
   new_plugin = container->plugin();
 
   plugin()->RestoreTitleText();
   container->invalidate();
   container->reportGeometry();
   plugin()->ReplayReceivedData(new_plugin);
   plugin()->destroy();
@@ skipping 56 matching lines @@
 void LoadablePluginPlaceholder::UpdateMessage() {
   if (!plugin())
     return;
   std::string script =
       "window.setMessage(" + base::GetQuotedJSONString(message_) + ")";
   plugin()->web_view()->mainFrame()->executeScript(
       WebScriptSource(base::UTF8ToUTF16(script)));
 }
 
 void LoadablePluginPlaceholder::PluginDestroyed() {
-  // Since the premade plugin has been detached from the container, it will not
-  // be automatically destroyed along with the page.
-  if (!placeholder_was_replaced_ && premade_throttler_) {
-    premade_throttler_->RemoveObserver(this);
-    premade_throttler_->GetWebPlugin()->destroy();
-    premade_throttler_ = nullptr;
+#if defined(ENABLE_PLUGINS)
+  if (power_saver_enabled_) {

[Bernhard Bauer, 2015/04/29 10:46:00]

Early-return if |power_saver_enabled_| is false?

[tommycli, 2015/04/29 18:17:28]

Err well we still want to call PluginPlaceholder::PluginDestroyed() at the very
end even if power_saver_enabled_ is false.

[Bernhard Bauer, 2015/04/29 18:53:26]

Oh, durr.

+    if (premade_throttler_) {
+      // Since the premade plugin has been detached from the container, it will
+      // not be automatically destroyed along with the page.
+      premade_throttler_->GetWebPlugin()->destroy();
+      premade_throttler_ = nullptr;
+    } else if (is_blocked_for_power_saver_poster_) {
+      // Record the NEVER unthrottle count only if there is no throttler.
+      PluginInstanceThrottler::RecordUnthrottleMethodMetric(
+          PluginInstanceThrottler::UNTHROTTLE_METHOD_NEVER);
+    }
+
+    // Prevent processing subsequent calls to MarkPluginEssential.
+    power_saver_enabled_ = false;
   }
+#endif
 
   PluginPlaceholder::PluginDestroyed();
 }
 
 void LoadablePluginPlaceholder::WasShown() {
   if (is_blocked_for_background_tab_) {
     is_blocked_for_background_tab_ = false;
     if (!LoadingBlocked())
       LoadPlugin();
   }
 }
 
-void LoadablePluginPlaceholder::OnThrottleStateChange() {
-  DCHECK(premade_throttler_);
-  if (!premade_throttler_->IsThrottled()) {
-    // Premade plugin has been unthrottled externally (by audio playback, etc.).
-    LoadPlugin();
-  }
-}
-
 void LoadablePluginPlaceholder::OnLoadBlockedPlugins(
     const std::string& identifier) {
   if (!identifier.empty() && identifier != identifier_)
     return;
 
   RenderThread::Get()->RecordAction(UserMetricsAction("Plugin_Load_UI"));
   LoadPlugin();
 }
 
 void LoadablePluginPlaceholder::OnSetIsPrerendering(bool is_prerendering) {
@@ skipping 13 matching lines @@
   if (hidden_)
     return;
   if (!plugin())
     return;
   if (!allow_loading_) {
     NOTREACHED();
     return;
   }
 
   if (premade_throttler_) {
-    premade_throttler_->RemoveObserver(this);
     premade_throttler_->SetHiddenForPlaceholder(false /* hidden */);
-
     ReplacePlugin(premade_throttler_->GetWebPlugin());
     premade_throttler_ = nullptr;
   } else {
     // TODO(mmenke):  In the case of prerendering, feed into
     //                ChromeContentRendererClient::CreatePlugin instead, to
     //                reduce the chance of future regressions.
     scoped_ptr<PluginInstanceThrottler> throttler;
 #if defined(ENABLE_PLUGINS)
     // If the plugin has already been marked essential in its placeholder form,
     // we shouldn't create a new throttler and start the process all over again.
-    if (!plugin_marked_essential_ && power_saver_enabled_)
+    if (power_saver_enabled_)
       throttler = PluginInstanceThrottler::Create();
 #endif
     WebPlugin* plugin = render_frame()->CreatePlugin(
         GetFrame(), plugin_info_, GetPluginParams(), throttler.Pass());
 
     ReplacePlugin(plugin);
   }
 }
 
 void LoadablePluginPlaceholder::LoadCallback() {
@@ skipping 11 matching lines @@
   HidePlugin();
 }
 
 void LoadablePluginPlaceholder::DidFinishLoadingCallback() {
   finished_loading_ = true;
   if (message_.length() > 0)
     UpdateMessage();
 
   // Wait for the placeholder to finish loading to hide the premade plugin.
   // This is necessary to prevent a flicker.
-  if (premade_throttler_ && !placeholder_was_replaced_)
+  if (premade_throttler_ && power_saver_enabled_)
     premade_throttler_->SetHiddenForPlaceholder(true /* hidden */);
 }
 
 void LoadablePluginPlaceholder::SetPluginInfo(
     const content::WebPluginInfo& plugin_info) {
   plugin_info_ = plugin_info;
 }
 
 const content::WebPluginInfo& LoadablePluginPlaceholder::GetPluginInfo() const {
   return plugin_info_;
 }
 
 void LoadablePluginPlaceholder::SetIdentifier(const std::string& identifier) {
   identifier_ = identifier;
 }
 
 bool LoadablePluginPlaceholder::LoadingBlocked() const {
   DCHECK(allow_loading_);
   return is_blocked_for_background_tab_ || is_blocked_for_power_saver_poster_ ||
          is_blocked_for_prerendering_;
 }
 
 }  // namespace plugins