Plugin Power Saver: Make PepperPluginInstanceThrottler interface public.

chrome/renderer/chrome_content_renderer_client.cc

Index: chrome/renderer/chrome_content_renderer_client.cc
diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc
index 356f7af0751fd310b6ae8ef434f386299cbe1b46..219df9881f262c7a9974b7b6ec69d85db5600341 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -70,6 +70,7 @@
 #include "components/visitedlink/renderer/visitedlink_slave.h"
 #include "components/web_cache/renderer/web_cache_render_process_observer.h"
 #include "content/public/common/content_constants.h"
+#include "content/public/renderer/plugin_instance_throttler.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_thread.h"
 #include "content/public/renderer/render_view.h"
@@ -152,6 +153,7 @@ using autofill::PasswordAutofillAgent;
 using autofill::PasswordGenerationAgent;
 using base::ASCIIToUTF16;
 using base::UserMetricsAction;
+using content::PluginInstanceThrottler;
 using content::RenderFrame;
 using content::RenderThread;
 using content::WebPluginInfo;
@@ -794,9 +796,9 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
         }
 #endif  // !defined(DISABLE_NACL) && defined(ENABLE_EXTENSIONS)
 
-        RenderFrame::PluginPowerSaverMode power_saver_mode =
-            RenderFrame::POWER_SAVER_MODE_ESSENTIAL;
 #if defined(ENABLE_PLUGINS)
+        PluginInstanceThrottler::PluginPowerSaverMode power_saver_mode =
+            PluginInstanceThrottler::POWER_SAVER_MODE_ESSENTIAL;
         bool show_poster = false;
         GURL poster_url;
         bool cross_origin_main_content = false;
@@ -807,14 +809,16 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
         if (render_frame->ShouldThrottleContent(params, frame->document().url(),
                                                 &poster_url,
                                                 &cross_origin_main_content)) {
-          if (status_value ==
-              ChromeViewHostMsg_GetPluginInfo_Status::kPlayImportantContent) {
+          // TODO(tommycli): Apply throttler behavior to all plugins.
+          if (status_value == ChromeViewHostMsg_GetPluginInfo_Status::

[Lei Zhang, 2015/01/14 23:49:33]

nit: can you pick one style of line breaks and stick with it? (vs line 847)

[tommycli, 2015/01/15 00:26:05]

I'm just doing 'git cl format'. Does it look better after reordering the
conditional?

[Lei Zhang, 2015/01/15 01:18:38]

No, but whatever.

[tommycli, 2015/01/15 23:19:32]

Done.

+                                  kPlayImportantContent &&
+              plugin.name == base::ASCIIToUTF16(content::kFlashPluginName)) {

[Lei Zhang, 2015/01/14 23:49:33]

Is this a behavior change that you didn't mention in the CL description?

[tommycli, 2015/01/15 00:26:05]

Previous the Flash-only conditional was in the Throttler. Now it's moved out
into here.

[Lei Zhang, 2015/01/15 01:18:38]

The comment in content/public/renderer/plugin_instance_throttler.h that mentions
Flash should be updated then?

[tommycli, 2015/01/15 23:19:32]

Done.

             power_saver_mode =
-                RenderFrame::POWER_SAVER_MODE_PERIPHERAL_THROTTLED;
+                PluginInstanceThrottler::POWER_SAVER_MODE_PERIPHERAL_THROTTLED;
             show_poster = poster_url.is_valid();
           } else {
-            power_saver_mode =
-                RenderFrame::POWER_SAVER_MODE_PERIPHERAL_UNTHROTTLED;
+            power_saver_mode = PluginInstanceThrottler::
+                POWER_SAVER_MODE_PERIPHERAL_UNTHROTTLED;
           }
         }
 
@@ -839,10 +843,18 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
           GURL content_origin = GURL(params.url).GetOrigin();
           render_frame->WhitelistContentOrigin(content_origin);
         }
-#endif  // defined(ENABLE_PLUGINS)
 
+        scoped_ptr<content::PluginInstanceThrottler> throttler;
+        if (power_saver_mode !=
+            content::PluginInstanceThrottler::POWER_SAVER_MODE_ESSENTIAL) {
+          throttler = content::PluginInstanceThrottler::Create(

[Lei Zhang, 2015/01/14 23:49:33]

You are never creating a throttler for POWER_SAVER_MODE_ESSENTIAL in all the
Create() calls, but plugin_instance_throttler_impl.cc has a "power_saver_mode !=
POWER_SAVER_MODE_ESSENTIAL" comparison that will be always true.

Previously, you always created a throttler.

[tommycli, 2015/01/15 00:26:05]

Nice catch! You are right. That was an oversight and very confusing.

Yes, the intent is now that throttlers are no longer created for ESSENTIAL
plugins.

The 'is_peripheral_content_' variable now has a very misleading name. I've
renamed it 'has_been_unthrottled_' and flipped all the conditionals.

[Lei Zhang, 2015/01/15 01:18:38]

It's still a bit unintuitive. We have:

ESSENTIAL: no throttler at all
UNTHROTTLED: throttler created, internally |power_saver_enabled_| is disabled...
why create the throttler in this case?
THROTTLED: throttler created, internally |power_saver_enabled_| is enabled

Also, you can still potentially create a throttler with ESSENTIAL as the power
mode, but it gets treated as UNTHROTTLED. PluginInstanceThrottler::Create()
should at least document this with a DCHECK in
Create/PluginInstanceThrottler{Impl}, or the enum should be a bool, or don't
create it for the unthrottled case and drop the parameter.

[tommycli, 2015/01/15 23:19:32]

Done.

Okay this comment helped clarify my thinking.

The throttler previously had 4 bools. But there's only 4 accessible states,
which I've now explicitly enumerated.

And yes, since the throttler is now no longer needed for the ESSENTIAL case,
I've replaced the argument with a bool.

I still need to pass a tri-state 'mode' to the placeholder though, so I've
preserved that enum. Check out the inter-patchset deltas.

+              render_frame, url, power_saver_mode);
+        }
         return render_frame->CreatePlugin(frame, plugin, params,
-                                          power_saver_mode);
+                                          throttler.Pass());
+#else  // !defined(ENABLE_PLUGINS)
+        return render_frame->CreatePlugin(frame, plugin, params, nullptr);
+#endif
       }
       case ChromeViewHostMsg_GetPluginInfo_Status::kNPAPINotSupported: {
         RenderThread::Get()->RecordAction(