Plugin Power Saver: Improve Poster behavior for essential plugins.

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 0db8e4144ce617af19dc943a8f2b9e1a24876785..81c84240333f6caa33d8f7ba8fd361a0df102b78 100644
--- a/chrome/renderer/chrome_content_renderer_client.cc
+++ b/chrome/renderer/chrome_content_renderer_client.cc
@@ -311,6 +311,58 @@ std::string GetPluginInstancePosterAttribute(
   }
   return std::string();
 }
+
+PowerSaverInfo GetPowerSaverInfo(

[groby-ooo-7-16, 2015/12/09 02:17:15]

Dare I suggest that this is now pretty unit-testable?

[tommycli, 2015/12/09 23:25:45]

Done. I moved this block of code to its own file.

To unit test it, the render_frame calls are the main obstacle. Since
render_frame is a little too big to easily mock.

We could make a PowerSaverInfo::GetInternal method that takes the render_frame
calls as parameters, and takes GetPeripheralContentStatus as a closure parameter
and instead test that. It's a bit complicated though. Thoughts?

+    RenderFrame* render_frame,
+    const WebPluginParams& params,
+    ChromeViewHostMsg_GetPluginInfo_Status plugin_info_status,

[groby-ooo-7-16, 2015/12/09 02:17:15]

I wonder if this should be instead "bool power_saver_setting_on", and you pass
in the result of 
    plugin_info_status ==
ChromeViewHostMsg_GetPluginInfo_Status::kPlayImportantContent

Reason being that at that point the function is pretty much Chrome-independent
and can move to content/

(Welcome to this week's yak shave :)

[tommycli, 2015/12/09 23:25:45]

Done. Though I didn't move this to content/, since all the callers are in
chrome/, and it would require making a new content/public interface.

+    const WebPluginInfo& plugin_info,
+    const GURL& document_url) {
+  bool is_flash = plugin_info.name == ASCIIToUTF16(content::kFlashPluginName);
+
+  base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
+  std::string override_for_testing = command_line->GetSwitchValueASCII(
+      switches::kOverridePluginPowerSaverForTesting);
+
+  // This feature has only been tested throughly with Flash thus far.

[groby-ooo-7-16, 2015/12/09 02:17:15]

thoroughly

[tommycli, 2015/12/09 23:25:45]

Done.

+  // It is also enabled for the Power Saver test plugin for browser tests.
+  bool can_throttle_plugin_type =
+      is_flash || override_for_testing == "ignore-list";
+
+  bool power_saver_setting_on =
+      plugin_info_status ==
+      ChromeViewHostMsg_GetPluginInfo_Status::kPlayImportantContent;
+
+  PowerSaverInfo power_saver_info;
+  power_saver_info.power_saver_enabled =
+      override_for_testing == "always" ||
+      (power_saver_setting_on && can_throttle_plugin_type);
+
+  if (power_saver_info.power_saver_enabled) {
+    // Even if we disable PPS in the next block because content is same-origin,
+    // it should still be eligible for background tab deferral if PPS is on.
+    power_saver_info.blocked_for_background_tab = render_frame->IsHidden();
+
+    auto status = render_frame->GetPeripheralContentStatus(
+        render_frame->GetWebFrame()->top()->securityOrigin(),

[groby-ooo-7-16, 2015/12/09 02:17:15]

Weird question: Is that function ever called with a different security origin?
If not, would it make sense for GetPeripheralContentStatus to just get the
securityOrigin locally, since it also lives on render_frame?

[tommycli, 2015/12/09 23:25:45]

I think ... we specifically want to get the top-level frame's securityOrigin,
rather than the current render_frame's securityOrigin.

+        url::Origin(params.url), gfx::Size());
+
+    if (status == RenderFrame::CONTENT_STATUS_ESSENTIAL_SAME_ORIGIN ||
+        status ==
+            RenderFrame::CONTENT_STATUS_ESSENTIAL_CROSS_ORIGIN_WHITELISTED) {
+      power_saver_info.power_saver_enabled = false;
+    } else {
+      power_saver_info.poster_attribute =
+          GetPluginInstancePosterAttribute(params);
+      power_saver_info.base_url = document_url;
+    }
+  }
+
+  if (is_flash)
+    TrackPosterParamPresence(params, power_saver_info.power_saver_enabled);
+
+  return power_saver_info;
+}
 #endif
 
 #if defined(ENABLE_EXTENSIONS)
@@ -724,14 +776,13 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
     }
 #endif
 
