Video to canvas: don't draw if a SkImage can't be created.

Video to canvas: don't draw if a SkImage can't be created.

Skia can return nullptr from NewFromAdoptedTexture and NewFromYUVTexturesCopy after
a context loss.
If this happens SkCanvasVideoRenderer should not draw anything instead of crashing.

BUG=526615
Committed: https://crrev.com/35fb56409a8ec2d873d95fdf130f98e39d21e938
Cr-Commit-Position: refs/heads/master@{#347810}

[Daniele Castagna, 2015-09-04 17:51:53]

dcastagna@chromium.org changed reviewers:
+ junov@chromium.org

[Justin Novosad, 2015-09-04 17:58:18]

Code looks good, but I think this calls for a unit test.

[Daniele Castagna, 2015-09-04 21:15:09]

On 2015/09/04 at 17:58:18, junov wrote:
> Code looks good, but I think this calls for a unit test.

Done.

[Daniele Castagna, 2015-09-04 21:21:30]

dcastagna@chromium.org changed reviewers:
+ dalecurtis@chromium.org

[Daniele Castagna, 2015-09-04 21:21:30]

+dalecurtis for ownership approval, assuming junov is fine with the added test.

[DaleCurtis, 2015-09-04 21:27:10]

lgtm % nit

https://codereview.chromium.org/1325173005/diff/20001/media/blink/skcanvas_vi...
File media/blink/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/1325173005/diff/20001/media/blink/skcanvas_vi...
media/blink/skcanvas_video_renderer.cc:336: if (last_image_) {
if (!last_image_)
  return;

[Daniele Castagna, 2015-09-04 21:31:32]

https://codereview.chromium.org/1325173005/diff/20001/media/blink/skcanvas_vi...
File media/blink/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/1325173005/diff/20001/media/blink/skcanvas_vi...
media/blink/skcanvas_video_renderer.cc:336: if (last_image_) {
On 2015/09/04 at 21:27:09, DaleCurtis wrote:
> if (!last_image_)
>   return;

Done.

[Justin Novosad, 2015-09-08 19:13:58]

lgtm with nits.

[Justin Novosad, 2015-09-08 19:14:17]

https://codereview.chromium.org/1325173005/diff/60001/media/blink/skcanvas_vi...
File media/blink/skcanvas_video_renderer_unittest.cc (right):

https://codereview.chromium.org/1325173005/diff/60001/media/blink/skcanvas_vi...
media/blink/skcanvas_video_renderer_unittest.cc:515:
TEST_F(SkCanvasVideoRendererTest, ContextLost) {
Add a comment to say that this test passes by not crashing.

https://codereview.chromium.org/1325173005/diff/60001/media/blink/skcanvas_vi...
media/blink/skcanvas_video_renderer_unittest.cc:522:
gr_context->abandonContext();
Nit: I would have put this after constructing Context3D to make it clear that
the use case this test is intetersted in has to do with a working context that
was dropped on the floor.

[Daniele Castagna, 2015-09-08 19:24:24]

Thank you for the review!

https://codereview.chromium.org/1325173005/diff/60001/media/blink/skcanvas_vi...
File media/blink/skcanvas_video_renderer_unittest.cc (right):

https://codereview.chromium.org/1325173005/diff/60001/media/blink/skcanvas_vi...
media/blink/skcanvas_video_renderer_unittest.cc:515:
TEST_F(SkCanvasVideoRendererTest, ContextLost) {
On 2015/09/08 at 19:14:17, Justin Novosad wrote:
> Add a comment to say that this test passes by not crashing.

Done.

https://codereview.chromium.org/1325173005/diff/60001/media/blink/skcanvas_vi...
media/blink/skcanvas_video_renderer_unittest.cc:522:
gr_context->abandonContext();
On 2015/09/08 at 19:14:17, Justin Novosad wrote:
> Nit: I would have put this after constructing Context3D to make it clear that
the use case this test is intetersted in has to do with a working context that
was dropped on the floor.

Done.

[Daniele Castagna, 2015-09-08 19:24:27]

The CQ bit was checked by dcastagna@chromium.org

[Daniele Castagna, 2015-09-08 19:24:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, junov@chromium.org
Link to the patchset: https://codereview.chromium.org/1325173005/#ps80001
(title: "Address junov's comments.")

[commit-bot: I haz the power, 2015-09-08 19:25:09]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1325173005/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1325173005/80001

[commit-bot: I haz the power, 2015-09-08 20:41:06]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-09-08 20:41:07]

Failed to apply patch for media/blink/skcanvas_video_renderer_unittest.cc:
While running git apply --index -3 -p1;
  error: patch failed: media/blink/skcanvas_video_renderer_unittest.cc:3
  Falling back to three-way merge...
  Applied patch to 'media/blink/skcanvas_video_renderer_unittest.cc' with
conflicts.
  U media/blink/skcanvas_video_renderer_unittest.cc

Patch:       media/blink/skcanvas_video_renderer_unittest.cc
Index: media/blink/skcanvas_video_renderer_unittest.cc
diff --git a/media/blink/skcanvas_video_renderer_unittest.cc
b/media/blink/skcanvas_video_renderer_unittest.cc
index
9b8765aa4c5784eed6fc273c9a2634adb8b9e338..348b497b5ef73b0782088987b91665e2d9951828
100644
--- a/media/blink/skcanvas_video_renderer_unittest.cc
+++ b/media/blink/skcanvas_video_renderer_unittest.cc
@@ -3,12 +3,16 @@
 // found in the LICENSE file.
 
 #include "base/message_loop/message_loop.h"
+#include "gpu/GLES2/gl2extchromium.h"
+#include "gpu/command_buffer/client/gles2_interface_stub.h"
 #include "media/base/timestamp_constants.h"
 #include "media/base/video_frame.h"
 #include "media/base/video_util.h"
 #include "media/blink/skcanvas_video_renderer.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/skia/include/core/SkCanvas.h"
+#include "third_party/skia/include/gpu/GrContext.h"
+#include "third_party/skia/include/gpu/gl/GrGLInterface.h"
 
 using media::VideoFrame;
 
@@ -75,7 +79,7 @@ class SkCanvasVideoRendererTest : public testing::Test {
   // Standard canvas.
   SkCanvas* target_canvas() { return &target_canvas_; }
 
- private:
+ protected:
   SkCanvasVideoRenderer renderer_;
 
   scoped_refptr<VideoFrame> natural_frame_;
@@ -497,4 +501,39 @@ TEST_F(SkCanvasVideoRendererTest,
Video_Translate_Rotation_270) {
   EXPECT_EQ(SK_ColorBLACK, GetColorAt(&canvas, kWidth / 2, kHeight - 1));
 }
 
+namespace {
+class TestGLES2Interface : public gpu::gles2::GLES2InterfaceStub {
+ public:
+  void GenTextures(GLsizei n, GLuint* textures) override {
+    DCHECK_EQ(1, n);
+    *textures = 1;
+  }
+};
+void MailboxHoldersReleased(uint32 sync_point) {}
+}  // namespace
+
+// Test that SkCanvasVideoRendererTest::Paint doesn't crash when GrContext is
+// abandoned.
+TEST_F(SkCanvasVideoRendererTest, ContextLost) {
+  skia::RefPtr<const GrGLInterface> null_interface =
+      skia::AdoptRef(GrGLCreateNullInterface());
+  auto gr_context = skia::AdoptRef(GrContext::Create(
+      kOpenGL_GrBackend,
+      reinterpret_cast<GrBackendContext>(null_interface.get())));
+  gr_context->abandonContext();
+
+  SkCanvas canvas(AllocBitmap(kWidth, kHeight));
+
+  TestGLES2Interface gles2;
+  Context3D context_3d(&gles2, gr_context.get());
+  gfx::Size size(kWidth, kHeight);
+  gpu::MailboxHolder mailbox(gpu::Mailbox::Generate(),
GL_TEXTURE_RECTANGLE_ARB,
+                             0);
+  auto video_frame = VideoFrame::WrapNativeTexture(
+      PIXEL_FORMAT_UYVY, mailbox, base::Bind(MailboxHoldersReleased), size,
+      gfx::Rect(size), size, kNoTimestamp());
+
+  renderer_.Paint(video_frame, &canvas, kNaturalRect, 0xFF,
+                  SkXfermode::kSrcOver_Mode, VIDEO_ROTATION_90, context_3d);
+}
 }  // namespace media

[commit-bot: I haz the power, 2015-09-08 20:41:52]

The CQ bit was unchecked by commit-bot@chromium.org

[Daniele Castagna, 2015-09-08 20:58:13]

The CQ bit was checked by dcastagna@chromium.org

[Daniele Castagna, 2015-09-08 20:58:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
dalecurtis@chromium.org, junov@chromium.org
Link to the patchset: https://codereview.chromium.org/1325173005/#ps100001
(title: "Rebase on master.")

[commit-bot: I haz the power, 2015-09-08 20:58:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1325173005/100001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1325173005/100001

[commit-bot: I haz the power, 2015-09-08 22:21:55]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-09-08 22:22:33]

Patchset 6 (id:??) landed as
https://crrev.com/35fb56409a8ec2d873d95fdf130f98e39d21e938
Cr-Commit-Position: refs/heads/master@{#347810}