Plugin Power Saver: Improve Poster behavior for essential plugins.

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 293 matching lines @@
   DCHECK_EQ(params.attributeNames.size(), params.attributeValues.size());
 
   for (size_t i = 0; i < params.attributeNames.size(); ++i) {
     if (params.attributeNames[i].utf8() == "poster" &&
         !params.attributeValues[i].isEmpty()) {
       return params.attributeValues[i].utf8();
     }
   }
   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)
 bool IsStandaloneExtensionProcess() {
   return base::CommandLine::ForCurrentProcess()->HasSwitch(
       extensions::switches::kExtensionProcess);
 }
 #endif
 
 // Defers media player loading in background pages until they're visible.
@@ skipping 393 matching lines @@
     // If user is on ALLOW or DETECT setting, loading needs to be blocked here.
     if ((status == ChromeViewHostMsg_GetPluginInfo_Status::kAllowed ||
          status ==
              ChromeViewHostMsg_GetPluginInfo_Status::kPlayImportantContent) &&
         info.type == content::WebPluginInfo::PLUGIN_TYPE_NPAPI) {
         if (observer->AreNPAPIPluginsBlocked())
           status = ChromeViewHostMsg_GetPluginInfo_Status::kNPAPINotSupported;
     }
 #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();
         break;
       }
       case ChromeViewHostMsg_GetPluginInfo_Status::kAllowed:
       case ChromeViewHostMsg_GetPluginInfo_Status::kPlayImportantContent: {
 #if !defined(DISABLE_NACL) && defined(ENABLE_EXTENSIONS)
         const bool is_nacl_plugin =
             info.name == ASCIIToUTF16(nacl::kNaClPluginName);
         const bool is_nacl_mime_type =
             actual_mime_type == nacl::kNaClPluginMimeType;
         const bool is_pnacl_mime_type =
             actual_mime_type == nacl::kPnaclPluginMimeType;
         if (is_nacl_plugin || is_nacl_mime_type || is_pnacl_mime_type) {
           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;
           }
           GURL manifest_url;
           GURL app_url;
           if (is_nacl_mime_type || is_pnacl_mime_type) {
             // Normal NaCl/PNaCl embed. The app URL is the page URL.
             manifest_url = url;
             app_url = frame->top()->document().url();
           } else {
@@ skipping 35 matching lines @@
         }
 #endif  // !defined(DISABLE_NACL) && defined(ENABLE_EXTENSIONS)
 
         // 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);
 
-        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(
               render_frame, frame, params, info, identifier, group_name,
               l10n_util::GetStringFUTF16(IDS_PLUGIN_BLOCKED, group_name),
               throttler.get());
         }
 
         return render_frame->CreatePlugin(frame, info, params,
                                           throttler.Pass());
@@ skipping 614 matching lines @@
 // chrome.system.network.getNetworkInterfaces provides the same
 // information. Also, the enforcement of sending and binding UDP is already done
 // by chrome extension permission model.
 bool ChromeContentRendererClient::ShouldEnforceWebRTCRoutingPreferences() {
 #if defined(ENABLE_EXTENSIONS)
   return !IsStandaloneExtensionProcess();
 #else
   return true;
 #endif
 }