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/CanvasImageLoader.cpp

@@ -0,0 +1,54 @@
+// 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.
+
+#include "sky/engine/config.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 {
+
+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);
+}
+
+CanvasImageLoader::~CanvasImageLoader() {
+}
+
+void CanvasImageLoader::load() {
+  fetcher_ = adoptPtr(new MojoFetcher(this, url_));
+}
+
+void CanvasImageLoader::OnReceivedResponse(mojo::URLResponsePtr response) {
+  if (response->status_code != 200) {
+    callback_->handleEvent(nullptr);
+    return;
+  }
+  buffer_ = SharedBuffer::create();
+  drainer_ =
+      adoptPtr(new mojo::common::DataPipeDrainer(this, response->body.Pass()));
+}
+
+void CanvasImageLoader::OnDataAvailable(const void* data, size_t num_bytes) {
+  buffer_->append(static_cast<const char*>(data), num_bytes);
+}
+
+void CanvasImageLoader::OnDataComplete() {
+  OwnPtr<ImageDecoder> decoder =
+      ImageDecoder::create(*buffer_.get(), ImageSource::AlphaPremultiplied,
+                           ImageSource::GammaAndColorProfileIgnored);
+  decoder->setData(buffer_.get(), true);
+  if (!decoder->failed() && decoder->frameCount() > 0) {
+    RefPtr<CanvasImage> resultImage = CanvasImage::create();
+    resultImage->setBitmap(decoder->frameBufferAtIndex(0)->getSkBitmap());
+    callback_->handleEvent(resultImage.get());
+  } else {
+    callback_->handleEvent(nullptr);
+  }
+}
+
+}  // namespace blink

engine/core/loader/CanvasImageLoader.h

@@ -0,0 +1,53 @@
+// 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_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"
+
+namespace blink {
+
+class CanvasImageLoader : public MojoFetcher::Client,
+                          public mojo::common::DataPipeDrainer::Client,
+                          public RefCounted<CanvasImageLoader>,
+                          public DartWrappable {
+  DEFINE_WRAPPERTYPEINFO();
+ public:
+  static PassRefPtr<CanvasImageLoader> create(const String& src, PassOwnPtr<ImageLoaderCallback> callback)
+  {
+    return adoptRef(new CanvasImageLoader(src, callback));
+  }
+  virtual ~CanvasImageLoader();
+
+  void load();
+
+  // MojoFetcher::Client
+  void OnReceivedResponse(mojo::URLResponsePtr) override;
+
+  // mojo::common::DataPipeDrainer::Client
+  void OnDataAvailable(const void*, size_t) override;
+  void OnDataComplete() override;
+
+ private:
+  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_CANVASIMAGELOADER_H_

engine/core/loader/ImageLoader.idl

@@ -0,0 +1,10 @@
+// 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

@@ -0,0 +1,20 @@
+// 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

@@ -0,0 +1,7 @@
+// 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/loader/NewImageLoader.cpp

@@ -1,53 +0,0 @@
-// 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.
-
-#include "sky/engine/config.h"
-#include "sky/engine/core/loader/NewImageLoader.h"
-#include "sky/engine/platform/image-decoders/ImageDecoder.h"
-#include "sky/engine/platform/SharedBuffer.h"
-
-namespace blink {
-
-NewImageLoader::NewImageLoader(NewImageLoaderClient* client) : client_(client) {
-}
-
-NewImageLoader::~NewImageLoader() {
-}
-
-void NewImageLoader::Load(const KURL& src) {
-  fetcher_ = adoptPtr(new MojoFetcher(this, src));
-}
-
-void NewImageLoader::OnReceivedResponse(mojo::URLResponsePtr response) {
-  if (response->status_code != 200) {
-    client_->OnLoadFinished(SkBitmap());
-    return;
-  }
-  buffer_ = SharedBuffer::create();
-  drainer_ =
-      adoptPtr(new mojo::common::DataPipeDrainer(this, response->body.Pass()));
-}
-
-void NewImageLoader::OnDataAvailable(const void* data, size_t num_bytes) {
-  buffer_->append(static_cast<const char*>(data), num_bytes);
-}
-
-void NewImageLoader::OnDataComplete() {
-  SkBitmap bitmap;
-  OwnPtr<ImageDecoder> decoder =
-      ImageDecoder::create(*buffer_.get(), ImageSource::AlphaPremultiplied,
-                           ImageSource::GammaAndColorProfileIgnored);
-  decoder->setData(buffer_.get(), true);
-
-  if (decoder->failed()) {
-    client_->OnLoadFinished(bitmap);
-  } else {
-    if (decoder->frameCount() > 0) {
-      bitmap = decoder->frameBufferAtIndex(0)->getSkBitmap();
-    }
-    client_->OnLoadFinished(bitmap);
-  }
-}
-
-}  // namespace blink

engine/core/loader/NewImageLoader.h

@@ -1,49 +0,0 @@
-// 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_
-
-#include "mojo/common/data_pipe_drainer.h"
-#include "sky/engine/platform/SharedBuffer.h"
-#include "sky/engine/platform/fetcher/MojoFetcher.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 {
- public:
-  virtual void OnLoadFinished(const SkBitmap& result) = 0;
-
- protected:
-  NewImageLoaderClient() {}
-};
-
-class NewImageLoader : public MojoFetcher::Client,
-                       public mojo::common::DataPipeDrainer::Client {
- public:
-  explicit NewImageLoader(NewImageLoaderClient* client);
-  virtual ~NewImageLoader();
-
-  void Load(const KURL& src);
-
-  // MojoFetcher::Client
-  void OnReceivedResponse(mojo::URLResponsePtr) override;
-
-  // mojo::common::DataPipeDrainer::Client
-  void OnDataAvailable(const void*, size_t) override;
-  void OnDataComplete() override;
-
- private:
-  NewImageLoaderClient* client_;
-  OwnPtr<MojoFetcher> fetcher_;
-  OwnPtr<mojo::common::DataPipeDrainer> drainer_;
-  RefPtr<SharedBuffer> buffer_;
-};
-
-}  // namespace blink
-
-#endif  // SKY_ENGINE_CORE_LOADER_NEWIMAGELOADER_H_

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

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();
 }