Plugin Power Saver: Enable JS access to throttled plugins.

chrome/browser/plugins/plugin_power_saver_browsertest.cc

Index: chrome/browser/plugins/plugin_power_saver_browsertest.cc
diff --git a/chrome/browser/plugins/plugin_power_saver_browsertest.cc b/chrome/browser/plugins/plugin_power_saver_browsertest.cc
index e31ad92478f74285c96c2a9e76b67b2e25f3cd34..2981c534bbda4bb50d97b0a1360e1f900af82f81 100644
--- a/chrome/browser/plugins/plugin_power_saver_browsertest.cc
+++ b/chrome/browser/plugins/plugin_power_saver_browsertest.cc
@@ -37,37 +37,38 @@ std::string RunTestScript(base::StringPiece test_script,
   return result;
 }
 
-void VerifyPluginIsPlaceholderOnly(content::WebContents* contents,
-                                   const char* element_id) {
+// This also tests that we have JavaScript access to the underlying plugin.
+bool PluginLoaded(content::WebContents* contents, const char* element_id) {
   std::string result = RunTestScript(
       "if (plugin.postMessage === undefined) {"
-      "  window.domAutomationController.send('placeholder');"
+      "  window.domAutomationController.send('poster_only');"
       "} else {"
       "  window.domAutomationController.send('plugin_loaded');"
       "}",
       contents, element_id);
-  EXPECT_EQ("placeholder", result);
+  EXPECT_NE("error", result);
+  return result == "plugin_loaded";
 }
 
-void VerifyPluginIsPeripheral(content::WebContents* contents,
-                              const char* element_id) {
+void VerifyPluginIsThrottled(content::WebContents* contents,
+                             const char* element_id) {
   std::string result = RunTestScript(
       "function handleEvent(event) {"
-      "  if (event.data.isPeripheral !== undefined && "
-      "      event.data.source === 'getPowerSaverStatusResponse') {"

[Lei Zhang, 2015/05/07 23:15:59]

Do you still care about the event's data source?

[tommycli, 2015/05/08 00:49:00]

We don't care about the source anymore. We just care that all the three
conditions are fulfilled.

-      "    window.domAutomationController.send("
-      "        event.data.isPeripheral ? 'peripheral' : 'essential');"
+      "  if (event.data.isPeripheral && event.data.isThrottled && "
+      "      event.data.isHiddenForPlaceholder) {"
+      "    window.domAutomationController.send('throttled');"
       "    plugin.removeEventListener('message', handleEvent);"
       "  }"
       "}"
-      "if (plugin.postMessage === undefined) {"
-      "  window.domAutomationController.send('peripheral');"
-      "} else {"
-      "  plugin.addEventListener('message', handleEvent);"
+      "plugin.addEventListener('message', handleEvent);"
+      "if (plugin.postMessage !== undefined) {"

[Lei Zhang, 2015/05/07 23:15:59]

So previously, if plugin.postMessage is undefined, it would send() and
RunTestScript() would return. Now, the code does nothing. Is it relying on
handleEvent() to eventually call send(), or will this end up hanging and timing
out?

[tommycli, 2015/05/08 00:49:00]

Exactly. Now it expects that if there is no postMessage yet, the plugin will
send the "initial" status messages and we will catch those.

       "  plugin.postMessage('getPowerSaverStatus');"
       "}",
       contents, element_id);
-  EXPECT_EQ("peripheral", result);
+  EXPECT_EQ("throttled", result);
+
+  // Page should continue to have JavaScript access to all throttled plugins.
+  EXPECT_TRUE(PluginLoaded(contents, element_id));
 }
 
 void VerifyPluginMarkedEssential(content::WebContents* contents,
@@ -85,6 +86,7 @@ void VerifyPluginMarkedEssential(content::WebContents* contents,
       "}",
       contents, element_id);
   EXPECT_EQ("essential", result);
+  EXPECT_TRUE(PluginLoaded(contents, element_id));
 }
 
 }  // namespace
@@ -135,13 +137,12 @@ IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, SmallSameOrigin) {
   VerifyPluginMarkedEssential(GetActiveWebContents(), "plugin");
 }
 
