Plugin Power Saver: Make PepperPluginInstanceThrottler interface public.

chrome/renderer/chrome_content_renderer_client.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 "chrome/renderer/chrome_content_renderer_client.h"
 
 #include "base/command_line.h"
 #include "base/debug/crash_logging.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
@@ skipping 52 matching lines @@
 #include "components/dns_prefetch/renderer/prescient_networking_dispatcher.h"
 #include "components/dom_distiller/core/url_constants.h"
 #include "components/nacl/renderer/ppb_nacl_private_impl.h"
 #include "components/password_manager/content/renderer/credential_manager_client.h"
 #include "components/pdf/renderer/ppb_pdf_impl.h"
 #include "components/plugins/renderer/mobile_youtube_plugin.h"
 #include "components/signin/core/common/profile_management_switches.h"
 #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"
 #include "content/public/renderer/render_view_visitor.h"
 #include "extensions/common/constants.h"
 #include "ipc/ipc_sync_channel.h"
 #include "net/base/net_errors.h"
 #include "ppapi/c/private/ppb_nacl_private.h"
 #include "ppapi/c/private/ppb_pdf.h"
 #include "ppapi/shared_impl/ppapi_switches.h"
@@ skipping 62 matching lines @@
 
 #if defined(OS_WIN)
 #include "chrome_elf/blacklist/blacklist.h"
 #endif
 
 using autofill::AutofillAgent;
 using autofill::PasswordAutofillAgent;
 using autofill::PasswordGenerationAgent;
 using base::ASCIIToUTF16;
 using base::UserMetricsAction;
+using content::PluginInstanceThrottler;
 using content::RenderFrame;
 using content::RenderThread;
 using content::WebPluginInfo;
 using extensions::Extension;
 using blink::WebCache;
 using blink::WebConsoleMessage;
 using blink::WebDataSource;
 using blink::WebDocument;
 using blink::WebFrame;
 using blink::WebLocalFrame;
@@ skipping 622 matching lines @@
                 l10n_util::GetStringUTF16(IDS_NACL_PLUGIN_BLOCKED),
   #else
                 l10n_util::GetStringFUTF16(IDS_PLUGIN_BLOCKED, group_name),
   #endif
                 GURL());
             break;
           }
         }
 #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;
         bool blocked_for_background_tab =
             render_frame->IsHidden() &&
             status_value ==
                 ChromeViewHostMsg_GetPluginInfo_Status::kPlayImportantContent;
         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;
           }
         }
 
         // Delay loading plugins if prerendering.
         // TODO(mmenke):  In the case of prerendering, feed into
         //                ChromeContentRendererClient::CreatePlugin instead, to
         //                reduce the chance of future regressions.
         bool is_prerendering =
             prerender::PrerenderHelper::IsPrerendering(render_frame);
         if (blocked_for_background_tab || is_prerendering || show_poster) {
           placeholder = create_blocked_plugin(
               show_poster ? IDR_PLUGIN_POSTER_HTML : IDR_BLOCKED_PLUGIN_HTML,
               l10n_util::GetStringFUTF16(IDS_PLUGIN_BLOCKED, group_name),
               poster_url);
           placeholder->set_blocked_for_background_tab(
               blocked_for_background_tab);
           placeholder->set_blocked_for_prerendering(is_prerendering);
           placeholder->set_power_saver_mode(power_saver_mode);
           placeholder->set_allow_loading(true);
           break;
         } else if (cross_origin_main_content) {
           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(
             UserMetricsAction("Plugin_NPAPINotSupported"));
         placeholder = create_blocked_plugin(
             IDR_BLOCKED_PLUGIN_HTML,
             l10n_util::GetStringUTF16(IDS_PLUGIN_NOT_SUPPORTED_METRO), GURL());
         render_frame->Send(new ChromeViewHostMsg_NPAPINotSupported(
             render_frame->GetRoutingID(), identifier));
         break;
@@ skipping 725 matching lines @@
   if (mime_type == content::kBrowserPluginMimeType) {
     return new extensions::ExtensionsGuestViewContainer(render_frame);
   } else {
     return new extensions::MimeHandlerViewContainer(
         render_frame, mime_type, original_url);
   }
 #else
   return NULL;
 #endif
 }