Refactor image handling in Sky to expose the loader and image as separate...

Refactor image handling in Sky to expose the loader and image as separate classes to Dart code. This makes it possible to avoid unnecessary paints, by only painting once when the image has loaded. Now that we've separated the loader and image classes, we can implement an image cache in Dart.

R=abarth@chromium.org, abarth

Review URL: https://codereview.chromium.org/1156003007

engine/core/core.gni

@@ -798,6 +798,8 @@ sky_core_files = [
   "inspector/ScriptAsyncCallStack.h",
   "inspector/ScriptGCEventListener.h",
   "layout/LayoutCallback.h",
+  "loader/CanvasImageLoader.cpp",
+  "loader/CanvasImageLoader.h",
   "loader/DocumentLoadTiming.cpp",
   "loader/DocumentLoadTiming.h",
   "loader/EmptyClients.cpp",
@@ -810,11 +812,10 @@ sky_core_files = [
   "loader/FrameLoaderTypes.h",
   "loader/ImageLoader.cpp",
   "loader/ImageLoader.h",
+  "loader/ImageLoaderCallback.h",
   "loader/MojoLoader.cpp",
   "loader/MojoLoader.h",
   "loader/NavigationPolicy.h",
-  "loader/NewImageLoader.cpp",
-  "loader/NewImageLoader.h",
   "loader/UniqueIdentifier.cpp",
   "loader/UniqueIdentifier.h",
   "page/ChromeClient.h",
@@ -1127,6 +1128,8 @@ core_idl_files = get_path_info([
                                  "html/TextMetrics.idl",
                                  "html/VoidCallback.idl",
                                  "layout/LayoutCallback.idl",
+                                 "loader/ImageLoader.idl",
+                                 "loader/ImageLoaderCallback.idl",
                                  "painting/Canvas.idl",
                                  "painting/ColorFilter.idl",
                                  "painting/DrawLooper.idl",

engine/core/loader/NewImageLoader.cpp → engine/core/loader/CanvasImageLoader.cpp

@@ -3,25 +3,29 @@
 // found in the LICENSE file.
 
 #include "sky/engine/config.h"
-#include "sky/engine/core/loader/NewImageLoader.h"
+#include "sky/engine/core/loader/CanvasImageLoader.h"
+#include "sky/engine/core/painting/CanvasImage.h"
 #include "sky/engine/platform/image-decoders/ImageDecoder.h"
 #include "sky/engine/platform/SharedBuffer.h"
 
 namespace blink {
 
-NewImageLoader::NewImageLoader(NewImageLoaderClient* client) : client_(client) {
+CanvasImageLoader::CanvasImageLoader(const String& src, PassOwnPtr<ImageLoaderCallback> callback)
+  : callback_(callback) {
+  // TODO(jackson): Figure out how to determine the proper base URL here
+  url_ = KURL(KURL(), src);
 }
 
-NewImageLoader::~NewImageLoader() {
+CanvasImageLoader::~CanvasImageLoader() {
 }
 
-void NewImageLoader::Load(const KURL& src) {
-  fetcher_ = adoptPtr(new MojoFetcher(this, src));
+void CanvasImageLoader::load() {
+  fetcher_ = adoptPtr(new MojoFetcher(this, url_));
 }
 
-void NewImageLoader::OnReceivedResponse(mojo::URLResponsePtr response) {
+void CanvasImageLoader::OnReceivedResponse(mojo::URLResponsePtr response) {
   if (response->status_code != 200) {
-    client_->OnLoadFinished(SkBitmap());
+    callback_->handleEvent(nullptr);
     return;
   }
   buffer_ = SharedBuffer::create();
@@ -29,24 +33,21 @@ void NewImageLoader::OnReceivedResponse(mojo::URLResponsePtr response) {
       adoptPtr(new mojo::common::DataPipeDrainer(this, response->body.Pass()));
 }
 
-void NewImageLoader::OnDataAvailable(const void* data, size_t num_bytes) {
+void CanvasImageLoader::OnDataAvailable(const void* data, size_t num_bytes) {
   buffer_->append(static_cast<const char*>(data), num_bytes);
 }
 
-void NewImageLoader::OnDataComplete() {
-  SkBitmap bitmap;
+void CanvasImageLoader::OnDataComplete() {
   OwnPtr<ImageDecoder> decoder =
       ImageDecoder::create(*buffer_.get(), ImageSource::AlphaPremultiplied,
                            ImageSource::GammaAndColorProfileIgnored);
   decoder->setData(buffer_.get(), true);
-
-  if (decoder->failed()) {
-    client_->OnLoadFinished(bitmap);
+  if (!decoder->failed() && decoder->frameCount() > 0) {
+    RefPtr<CanvasImage> resultImage = CanvasImage::create();
+    resultImage->setBitmap(decoder->frameBufferAtIndex(0)->getSkBitmap());
+    callback_->handleEvent(resultImage.get());
   } else {
-    if (decoder->frameCount() > 0) {
-      bitmap = decoder->frameBufferAtIndex(0)->getSkBitmap();
-    }
-    client_->OnLoadFinished(bitmap);
+    callback_->handleEvent(nullptr);
   }
 }
 

engine/core/loader/NewImageLoader.h → engine/core/loader/CanvasImageLoader.h

+// SKY_ENGINE_CORE_PAINTING_CANVASIMAGE_H_
 // Copyright 2015 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.
 
-#ifndef SKY_ENGINE_CORE_LOADER_NEWIMAGELOADER_H_
-#define SKY_ENGINE_CORE_LOADER_NEWIMAGELOADER_H_
+#ifndef SKY_ENGINE_CORE_LOADER_CANVASIMAGELOADER_H_
+#define SKY_ENGINE_CORE_LOADER_CANVASIMAGELOADER_H_
 
 #include "mojo/common/data_pipe_drainer.h"
+#include "sky/engine/core/loader/ImageLoaderCallback.h"
 #include "sky/engine/platform/SharedBuffer.h"
 #include "sky/engine/platform/fetcher/MojoFetcher.h"
+#include "sky/engine/tonic/dart_wrappable.h"
 #include "sky/engine/wtf/OwnPtr.h"
 #include "sky/engine/wtf/text/AtomicString.h"
-#include "third_party/skia/include/core/SkBitmap.h"
 
 namespace blink {
 
-class NewImageLoaderClient {
+class CanvasImageLoader : public MojoFetcher::Client,
+                          public mojo::common::DataPipeDrainer::Client,
+                          public RefCounted<CanvasImageLoader>,
+                          public DartWrappable {
+  DEFINE_WRAPPERTYPEINFO();
  public:
-  virtual void OnLoadFinished(const SkBitmap& result) = 0;
+  static PassRefPtr<CanvasImageLoader> create(const String& src, PassOwnPtr<ImageLoaderCallback> callback)
+  {
+    return adoptRef(new CanvasImageLoader(src, callback));
+  }
+  virtual ~CanvasImageLoader();
 
- protected:
-  NewImageLoaderClient() {}
-};
-
-class NewImageLoader : public MojoFetcher::Client,
-                       public mojo::common::DataPipeDrainer::Client {
- public:
-  explicit NewImageLoader(NewImageLoaderClient* client);
-  virtual ~NewImageLoader();
-
-  void Load(const KURL& src);
+  void load();
 
   // MojoFetcher::Client
   void OnReceivedResponse(mojo::URLResponsePtr) override;
@@ -38,12 +38,16 @@ class NewImageLoader : public MojoFetcher::Client,
   void OnDataComplete() override;
 
  private:
-  NewImageLoaderClient* client_;
+  explicit CanvasImageLoader(const String& src, PassOwnPtr<ImageLoaderCallback> callback);
+
   OwnPtr<MojoFetcher> fetcher_;
   OwnPtr<mojo::common::DataPipeDrainer> drainer_;
   RefPtr<SharedBuffer> buffer_;
+
+  KURL url_;
+  OwnPtr<ImageLoaderCallback> callback_;
 };
 
 }  // namespace blink
 
-#endif  // SKY_ENGINE_CORE_LOADER_NEWIMAGELOADER_H_
+#endif  // SKY_ENGINE_CORE_LOADER_CANVASIMAGELOADER_H_

engine/core/loader/ImageLoader.idl

+// Copyright 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.
+
+[
+  Constructor(DOMString src, ImageLoaderCallback callback),
+  ImplementedAs=CanvasImageLoader,
+] interface ImageLoader {
+    void load();
+};

engine/core/loader/ImageLoaderCallback.h

+// Copyright 2015 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.
+
+#ifndef SKY_ENGINE_CORE_LOADER_IMAGELOADERCALLBACK_H_
+#define SKY_ENGINE_CORE_LOADER_IMAGELOADERCALLBACK_H_
+
+#include "sky/engine/core/painting/CanvasImage.h"
+
+namespace blink {
+
+class ImageLoaderCallback {
+public:
+    virtual ~ImageLoaderCallback() {}
+    virtual void handleEvent(CanvasImage* result) = 0;
+};
+
+} // namespace blink
+
+#endif  // SKY_ENGINE_CORE_LOADER_IMAGELOADERCALLBACK_H_

engine/core/loader/ImageLoaderCallback.idl

+// Copyright 2015 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.
+
+callback interface ImageLoaderCallback {
+    void handleEvent(Image result);
+};

engine/core/painting/CanvasImage.cpp

@@ -7,7 +7,7 @@
 
 namespace blink {
 
-CanvasImage::CanvasImage() : imageLoader_(adoptPtr(new NewImageLoader(this))) {
+CanvasImage::CanvasImage() {
 }
 
 CanvasImage::~CanvasImage() {
@@ -21,19 +21,4 @@ int CanvasImage::height() const {
   return bitmap_.height();
 }
 
-void CanvasImage::setSrc(const String& url) {
-  // TODO(jackson): Figure out how to determine the proper base URL here
-  KURL newSrcURL = KURL(KURL(), url);
-  if (srcURL_ != newSrcURL) {
-    srcURL_ = newSrcURL;
-    imageLoader_->Load(srcURL_);
-  }
-}
-
-void CanvasImage::OnLoadFinished(const SkBitmap& result) {
-  // TODO(jackson): We'll eventually need a notification pathway for
-  // when the image load is complete so that we know to repaint, etc.
-  bitmap_ = result;
-}
-
 }  // namespace blink
\ No newline at end of file

engine/core/painting/CanvasImage.h

@@ -5,7 +5,6 @@
 #ifndef SKY_ENGINE_CORE_PAINTING_CANVASIMAGE_H_
 #define SKY_ENGINE_CORE_PAINTING_CANVASIMAGE_H_
 
-#include "sky/engine/core/loader/NewImageLoader.h"
 #include "sky/engine/platform/weborigin/KURL.h"
 #include "sky/engine/tonic/dart_wrappable.h"
 #include "sky/engine/wtf/PassRefPtr.h"
@@ -15,8 +14,7 @@
 namespace blink {
 
 class CanvasImage final : public RefCounted<CanvasImage>,
-                          public DartWrappable,
-                          public NewImageLoaderClient {
+                          public DartWrappable {
   DEFINE_WRAPPERTYPEINFO();
  public:
   ~CanvasImage() override;
@@ -25,21 +23,13 @@ class CanvasImage final : public RefCounted<CanvasImage>,
   int width() const;
   int height() const;
 
-  KURL src() const { return srcURL_; }
-  void setSrc(const String&);
-
   const SkBitmap& bitmap() const { return bitmap_; }
   void setBitmap(const SkBitmap& bitmap) { bitmap_ = bitmap; }
 
  private:
   CanvasImage();
 
-  // NewImageLoaderClient
-  void OnLoadFinished(const SkBitmap& result) override;
-
-  KURL srcURL_;
   SkBitmap bitmap_;
-  OwnPtr<NewImageLoader> imageLoader_;
 };
 
 }  // namespace blink

engine/core/painting/Image.idl

@@ -8,5 +8,4 @@
 ] interface Image {
     readonly attribute long width;
     readonly attribute long height;
-    attribute DOMString src;
 };

examples/raw/spinning_image.dart

@@ -6,7 +6,7 @@ import 'dart:sky';
 
 double timeBase = null;
 
-Image image;
+Image image = null;
 
 void beginFrame(double timeStamp) {
   if (timeBase == null) timeBase = timeStamp;
@@ -16,14 +16,21 @@ void beginFrame(double timeStamp) {
   canvas.rotateDegrees(delta / 10);
   canvas.scale(0.2, 0.2);
   Paint paint = new Paint()..setARGB(255, 0, 255, 0);
-  canvas.drawImage(image, -image.width / 2.0, -image.height / 2.0, paint);
+  if (image != null)
+    canvas.drawImage(image, -image.width / 2.0, -image.height / 2.0, paint);
   view.picture = canvas.endRecording();
   view.scheduleFrame();
 }
 
 void main() {
-  image = new Image();
-  image.src = "https://www.dartlang.org/logos/dart-logo.png";
+  new ImageLoader("https://www.dartlang.org/logos/dart-logo.png", (result) {
+    if (result != null) {
+      print("${result.width}x${result.width} image loaded!");
+      image = result;
+      view.scheduleFrame();
+    } else {
+      print("Image failed to load");
+    }
+  }).load();
   view.setBeginFrameCallback(beginFrame);
-  view.scheduleFrame();
 }