-// Flaky: https://crbug.com/485160
-IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, DISABLED_SmallCrossOrigin) {
+IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, SmallCrossOrigin) {

[tommycli, 2015/05/07 17:23:00]

I believe this patch incidentally fixes this flaky test (but I am not sure).

[Bernhard Bauer, 2015/05/07 17:46:28]

Does it? I thought there was the race condition where we dispatch the click
before the inner HTML has loaded?

[tommycli, 2015/05/07 17:55:31]

Since we changed it to VerifyPluginIsThrottled, it should wait until
event.data.isHiddenForPlaceholder is true also. (See line 58)

Since throttler->SetHiddenForPlaceholder is only called in
DidFinishLoadingCallback, my theory is that in this particular case, the race
condition is resolved.

The remaining issue is for when there is no throttled plugin and we have a
poster image only. Then we have no notification for DidFinishLoadingCallback.

[Bernhard Bauer, 2015/05/07 18:23:26]

Acknowledged.

[Lei Zhang, 2015/05/07 23:15:59]

Can you add a comment to the top of VerifyPluginIsThrottled() to mention it
waits for the plugin to finish loading?

[tommycli, 2015/05/08 00:49:00]

Done.

   LoadHTML(
       "<object id='plugin' data='http://otherorigin.com/fake.swf' "
       "    type='application/x-ppapi-tests' width='400' height='100'>"
       "</object>");
-  VerifyPluginIsPeripheral(GetActiveWebContents(), "plugin");
+  VerifyPluginIsThrottled(GetActiveWebContents(), "plugin");
 
   SimulateClickAndAwaitMarkedEssential("plugin", gfx::Point(50, 50));
 }
@@ -172,11 +173,11 @@ IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest,
       "    width='400' height='500'"
       "    poster='snapshot1x.png 1x, snapshot2x.png 2x'></embed>");
 
-  VerifyPluginIsPlaceholderOnly(GetActiveWebContents(), "plugin_src");
-  VerifyPluginIsPlaceholderOnly(GetActiveWebContents(), "plugin_srcset");
-  VerifyPluginIsPlaceholderOnly(GetActiveWebContents(), "plugin_legacy_syntax");
-  VerifyPluginIsPlaceholderOnly(GetActiveWebContents(), "plugin_embed_src");
-  VerifyPluginIsPlaceholderOnly(GetActiveWebContents(), "plugin_embed_srcset");
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_src"));
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_srcset"));
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_legacy_syntax"));
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_embed_src"));
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_embed_srcset"));
 }
 
 // flaky: crbug.com/481687
@@ -185,7 +186,7 @@ IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest,
   LoadHTML(
       "<object id='plugin' type='application/x-ppapi-tests' "
       "    width='400' height='100' poster='snapshot1x.png'></object>");
-  VerifyPluginIsPlaceholderOnly(GetActiveWebContents(), "plugin");
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin"));
 
   SimulateClickAndAwaitMarkedEssential("plugin", gfx::Point(50, 50));
 }
@@ -208,7 +209,7 @@ IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, LargeCrossOriginObscured) {
       "      type='application/x-ppapi-tests' width='400' height='500'>"
       "  </object>"
       "</div>");
-  VerifyPluginIsPeripheral(GetActiveWebContents(), "plugin");
+  VerifyPluginIsThrottled(GetActiveWebContents(), "plugin");
 }
 
 IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, BackgroundTabPlugins) {
@@ -227,12 +228,12 @@ IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, BackgroundTabPlugins) {
   EXPECT_TRUE(
       content::WaitForRenderFrameReady(background_contents->GetMainFrame()));
 
-  VerifyPluginIsPlaceholderOnly(background_contents, "same_origin");
-  VerifyPluginIsPlaceholderOnly(background_contents, "small_cross_origin");
+  EXPECT_FALSE(PluginLoaded(background_contents, "same_origin"));
+  EXPECT_FALSE(PluginLoaded(background_contents, "small_cross_origin"));
 
   browser()->tab_strip_model()->SelectNextTab();
   EXPECT_EQ(background_contents, GetActiveWebContents());
 
   VerifyPluginMarkedEssential(background_contents, "same_origin");
-  VerifyPluginIsPeripheral(background_contents, "small_cross_origin");
+  VerifyPluginIsThrottled(background_contents, "small_cross_origin");
 }