Plugin Power Saver: Implement pixel tests for plugin placeholders.

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 333720d38cf3728abeb7a5bffab2b9efd6ca013e..8b9ac858a67b017cd61e4fb90edfde667bc7f094 100644
--- a/chrome/browser/plugins/plugin_power_saver_browsertest.cc
+++ b/chrome/browser/plugins/plugin_power_saver_browsertest.cc
@@ -6,11 +6,15 @@
 #include "base/strings/string_piece.h"
 #include "base/strings/stringprintf.h"
 #include "chrome/browser/ui/browser.h"
+#include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/test/base/in_process_browser_test.h"
 #include "chrome/test/base/ui_test_utils.h"
 #include "components/ui/zoom/zoom_controller.h"
+#include "content/public/browser/readback_types.h"
+#include "content/public/browser/render_view_host.h"
+#include "content/public/browser/render_widget_host.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/ppapi_test_utils.h"
@@ -19,11 +23,28 @@
 #include "net/test/embedded_test_server/http_response.h"
 #include "ppapi/shared_impl/ppapi_switches.h"
 #include "third_party/WebKit/public/web/WebInputEvent.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/window_open_disposition.h"
+#include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/geometry/point.h"
+#include "ui/gfx/screen.h"
 
 namespace {
 
+// Use fixed browser dimensions for pixel tests.
+const int kBrowserWidth = 600;
+const int kBrowserHeight = 700;
+
+// Compare only a portion of the snapshots, as different platforms will
+// capture different sized snapshots (due to differences in browser chrome).
+const int kComparisonWidth = 500;
+const int kComparisonHeight = 600;
+
+// Different platforms have slightly different pixel output, due to different
+// graphics implementations. Slightly different pixels (in BGR space) are still
+// counted as a matching pixel by this simple manhattan distance threshold.
+const int kPixelManhattanDistanceTolerance = 8;
+
 std::string RunTestScript(base::StringPiece test_script,
                           content::WebContents* contents,
                           const std::string& element_id) {
@@ -105,13 +126,108 @@ scoped_ptr<net::test_server::HttpResponse> RespondWithHTML(
   return response.Pass();
 }
 
+bool SnapshotMatches(const base::FilePath& reference, const SkBitmap& bitmap) {
+  base::File::Info info;
+  if (!base::GetFileInfo(reference, &info))
+    return false;
+  int size = static_cast<size_t>(info.size);
+  scoped_ptr<char[]> data(new char[size]);

[Lei Zhang, 2015/10/21 07:24:52]

Just use a std::vector, sans scoped_ptr. vector_as_array() may be helpful.

[tommycli, 2015/10/21 18:16:04]

Done.

+  if (base::ReadFile(reference, data.get(), size) != size)

[Lei Zhang, 2015/10/21 07:24:52]

Or just use ReadFileToString().

[tommycli, 2015/10/21 18:16:04]

Done.

+    return false;
+
+  int w, h;

[Lei Zhang, 2015/10/21 07:24:52]

1 decl per line

[tommycli, 2015/10/21 18:16:04]

Done.

+  std::vector<unsigned char> decoded;
+  if (!gfx::PNGCodec::Decode(reinterpret_cast<unsigned char*>(data.get()), size,
+                             gfx::PNGCodec::FORMAT_BGRA, &decoded, &w, &h)) {
+    return false;
+  }
+
+  if (bitmap.width() < kComparisonWidth || w < kComparisonWidth ||

[Lei Zhang, 2015/10/21 07:24:52]

You could have done this first without even opening the file.

[tommycli, 2015/10/21 18:16:04]

Done.

+      bitmap.height() < kComparisonHeight || h < kComparisonHeight) {
+    return false;
+  }
+
+  int32* ref_pixels = reinterpret_cast<int32*>(&decoded[0]);

[Lei Zhang, 2015/10/21 07:24:52]

Use stdint.h types - int32_t

[tommycli, 2015/10/21 18:16:04]

Done.

+
+  SkAutoLockPixels lock_image(bitmap);
+  int32* pixels = static_cast<int32*>(bitmap.getPixels());
+
+  int stride = bitmap.rowBytes();
+  for (int y = 0; y < kComparisonHeight; ++y) {
+    for (int x = 0; x < kComparisonWidth; ++x) {
+      int32 pixel = pixels[y * stride / sizeof(int32) + x];
+      int pixel_b = pixel & 0xFF;
+      int pixel_g = (pixel >> 8) & 0xFF;
+      int pixel_r = (pixel >> 16) & 0xFF;
+
+      int32 ref_pixel = ref_pixels[y * w + x];
+      int ref_pixel_b = ref_pixel & 0xFF;
+      int ref_pixel_g = (ref_pixel >> 8) & 0xFF;
+      int ref_pixel_r = (ref_pixel >> 16) & 0xFF;
+
+      int manhattan_distance = abs(pixel_b - ref_pixel_b) +
+                               abs(pixel_g - ref_pixel_g) +
+                               abs(pixel_r - ref_pixel_r);
+
+      if (manhattan_distance > kPixelManhattanDistanceTolerance)
+        return false;
+    }
+  }
+
+  return true;
+}
+
+// |snapshot_matches| is set to true if the snapshot matches the reference and
+// the test passes. Otherwise, set to false.
+void CompareSnapshotToReference(const base::FilePath& reference,
+                                bool* snapshot_matches,
+                                const base::Closure& done_cb,
+                                const SkBitmap& bitmap,
+                                content::ReadbackResponse response) {
+  DCHECK(snapshot_matches);
+  if (response != content::READBACK_SUCCESS) {
+    *snapshot_matches = false;
+    done_cb.Run();
+    return;
+  }
+
+  *snapshot_matches = SnapshotMatches(reference, bitmap);
+
+  // When rebaselining the pixel test, the test will fail, and we will
+  // overwrite the reference file. On the next try through, the test will then
+  // pass, since we just overwrote the reference file. A bit wonky.
+  if (!(*snapshot_matches) &&
+      base::CommandLine::ForCurrentProcess()->HasSwitch(

[Lei Zhang, 2015/10/21 07:24:52]

This is just for convenience, right?

[tommycli, 2015/10/21 18:16:04]

Yeah exactly. I'm not married to this. Since kCCRebaselinePixeltests[] already
exists, I figured having a kRebaselinePixelTests also made sense.

+          switches::kRebaselinePixelTests)) {
+    SkBitmap clipped_bitmap;
+    bitmap.extractSubset(&clipped_bitmap,
+                         SkIRect::MakeWH(kComparisonWidth, kComparisonHeight));
+    std::vector<unsigned char> png_data;
+    gfx::PNGCodec::EncodeBGRASkBitmap(clipped_bitmap, false, &png_data);
+    base::WriteFile(reference, reinterpret_cast<char*>(&png_data[0]),

[Lei Zhang, 2015/10/21 07:24:52]

Use vector_as_array.

[tommycli, 2015/10/21 18:16:04]

Done.

+                    png_data.size());
+  }
+
+  done_cb.Run();
+}
+
 }  // namespace
 
 class PluginPowerSaverBrowserTest : public InProcessBrowserTest {
  public:
+  void SetUp() override {
+    EnablePixelOutput();
+    InProcessBrowserTest::SetUp();
+  }
+
   void SetUpOnMainThread() override {
     InProcessBrowserTest::SetUpOnMainThread();
     ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
+
+    // Reuse an image from the printing tests as a poster image.

[Lei Zhang, 2015/10/21 07:24:52]

You are adding a plugin_power_saver dir. Use your own image.

[tommycli, 2015/10/21 18:16:04]

Done.

+    embedded_test_server()->ServeFilesFromDirectory(
+        ui_test_utils::GetTestFilePath(
+            base::FilePath(FILE_PATH_LITERAL("printing")), base::FilePath()));
   }
 
   void SetUpCommandLine(base::CommandLine* command_line) override {
@@ -126,6 +242,13 @@ class PluginPowerSaverBrowserTest : public InProcessBrowserTest {
 
  protected:
   void LoadHTML(const std::string& html) {
+    gfx::Rect bounds(gfx::Rect(0, 0, kBrowserWidth, kBrowserHeight));
+    gfx::Rect screen_bounds =
+        gfx::Screen::GetNativeScreen()->GetPrimaryDisplay().bounds();
+    ASSERT_GT(screen_bounds.width(), kBrowserWidth);
+    ASSERT_GT(screen_bounds.height(), kBrowserHeight);
+    browser()->window()->SetBounds(bounds);
+
     ASSERT_TRUE(embedded_test_server()->Started());
     embedded_test_server()->RegisterRequestHandler(
         base::Bind(&RespondWithHTML, html));
@@ -168,6 +291,51 @@ class PluginPowerSaverBrowserTest : public InProcessBrowserTest {
 
     VerifyPluginMarkedEssential(GetActiveWebContents(), element_id);
   }
+
+  bool VerifySnapshot(const std::string& expected_filename) {
+    // If we are rebaselining, give the placeholders an extra 2 seconds to

[Lei Zhang, 2015/10/21 07:24:52]

What if your machine has a bad day? Are you going to write out the wrong
reference image?

[tommycli, 2015/10/21 18:16:04]

Yes... and there's no real way to tell that it's the wrong reference image
without human inspection. Open to suggestions...

[Lei Zhang, 2015/10/21 23:25:32]

Are there any JS / DOM events that corrolates with when the plugin finishes
loading? Are there ways to add some?

[tommycli, 2015/10/21 23:57:22]

Oh - I tried so hard believe me, the existing VerifyPluginIsThrottled and
SimulateClickAndAwaitMarkedEssential already do a huge song and dance between
Pepper and JS. Fires a custom DOM event actually.

The problem is that even when that's done, it may not have arrived in the
backing store.

[Lei Zhang, 2015/10/22 00:16:13]

Can you ask the compositor folks if there's a better way, assuming they are the
right folks to talk to?

[tommycli, 2015/10/22 19:17:24]

Yes - I'll reach out to piman / some other compositor guy to get some more
advice on how to avoid the hacky RunLoopUntil.

[tommycli, 2015/10/22 22:32:42]

I have trying to implement pixel tests for Plugin Power Saver, however, I am
having difficulty with timing issues.

The pixels I copy from the backing store are often older than I expect, even
after trying to synchronize the state between Pepper and JavaScript.

I have been forced to use a hacky RunLoopUntil method to wait for the backing
store pixels to match the reference image.

Line 75 of
https://codereview.chromium.org/1128453003/diff/20001/chrome/browser/ui/pdf/p...
seems to make a comment to the same effect.

The FlashFullscreenInteractiveBrowsertest also uses the same method.

thestig@ suggested I contact some compositor folks, as they may have a better
idea - or can shed more light on the topic.

[piman, 2015/10/23 01:45:31]

Well, it's not exactly simple to do it accurately... After the renderer received
a frame from the plugin, you need to wait:
1- for the compositor to commit the layer tree (on the renderer main thread,
asynchronously)
2- then for the compositor to send a frame to the browser (on the renderer's
compositor thread)
3- then for the browser to receive the message (ViewHostMsg_SwapCompositorFrame)
and dispatches it to the view.

After that, you can queue the CopyFromBackingStore, and it should give you the
right thing when it gives you the callback.


The problem is that it's not possible to directly observe (1) and (2) from a
browser test.

I think the best thing you can do is to use
RenderFrameHost::InsertVisualStateCallback, on the WebContent's main frame which
will call you back as soon as the renderer state known so far went through (1),
(2), and (3). Actually that may be your best solution methinks. You do need to
know (from the browser test side) that the renderer has received a frame from
the plugin though.

+    // settle, so we write the correct reference images. See comment below.
+    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kRebaselinePixelTests)) {
+      base::MessageLoop::current()->task_runner()->PostDelayedTask(
+          FROM_HERE, base::MessageLoop::QuitWhenIdleClosure(),
+          base::TimeDelta::FromSeconds(2));
+      content::RunMessageLoop();
+    }
+
+    // Because of how Pepper painting works, even if the plugin is ready, the
+    // image data may not have arrived in the backing store. Retry verifying
+    // the snapshot with a timeout.
+    return ui_test_utils::RunLoopUntil(

[Lei Zhang, 2015/10/21 07:24:52]

So if the test is going to fail, we'll retry a bunch of times anyway?

If the renderer / plugin is running really slow, we'll flake due to the timeout?

[tommycli, 2015/10/21 18:16:04]

Yes, we keep retrying, but there's a timeout of 5 seconds (within RunLoopUntil).
If it doesn't pass within 5 seconds, it's marked as an explicit failure.

+        base::Bind(&PluginPowerSaverBrowserTest::TryVerifySnapshot,
+                   base::Unretained(this), expected_filename));
+  }
+
+ private:
+  bool TryVerifySnapshot(const std::string& expected_filename) {
+    bool snapshot_matches = false;
+    base::FilePath reference = ui_test_utils::GetTestFilePath(
+        base::FilePath(FILE_PATH_LITERAL("plugin_power_saver")),
+        base::FilePath().AppendASCII(expected_filename));

[Lei Zhang, 2015/10/21 07:24:52]

Pass |expected_filename| as a FilePath all the way down?

[tommycli, 2015/10/21 18:16:04]

Done. It is converted to a FilePath in VerifySnapshot now.

[Lei Zhang, 2015/10/21 23:25:32]

Why not keep going and change VerifySnapshot() too?

[tommycli, 2015/10/21 23:57:22]

Ah... because it would cause me to write ui_test_utils::GetTestFilePath(
         base::FilePath(FILE_PATH_LITERAL("plugin_power_saver")),
         base::FilePath().AppendASCII(...) in 4 different places.

[Lei Zhang, 2015/10/22 00:16:13]

Can we do this?

bool VerifySnapshot(const base::FilePath::StringType& expected_filename);

...

VerifySnapshot(FILE_PATH_LITERAL("small_cross_origin_expected.png"));

...

bool TryVerifySnapshot(const base::FilePath::StringType& expected_filename) {
  base::FilePath reference = ui_test_utils::GetTestFilePath(
  base::FilePath(FILE_PATH_LITERAL("plugin_power_saver")),
  base::FilePath(expected_filename));
  ...
}

or do you have to do the wonky base::FilePath().AppendASCII(expected_filename));
?

[tommycli, 2015/10/22 19:17:24]

Done. Yeah that's fine with me.

+    content::RenderWidgetHost* rwh =
+        GetActiveWebContents()->GetRenderViewHost()->GetWidget();
+
+    // When a widget is first shown, it can take some time before it is ready
+    // for copying from its backing store.  This is a transient condition, and
+    // so it is not being treated as a test failure.
+    if (!rwh->CanCopyFromBackingStore())
+      return false;
+
+    rwh->CopyFromBackingStore(
+        gfx::Rect(), gfx::Size(),
+        base::Bind(&CompareSnapshotToReference, reference, &snapshot_matches,
+                   base::MessageLoop::QuitWhenIdleClosure()),
+        kN32_SkColorType);
+
+    content::RunMessageLoop();
+
+    return snapshot_matches;
+  }
 };
 
 IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, SmallSameOrigin) {
@@ -182,10 +350,20 @@ IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, SmallCrossOrigin) {
   LoadHTML(
       "<object id='plugin' data='http://otherorigin.com/fake.swf' "
       "    type='application/x-ppapi-tests' width='400' height='100'>"
+      "</object>"
+      "<br>"
+      "<object id='plugin_poster' data='http://otherorigin.com/fake.swf' "
+      "    type='application/x-ppapi-tests' width='400' height='100' "
+      "    poster='click_me.png'>"
       "</object>");
+
   VerifyPluginIsThrottled(GetActiveWebContents(), "plugin");
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_poster"));
+
+  EXPECT_TRUE(VerifySnapshot("small_cross_origin_expected.png"));
 
   SimulateClickAndAwaitMarkedEssential("plugin", gfx::Point(50, 50));
+  SimulateClickAndAwaitMarkedEssential("plugin_poster", gfx::Point(50, 150));
 }
 
 IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, LargeCrossOrigin) {
@@ -200,29 +378,93 @@ IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, LargeCrossOrigin) {
   VerifyPluginMarkedEssential(GetActiveWebContents(), "medium_16_9");
 }
 
-IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest,
-                       LargePluginsPeripheralWhenPosterSpecified) {
+IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, SmallerThanPlayIcon) {
+  LoadHTML(
+      "<object id='plugin_16' type='application/x-ppapi-tests' "
+      "    width='16' height='16'></object>"
+      "<object id='plugin_32' type='application/x-ppapi-tests' "
+      "    width='32' height='32'></object>"
+      "<object id='plugin_16_64' type='application/x-ppapi-tests' "
+      "    width='16' height='64'></object>"
+      "<object id='plugin_64_16' type='application/x-ppapi-tests' "
+      "    width='64' height='16'></object>");
+
+  VerifyPluginIsThrottled(GetActiveWebContents(), "plugin_16");
+  VerifyPluginIsThrottled(GetActiveWebContents(), "plugin_32");
+  VerifyPluginIsThrottled(GetActiveWebContents(), "plugin_16_64");
+  VerifyPluginIsThrottled(GetActiveWebContents(), "plugin_64_16");
+
+  EXPECT_TRUE(VerifySnapshot("smaller_than_play_icon_expected.png"));
+}
+
+IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, PosterTests) {
+  // This test simulateously verifies the varied supported poster syntaxes,

[Lei Zhang, 2015/10/21 07:24:52]

typo

[tommycli, 2015/10/21 18:16:04]

Done.

+  // as well as verifies that the poster is rendered correctly with various
+  // mismatched aspect ratios and sizes, following the same rules as VIDEO.
   LoadHTML(
       "<object id='plugin_src' type='application/x-ppapi-tests' "
-      "    width='400' height='500' poster='snapshot1x.png'></object>"
+      "    width='100' height='100' poster='click_me.png'></object>"
       "<object id='plugin_srcset' type='application/x-ppapi-tests' "
-      "    width='400' height='500' "
-      "    poster='snapshot1x.png 1x, snapshot2x.png 2x'></object>"
-      "<object id='plugin_legacy_syntax' type='application/x-ppapi-tests' "
-      "    width='400' height='500'>"
-      "  <param name='poster' value='snapshot1x.png 1x, snapshot2x.png 2x'>"
+      "    width='100' height='100' "
+      "    poster='click_me.png 1x, click_me.png 2x'></object>"
+      "<br>"
+
+      "<object id='plugin_poster_param' type='application/x-ppapi-tests' "
+      "    width='100' height='100'>"
+      "  <param name='poster' value='click_me.png 1x, click_me.png 2x'>"
       "</object>"
       "<embed id='plugin_embed_src' type='application/x-ppapi-tests' "
-      "    width='400' height='500' poster='snapshot1x.png'></embed>"
+      "    width='100' height='100' poster='click_me.png'></embed>"
       "<embed id='plugin_embed_srcset' type='application/x-ppapi-tests' "
-      "    width='400' height='500'"
-      "    poster='snapshot1x.png 1x, snapshot2x.png 2x'></embed>");
+      "    width='100' height='100'"
+      "    poster='click_me.png 1x, click_me.png 2x'></embed>"
+      "<br>"
+
+      "<object id='poster_missing' type='application/x-ppapi-tests' "
+      "    width='100' height='100' poster='missing.png'></object>"
+      "<object id='poster_too_small' type='application/x-ppapi-tests' "
+      "    width='100' height='50' poster='click_me.png'></object>"
+      "<object id='poster_too_big' type='application/x-ppapi-tests' "
+      "    width='100' height='150' poster='click_me.png'></object>"
+      "<br>"
+
+      "<object id='poster_16' type='application/x-ppapi-tests' "
+      "    width='16' height='16' poster='click_me.png'></object>"
+      "<object id='poster_32' type='application/x-ppapi-tests' "
+      "    width='32' height='32' poster='click_me.png'></object>"
+      "<object id='poster_16_64' type='application/x-ppapi-tests' "
+      "    width='16' height='64' poster='click_me.png'></object>"
+      "<object id='poster_64_16' type='application/x-ppapi-tests' "
+      "    width='64' height='16' poster='click_me.png'></object>"
+      "<br>"
+
+      "<div id='container' "
+      "    style='width: 400px; height: 100px; overflow: hidden;'>"
+      "  <object id='poster_obscured' data='http://otherorigin.com/fake.swf' "
+      "      type='application/x-ppapi-tests' width='400' height='500' "
+      "      poster='click_me.png'>"
+      "  </object>"
+      "</div>");
 
   EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_src"));
   EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_srcset"));
