Plugin Power Saver Tiny: Fix Plugin.PowerSaver.PeripheralHeuristic UMA

Previously, it counted every status query. As we implemented PPS tiny,
the number of queries for tiny plugins increased, but we didn't
enforce that it was only recorded once.

This CL enforces that only the initial decision for a plugin is
recorded. It also migrates the UMA to a
Plugin.PowerSaver.PeripheralHeuristicInitialDecision name to prevent
further confusion.

This is complicated because there are three paths a plugin may follow:

1. Essential origin case. No placeholder, no throttler - Record
   immediately, in PowerSaverInfo.

2. Placeholder spawned first (because PPS tiny or background tab or
   prerendering) - Don't record in PowerSaverInfo. Record in
   LoadablePluginPlaceholder after size known. If a Throttler is
   spawned from the placeholder, tell it to not record.

3. Throttler spawned first - Record in the throttler.

Once PPS Tiny is launched 100%, case chromium#3 can be eliminated.

BUG=634130

Review-Url: https://codereview.chromium.org/2211753002
Cr-Commit-Position: refs/heads/master@{#411174}

chrome/renderer/chrome_content_renderer_client.cc

@@ -808,7 +808,8 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
 
         std::unique_ptr<content::PluginInstanceThrottler> throttler;
         if (power_saver_info.power_saver_enabled) {
-          throttler = PluginInstanceThrottler::Create();
+          throttler =
+              PluginInstanceThrottler::Create(RenderFrame::RECORD_DECISION);
           // PluginPreroller manages its own lifetime.
           new PluginPreroller(
               render_frame, frame, params, info, identifier, group_name,

chrome/renderer/plugins/chrome_plugin_placeholder.cc

@@ -383,7 +383,9 @@ blink::WebPlugin* ChromePluginPlaceholder::CreatePlugin() {
   // 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 (power_saver_enabled()) {
-    throttler = content::PluginInstanceThrottler::Create();
+    throttler = content::PluginInstanceThrottler::Create(
+        heuristic_run_before_ ? content::RenderFrame::DONT_RECORD_DECISION
+                              : content::RenderFrame::RECORD_DECISION);
     // PluginPreroller manages its own lifetime.
     new PluginPreroller(render_frame(), GetFrame(), GetPluginParams(),
                         GetPluginInfo(), GetIdentifier(), title_,

chrome/renderer/plugins/power_saver_info.cc

@@ -102,7 +102,8 @@ PowerSaverInfo PowerSaverInfo::Get(content::RenderFrame* render_frame,
 
     auto status = render_frame->GetPeripheralContentStatus(
         render_frame->GetWebFrame()->top()->getSecurityOrigin(),
-        url::Origin(params.url), gfx::Size());
+        url::Origin(params.url), gfx::Size(),
+        content::RenderFrame::RECORD_DECISION);
 
     // Early-exit from the whole Power Saver system if the content is
     // same-origin or whitelisted-origin. We ignore the other possibilities,

components/plugins/renderer/loadable_plugin_placeholder.cc

@@ -32,6 +32,7 @@
 
 using base::UserMetricsAction;
 using content::PluginInstanceThrottler;
+using content::RenderFrame;
 using content::RenderThread;
 
 namespace plugins {
@@ -51,15 +52,17 @@ void LoadablePluginPlaceholder::SetPremadePlugin(
     content::PluginInstanceThrottler* throttler) {
   DCHECK(throttler);
   DCHECK(!premade_throttler_);
+  heuristic_run_before_ = true;
   premade_throttler_ = throttler;
 }
 
 LoadablePluginPlaceholder::LoadablePluginPlaceholder(
-    content::RenderFrame* render_frame,
+    RenderFrame* render_frame,
     blink::WebLocalFrame* frame,
     const blink::WebPluginParams& params,
     const std::string& html_data)
     : PluginPlaceholderBase(render_frame, frame, params, html_data),
+      heuristic_run_before_(false),
       is_blocked_for_tinyness_(false),
       is_blocked_for_background_tab_(false),
       is_blocked_for_prerendering_(false),
@@ -68,7 +71,6 @@ LoadablePluginPlaceholder::LoadablePluginPlaceholder(
       premade_throttler_(nullptr),
       allow_loading_(false),
       finished_loading_(false),
-      heuristic_run_before_(premade_throttler_ != nullptr),
       weak_factory_(this) {}
 
 LoadablePluginPlaceholder::~LoadablePluginPlaceholder() {
@@ -199,27 +201,27 @@ void LoadablePluginPlaceholder::OnUnobscuredRectUpdate(
 
   // On a size update check if we now qualify as a essential plugin.
   url::Origin content_origin = url::Origin(GetPluginParams().url);
-  content::RenderFrame::PeripheralContentStatus status =
+  RenderFrame::PeripheralContentStatus status =
       render_frame()->GetPeripheralContentStatus(
           render_frame()->GetWebFrame()->top()->getSecurityOrigin(),
-          content_origin, gfx::Size(width, height));
+          content_origin, gfx::Size(width, height),
+          heuristic_run_before_ ? RenderFrame::DONT_RECORD_DECISION
+                                : RenderFrame::RECORD_DECISION);
 
   bool plugin_is_tiny_and_blocked =
       is_blocked_for_tinyness_ &&
-      status ==
-          content::RenderFrame::CONTENT_STATUS_ESSENTIAL_CROSS_ORIGIN_TINY;
+      status == RenderFrame::CONTENT_STATUS_ESSENTIAL_CROSS_ORIGIN_TINY;
 
   // Early exit for plugins that we've discovered to be essential.
   if (!plugin_is_tiny_and_blocked &&
-      status != content::RenderFrame::CONTENT_STATUS_PERIPHERAL) {
+      status != 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) {
+        status == RenderFrame::CONTENT_STATUS_ESSENTIAL_CROSS_ORIGIN_BIG) {
       render_frame()->WhitelistContentOrigin(content_origin);
     }
 

components/plugins/renderer/loadable_plugin_placeholder.h

@@ -82,6 +82,9 @@ class LoadablePluginPlaceholder : public PluginPlaceholderBase {
   void DidFinishLoadingCallback();
   void DidFinishIconRepositionForTestingCallback();
 
+  // True if the power saver heuristic has already been run on this content.
+  bool heuristic_run_before_;
+
  private:
   // WebViewPlugin::Delegate methods:
   void PluginDestroyed() override;
@@ -135,9 +138,6 @@ class LoadablePluginPlaceholder : public PluginPlaceholderBase {
 
   gfx::Rect unobscured_rect_;
 
-  // True if the power saver heuristic has already been run on this content.
-  bool heuristic_run_before_;
-
   base::WeakPtrFactory<LoadablePluginPlaceholder> weak_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(LoadablePluginPlaceholder);

content/public/renderer/plugin_instance_throttler.h

@@ -9,6 +9,7 @@
 
 #include "base/macros.h"
 #include "content/common/content_export.h"
+#include "content/public/renderer/render_frame.h"
 
 namespace blink {
 class WebPlugin;
@@ -73,7 +74,8 @@ class CONTENT_EXPORT PluginInstanceThrottler {
     virtual void OnThrottlerDestroyed() {}
   };
 
-  static std::unique_ptr<PluginInstanceThrottler> Create();
+  static std::unique_ptr<PluginInstanceThrottler> Create(
+      RenderFrame::RecordPeripheralDecision record_decision);
 
   static void RecordUnthrottleMethodMetric(PowerSaverUnthrottleMethod method);
 

content/public/renderer/render_frame.h

@@ -82,6 +82,11 @@ class CONTENT_EXPORT RenderFrame : public IPC::Listener,
     CONTENT_STATUS_NUM_ITEMS
   };
 
+  enum RecordPeripheralDecision {
+    DONT_RECORD_DECISION = 0,
+    RECORD_DECISION = 1
+  };
+
   // Returns the RenderFrame given a WebFrame.
   static RenderFrame* FromWebFrame(blink::WebFrame* web_frame);
 
@@ -177,7 +182,8 @@ class CONTENT_EXPORT RenderFrame : public IPC::Listener,
   virtual PeripheralContentStatus GetPeripheralContentStatus(
       const url::Origin& main_frame_origin,
       const url::Origin& content_origin,
-      const gfx::Size& unobscured_size) const = 0;
+      const gfx::Size& unobscured_size,
+      RecordPeripheralDecision record_decision) const = 0;
 
   // Whitelists a |content_origin| so its content will never be throttled in
   // this RenderFrame. Whitelist is cleared by top level navigation.

content/renderer/pepper/pepper_webplugin_impl_browsertest.cc

@@ -169,7 +169,8 @@ class PepperWebPluginImplBrowserTest
                               blink::WebLocalFrame* frame,
                               const blink::WebPluginParams& params,
                               blink::WebPlugin** plugin) override {
-      current_test_->throttler_ = new PluginInstanceThrottlerImpl;
+      current_test_->throttler_ =
+          new PluginInstanceThrottlerImpl(RenderFrame::DONT_RECORD_DECISION);
       current_test_->throttler_->AddObserver(current_test_);
       *plugin = render_frame->CreatePlugin(
           frame, GetPluginInfo().ToWebPluginInfo(), params,

content/renderer/pepper/plugin_instance_throttler_impl.cc

@@ -41,8 +41,9 @@ const int kAudioThrottledFrameTimeoutMilliseconds = 500;
 const int PluginInstanceThrottlerImpl::kMaximumFramesToExamine = 150;
 
 // static
-std::unique_ptr<PluginInstanceThrottler> PluginInstanceThrottler::Create() {
-  return base::WrapUnique(new PluginInstanceThrottlerImpl);
+std::unique_ptr<PluginInstanceThrottler> PluginInstanceThrottler::Create(
+    RenderFrame::RecordPeripheralDecision record_decision) {
+  return base::WrapUnique(new PluginInstanceThrottlerImpl(record_decision));
 }
 
 // static
@@ -53,8 +54,10 @@ void PluginInstanceThrottler::RecordUnthrottleMethodMetric(
       PluginInstanceThrottler::UNTHROTTLE_METHOD_NUM_ITEMS);
 }
 
-PluginInstanceThrottlerImpl::PluginInstanceThrottlerImpl()
-    : state_(THROTTLER_STATE_AWAITING_KEYFRAME),
+PluginInstanceThrottlerImpl::PluginInstanceThrottlerImpl(
+    content::RenderFrame::RecordPeripheralDecision record_decision)
+    : record_decision_(record_decision),
+      state_(THROTTLER_STATE_AWAITING_KEYFRAME),
       is_hidden_for_placeholder_(false),
       web_plugin_(nullptr),
       frames_examined_(0),
@@ -65,8 +68,7 @@ PluginInstanceThrottlerImpl::PluginInstanceThrottlerImpl()
               kAudioThrottledFrameTimeoutMilliseconds),
           this,
           &PluginInstanceThrottlerImpl::EngageThrottle),
-      weak_factory_(this) {
-}
+      weak_factory_(this) {}
 
 PluginInstanceThrottlerImpl::~PluginInstanceThrottlerImpl() {
   FOR_EACH_OBSERVER(Observer, observer_list_, OnThrottlerDestroyed());
@@ -144,7 +146,8 @@ void PluginInstanceThrottlerImpl::Initialize(
     auto status = frame->GetPeripheralContentStatus(
         frame->GetWebFrame()->top()->getSecurityOrigin(), content_origin,
         gfx::Size(roundf(unobscured_size.width() / zoom_factor),
-                  roundf(unobscured_size.height() / zoom_factor)));
+                  roundf(unobscured_size.height() / zoom_factor)),
+        record_decision_);
     if (status != RenderFrame::CONTENT_STATUS_PERIPHERAL) {
       DCHECK_NE(THROTTLER_STATE_MARKED_ESSENTIAL, state_);
       state_ = THROTTLER_STATE_MARKED_ESSENTIAL;

content/renderer/pepper/plugin_instance_throttler_impl.h

@@ -33,7 +33,8 @@ class RenderFrameImpl;
 class CONTENT_EXPORT PluginInstanceThrottlerImpl
     : public PluginInstanceThrottler {
  public:
-  PluginInstanceThrottlerImpl();
+  explicit PluginInstanceThrottlerImpl(
+      RenderFrame::RecordPeripheralDecision record_decision);
 
   ~PluginInstanceThrottlerImpl() override;
 
@@ -90,6 +91,8 @@ class CONTENT_EXPORT PluginInstanceThrottlerImpl
   void AudioThrottledFrameTimeout();
   void EngageThrottle();
 
+  RenderFrame::RecordPeripheralDecision record_decision_;
+
   ThrottlerState state_;
 
   bool is_hidden_for_placeholder_;

content/renderer/pepper/plugin_instance_throttler_impl_unittest.cc

@@ -38,7 +38,8 @@ class PluginInstanceThrottlerImplTest
   }
 
   void SetUp() override {
-    throttler_.reset(new PluginInstanceThrottlerImpl);
+    throttler_.reset(
+        new PluginInstanceThrottlerImpl(RenderFrame::DONT_RECORD_DECISION));
     throttler_->Initialize(nullptr, url::Origin(GURL("http://example.com")),
                            "Shockwave Flash", gfx::Size(100, 100));
     throttler_->AddObserver(this);

content/renderer/pepper/plugin_power_saver_helper.cc

@@ -25,7 +25,7 @@ namespace content {
 namespace {
 
 const char kPeripheralHeuristicHistogram[] =
-    "Plugin.PowerSaver.PeripheralHeuristic";
+    "Plugin.PowerSaver.PeripheralHeuristicInitialDecision";
 
 }  // namespace
 
@@ -102,23 +102,23 @@ RenderFrame::PeripheralContentStatus
 PluginPowerSaverHelper::GetPeripheralContentStatus(
     const url::Origin& main_frame_origin,
     const url::Origin& content_origin,
-    const gfx::Size& unobscured_size) const {
+    const gfx::Size& unobscured_size,
+    RenderFrame::RecordPeripheralDecision record_decision) const {
   if (base::CommandLine::ForCurrentProcess()->GetSwitchValueASCII(
           switches::kOverridePluginPowerSaverForTesting) == "always") {
     return RenderFrame::CONTENT_STATUS_PERIPHERAL;
   }
 
   auto status = PeripheralContentHeuristic::GetPeripheralStatus(
       origin_whitelist_, main_frame_origin, content_origin, unobscured_size);
-  if (status == RenderFrame::CONTENT_STATUS_ESSENTIAL_UNKNOWN_SIZE) {
-    // Early exit here to avoid recording a UMA. Every plugin will call this
-    // method once before the size is known (to faciliate early-exit for
-    // same-origin and whitelisted-origin content).
-    return status;
+
+  // Never record ESSENTIAL_UNKNOWN_SIZE. Wait for retest after size is known.
+  if (record_decision == RenderFrame::RECORD_DECISION &&
+      status != RenderFrame::CONTENT_STATUS_ESSENTIAL_UNKNOWN_SIZE) {
+    UMA_HISTOGRAM_ENUMERATION(kPeripheralHeuristicHistogram, status,
+                              RenderFrame::CONTENT_STATUS_NUM_ITEMS);
   }
 
-  UMA_HISTOGRAM_ENUMERATION(kPeripheralHeuristicHistogram, status,
-                            RenderFrame::CONTENT_STATUS_NUM_ITEMS);
   return status;
 }
 

content/renderer/pepper/plugin_power_saver_helper.h

@@ -50,7 +50,8 @@ class CONTENT_EXPORT PluginPowerSaverHelper : public RenderFrameObserver {
   RenderFrame::PeripheralContentStatus GetPeripheralContentStatus(
       const url::Origin& main_frame_origin,
       const url::Origin& content_origin,
-      const gfx::Size& unobscured_size) const;
+      const gfx::Size& unobscured_size,
+      RenderFrame::RecordPeripheralDecision record_decision) const;
   void WhitelistContentOrigin(const url::Origin& content_origin);
 
   // RenderFrameObserver implementation.

content/renderer/pepper/plugin_power_saver_helper_browsertest.cc

@@ -47,7 +47,8 @@ TEST_F(PluginPowerSaverHelperTest, TemporaryOriginWhitelist) {
   EXPECT_EQ(RenderFrame::CONTENT_STATUS_PERIPHERAL,
             frame()->GetPeripheralContentStatus(
                 url::Origin(GURL("http://same.com")),
-                url::Origin(GURL("http://other.com")), gfx::Size(100, 100)));
+                url::Origin(GURL("http://other.com")), gfx::Size(100, 100),
+                RenderFrame::DONT_RECORD_DECISION));
 
   // Clear out other messages so we find just the plugin power saver IPCs.
   sink_->ClearMessages();
@@ -57,7 +58,8 @@ TEST_F(PluginPowerSaverHelperTest, TemporaryOriginWhitelist) {
   EXPECT_EQ(RenderFrame::CONTENT_STATUS_ESSENTIAL_CROSS_ORIGIN_WHITELISTED,
             frame()->GetPeripheralContentStatus(
                 url::Origin(GURL("http://same.com")),
-                url::Origin(GURL("http://other.com")), gfx::Size(100, 100)));
+                url::Origin(GURL("http://other.com")), gfx::Size(100, 100),
+                RenderFrame::DONT_RECORD_DECISION));
 
   // Test that we've sent an IPC to the browser.
   ASSERT_EQ(1u, sink_->message_count());
@@ -89,14 +91,16 @@ TEST_F(PluginPowerSaverHelperTest, ClearWhitelistOnNavigate) {
   EXPECT_EQ(RenderFrame::CONTENT_STATUS_ESSENTIAL_CROSS_ORIGIN_WHITELISTED,
             frame()->GetPeripheralContentStatus(
                 url::Origin(GURL("http://same.com")),
-                url::Origin(GURL("http://other.com")), gfx::Size(100, 100)));
+                url::Origin(GURL("http://other.com")), gfx::Size(100, 100),
+                RenderFrame::DONT_RECORD_DECISION));
 
   LoadHTML("<html></html>");
 
   EXPECT_EQ(RenderFrame::CONTENT_STATUS_PERIPHERAL,
             frame()->GetPeripheralContentStatus(
                 url::Origin(GURL("http://same.com")),
-                url::Origin(GURL("http://other.com")), gfx::Size(100, 100)));
+                url::Origin(GURL("http://other.com")), gfx::Size(100, 100),
+                RenderFrame::DONT_RECORD_DECISION));
 }
 
 }  // namespace content

content/renderer/render_frame_impl.cc

@@ -2415,9 +2415,10 @@ RenderFrame::PeripheralContentStatus
 RenderFrameImpl::GetPeripheralContentStatus(
     const url::Origin& main_frame_origin,
     const url::Origin& content_origin,
-    const gfx::Size& unobscured_size) const {
+    const gfx::Size& unobscured_size,
+    RecordPeripheralDecision record_decision) const {
   return plugin_power_saver_helper_->GetPeripheralContentStatus(
-      main_frame_origin, content_origin, unobscured_size);
+      main_frame_origin, content_origin, unobscured_size, record_decision);
 }
 
 void RenderFrameImpl::WhitelistContentOrigin(

content/renderer/render_frame_impl.h

@@ -414,7 +414,8 @@ class CONTENT_EXPORT RenderFrameImpl
   RenderFrame::PeripheralContentStatus GetPeripheralContentStatus(
       const url::Origin& main_frame_origin,
       const url::Origin& content_origin,
-      const gfx::Size& unobscured_size) const override;
+      const gfx::Size& unobscured_size,
+      RecordPeripheralDecision record_decision) const override;
   void WhitelistContentOrigin(const url::Origin& content_origin) override;
 #endif
   bool IsFTPDirectoryListing() override;

tools/metrics/histograms/histograms.xml

@@ -42852,13 +42852,24 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries.
 
 <histogram name="Plugin.PowerSaver.PeripheralHeuristic"
     enum="PluginPowerSaverPeripheralHeuristicDecision">
+  <obsolete>
+    Deprecated in favor of Plugin.PowerSaver.PeripheralHeuristicFinalDecision.
+  </obsolete>
+  <owner>tommycli@chromium.org</owner>
+  <summary>
+    Records each decision of the Plugin Power Saver peripheral content
+    heuristic. This UMA is counted once per peripheral query, and may count a
+    single plugin instance multiple times as it is resized.
+  </summary>
+</histogram>
+
+<histogram name="Plugin.PowerSaver.PeripheralHeuristicInitialDecision"
+    enum="PluginPowerSaverPeripheralHeuristicDecision">
   <owner>tommycli@chromium.org</owner>
   <summary>
-    Record the initial decision of the Plugin Power Saver peripheral content
-    heuristic. For each plugin instance, this heuristic decides whether the
-    plugin instance is essential content or peripheral. All same-origin content
-    is essential. Cross-origin content is peripheral if it is small and not on
-    the origin whitelist.
+    Records the initial decision of the Plugin Power Saver peripheral content
+    heuristic for each plugin instance. This is recorded once per plugin
+    instance.
   </summary>
 </histogram>