-    base::CommandLine* command_line = base::CommandLine::ForCurrentProcess();
-    auto create_blocked_plugin =
-        [&render_frame, &frame, &params, &info, &identifier, &group_name](
-            int template_id, const base::string16& message) {
-          return ChromePluginPlaceholder::CreateBlockedPlugin(
-              render_frame, frame, params, info, identifier, group_name,
-              template_id, message, PlaceholderPosterInfo());
-        };
+    auto create_blocked_plugin = [&render_frame, &frame, &params, &info,
+                                  &identifier, &group_name](
+        int template_id, const base::string16& message) {
+      return ChromePluginPlaceholder::CreateBlockedPlugin(
+          render_frame, frame, params, info, identifier, group_name,
+          template_id, message, PowerSaverInfo());
+    };
     switch (status) {
       case ChromeViewHostMsg_GetPluginInfo_Status::kNotFound: {
         NOTREACHED();
@@ -750,7 +801,8 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
           bool is_nacl_unrestricted = false;
           if (is_nacl_mime_type) {
             is_nacl_unrestricted =
-                command_line->HasSwitch(switches::kEnableNaCl);
+                base::CommandLine::ForCurrentProcess()->HasSwitch(
+                    switches::kEnableNaCl);
           } else if (is_pnacl_mime_type) {
             is_nacl_unrestricted = true;
           }
@@ -806,54 +858,24 @@ WebPlugin* ChromeContentRendererClient::CreatePlugin(
         bool is_prerendering =
             prerender::PrerenderHelper::IsPrerendering(render_frame);
 
-        bool is_flash = info.name == ASCIIToUTF16(content::kFlashPluginName);
-
-        std::string override_for_testing = command_line->GetSwitchValueASCII(
-            switches::kOverridePluginPowerSaverForTesting);
-
-        // This feature has only been tested throughly with Flash thus far.
-        // It is also enabled for the Power Saver test plugin for browser tests.
-        bool can_throttle_plugin_type =
-            is_flash || override_for_testing == "ignore-list";
-
-        bool power_saver_setting_on =
-            status ==
-            ChromeViewHostMsg_GetPluginInfo_Status::kPlayImportantContent;
-
-        bool power_saver_enabled =
-            override_for_testing == "always" ||
-            (power_saver_setting_on && can_throttle_plugin_type);
-        bool blocked_for_background_tab =
-            power_saver_enabled && render_frame->IsHidden();
-
-        PlaceholderPosterInfo poster_info;
-        if (power_saver_enabled) {
-          poster_info.poster_attribute =
-              GetPluginInstancePosterAttribute(params);
-          poster_info.base_url = frame->document().url();
-        }
-
-        if (is_flash)
-          TrackPosterParamPresence(params, power_saver_enabled);
-
-        if (blocked_for_background_tab || is_prerendering ||
-            !poster_info.poster_attribute.empty()) {
+        PowerSaverInfo power_saver_info = GetPowerSaverInfo(
+            render_frame, params, status, info, frame->document().url());
+        if (power_saver_info.blocked_for_background_tab || is_prerendering ||

[groby-ooo-7-16, 2015/12/09 02:17:15]

Can we combine the two power_saver_info members into
power_saver_info.should_block?

[tommycli, 2015/12/09 23:25:45]

Don't think so. We have to set a separate 'block' for prerendering on line 872
placeholder->set_blocked_for_prerendering(is_prerendering)

+            !power_saver_info.poster_attribute.empty()) {
           placeholder = ChromePluginPlaceholder::CreateBlockedPlugin(
               render_frame, frame, params, info, identifier, group_name,
-              poster_info.poster_attribute.empty() ? IDR_BLOCKED_PLUGIN_HTML
-                                                   : IDR_PLUGIN_POSTER_HTML,
+              power_saver_info.poster_attribute.empty()
+                  ? IDR_BLOCKED_PLUGIN_HTML
+                  : IDR_PLUGIN_POSTER_HTML,
               l10n_util::GetStringFUTF16(IDS_PLUGIN_BLOCKED, group_name),
-              poster_info);
-          placeholder->set_blocked_for_background_tab(
-              blocked_for_background_tab);
+              power_saver_info);
           placeholder->set_blocked_for_prerendering(is_prerendering);
-          placeholder->set_power_saver_enabled(power_saver_enabled);
           placeholder->AllowLoading();
           break;
         }
 
         scoped_ptr<content::PluginInstanceThrottler> throttler;
-        if (power_saver_enabled) {
+        if (power_saver_info.power_saver_enabled) {
           throttler = PluginInstanceThrottler::Create();
           // PluginPreroller manages its own lifetime.
           new PluginPreroller(