-  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_legacy_syntax"));
+
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_poster_param"));
   EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_embed_src"));
   EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "plugin_embed_srcset"));
+
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "poster_missing"));
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "poster_too_small"));
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "poster_too_big"));
+
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "poster_16"));
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "poster_32"));
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "poster_16_64"));
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "poster_64_16"));
+
+  EXPECT_FALSE(PluginLoaded(GetActiveWebContents(), "poster_obscured"));
+
+  EXPECT_TRUE(VerifySnapshot("poster_tests_expected.png"));
 }
 
 IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest,
@@ -249,12 +491,14 @@ IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, OriginWhitelisting) {
 IN_PROC_BROWSER_TEST_F(PluginPowerSaverBrowserTest, LargeCrossOriginObscured) {
   LoadHTML(
       "<div id='container' "
-      "    style='width: 400px; height: 100px; overflow: hidden;'>"
+      "    style='width: 100px; height: 400px; overflow: hidden;'>"
       "  <object id='plugin' data='http://otherorigin.com/fake.swf' "
-      "      type='application/x-ppapi-tests' width='400' height='500'>"
+      "      type='application/x-ppapi-tests' width='400' height='500' "
+      "      style='float: right;'>"
       "  </object>"
       "</div>");
   VerifyPluginIsThrottled(GetActiveWebContents(), "plugin");
+  EXPECT_TRUE(VerifySnapshot("large_cross_origin_obscured_expected.png"));
 
   // Test that's unthrottled if it is unobscured.
   std::string script =