[devtools] Support different encodings for Page.CaptureScreenshot.

ui/snapshot/snapshot_aura_unittest.cc

 // Copyright (c) 2013 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 "ui/snapshot/snapshot.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include "base/bind.h"
@@ skipping 122 matching lines @@
     delegate_.reset(new TestPaintingWindowDelegate(window_bounds.size()));
     test_window_.reset(aura::test::CreateTestWindowWithDelegate(
         delegate_.get(), 0, window_bounds, root_window()));
   }
 
   gfx::Image GrabSnapshotForTestWindow() {
     gfx::Rect source_rect(test_window_->bounds().size());
     aura::Window::ConvertRectToTarget(
         test_window(), root_window(), &source_rect);
 
-    scoped_refptr<base::TestSimpleTaskRunner> task_runner(
-        new base::TestSimpleTaskRunner());
     scoped_refptr<SnapshotHolder> holder(new SnapshotHolder);
     ui::GrabWindowSnapshotAsync(
         root_window(),
         source_rect,
-        task_runner,
         base::Bind(&SnapshotHolder::SnapshotCallback, holder));
 
     // Wait for copy response.
+    // TODO(eseckler): This doesn't actually wait until the copy response is

[Eric Seckler, 2017/01/09 14:52:05]

enne@/sadrul@: These tests are super flaky and don't actually pass even before
my change. It seems they may simply not be run on the bots?

The reason for their flakiness seems to be that the WaitForDraw() below doesn't
actually ensure that the copy response was received. It looks like the
cc::Display actually calls the draw callbacks before swapping buffers. Even if
that were the case, I'm not entirely sure that the copy response from the
GLRenderer would be sent immediately during SwapBuffers (it looks async in
GLRenderer::GetFramebufferPixelsAsync).

Do you know of a more convenient event we could wait for here, or is the better
option to wait until our own callback (SnapshotHolder::SnapshotCallback) has
been run and fail through timeout otherwise?

[sadrul, 2017/01/10 05:12:50]

Ugh, it does look like these tests are not actually run on any bots :-/
I cannot tell you of what the best fix would be, but the option you suggest
doesn't sound like a bad one. enne@ can tell you what the right fix should be.

[enne (OOO), 2017/01/10 19:26:04]

It sounds likely that this has been broken ever since there was a cc::Display. 
I think waiting on your own callback is going to be the most robust solution
into the future.

[Eric Seckler, 2017/01/11 15:58:44]

OK, thanks!

Turns out, there's more failing than just this (probably for a while as well) -
apparently the screenshot bounds are affected by test_screen()->SetUIScale(),
even though the tests don't expect them to be:

[ RUN      ] SnapshotAuraTest.UIScale
../../ui/snapshot/snapshot_aura_unittest.cc:234: Failure
Value of: snapshot.Size().ToString()
  Actual: "240x160"
Expected: gfx::ToRoundedSize(snapshot_size).ToString()
Which is: "300x200"

Can you tell whether this is a valid bug - or incorrect tests?

[sadrul, 2017/01/11 17:16:53]

I think the current expectations are correct. Sounds like a valid bug? oshima@
can confirm.

+    // issued, because cc::Display::DrawAndSwap() executes surface draw
+    // callbacks before drawing the frame on the renderer.
     WaitForDraw();
-    // Run internal snapshot callback to scale/rotate response image.
-    task_runner->RunUntilIdle();
     // Run SnapshotHolder callback.
     helper_->RunAllPendingInMessageLoop();
 
     if (holder->completed())
       return holder->image();
 
     // Callback never called.
     NOTREACHED();
     return gfx::Image();
   }
 
  private:
   class SnapshotHolder : public base::RefCountedThreadSafe<SnapshotHolder> {
    public:
     SnapshotHolder() : completed_(false) {}
 
-    void SnapshotCallback(scoped_refptr<base::RefCountedBytes> png_data) {
+    void SnapshotCallback(const gfx::Image& image) {
       DCHECK(!completed_);
-      image_ = gfx::Image::CreateFrom1xPNGBytes(&(png_data->data()[0]),
-                                                png_data->size());
+      image_ = image;
       completed_ = true;
     }
     bool completed() const {
       return completed_;
     };
     const gfx::Image& image() const { return image_; }
 
    private:
     friend class base::RefCountedThreadSafe<SnapshotHolder>;
 
@@ skipping 109 matching lines @@
   gfx::SizeF snapshot_size(test_bounds.size());
   snapshot_size.Scale(2.0f);
 
   gfx::Image snapshot = GrabSnapshotForTestWindow();
   EXPECT_EQ(gfx::ToRoundedSize(snapshot_size).ToString(),
             snapshot.Size().ToString());
   EXPECT_EQ(0u, GetFailedPixelsCountWithScaleFactor(snapshot, 2));
 }
 
 }  // namespace ui