From 8f975399e1d1032ca5a773c75c239e14fa9d9f21 Mon Sep 17 00:00:00 2001 From: Eric Seidel Date: Tue, 9 Jun 2015 12:14:13 -0700 Subject: [PATCH] Move image loading out of C++ into Dart We already know how to talk to the network_service from Dart via fetch.dart. Might as well use that for Image loading as well insetad of having ImageLoader do it. As part of this I've renamed *ImageLoader to *ImageDecoder and moved all the image loading logic into Dart. This required me to teach the idl system about mojo handles so that I could pass the resulting MojoHandle from fetch.dart up through to ImageDecoder. R=abarth@chromium.org, jackson@google.com, hansmuller@google.com Review URL: https://codereview.chromium.org/1173703002. --- engine/bindings/scripts/dart_methods.py | 8 +++-- engine/bindings/scripts/dart_types.py | 7 +++++ engine/core/core.gni | 10 +++---- ...ImageLoader.cpp => CanvasImageDecoder.cpp} | 29 +++++++++---------- ...nvasImageLoader.h => CanvasImageDecoder.h} | 25 +++++----------- .../{ImageLoader.idl => ImageDecoder.idl} | 6 ++-- ...oaderCallback.h => ImageDecoderCallback.h} | 4 +-- ...rCallback.idl => ImageDecoderCallback.idl} | 2 +- engine/tonic/mojo_converter.h | 14 +++++++++ examples/stocks/lib/stock_data.dart | 2 +- examples/stocks2/lib/stock_data.dart | 2 +- sdk/README.md | 2 +- sdk/lib/framework/net/fetch.dart | 26 +++++++++++------ sdk/lib/framework/net/image_cache.dart | 28 ++++++++++-------- 14 files changed, 94 insertions(+), 71 deletions(-) rename engine/core/loader/{CanvasImageLoader.cpp => CanvasImageDecoder.cpp} (60%) rename engine/core/loader/{CanvasImageLoader.h => CanvasImageDecoder.h} (52%) rename engine/core/loader/{ImageLoader.idl => ImageDecoder.idl} (54%) rename engine/core/loader/{ImageLoaderCallback.h => ImageDecoderCallback.h} (87%) rename engine/core/loader/{ImageLoaderCallback.idl => ImageDecoderCallback.idl} (82%) diff --git a/engine/bindings/scripts/dart_methods.py b/engine/bindings/scripts/dart_methods.py index 3ce015278..8fd541569 100644 --- a/engine/bindings/scripts/dart_methods.py +++ b/engine/bindings/scripts/dart_methods.py @@ -154,6 +154,10 @@ def cpp_value(interface, method, number_of_arguments): if idl_type.is_typed_array_type: return '%s.get()' % argument_name + # TODO(eseidel): This should check cpp_type.endswith('Handle') + if idl_type.name == 'MojoDataPipeConsumer': + return '%s.Pass()' % argument_name + if idl_type.name == 'EventListener': if (interface.name == 'EventTarget' and method.name == 'removeEventListener'): @@ -161,9 +165,7 @@ def cpp_value(interface, method, number_of_arguments): # EventTarget::removeEventListener return '%s.get()' % argument_name return argument.name - if (idl_type.is_callback_interface or - idl_type.name in ['NodeFilter', 'XPathNSResolver']): - # FIXME: remove this special case + if idl_type.is_callback_interface: return '%s.release()' % argument_name return argument_name diff --git a/engine/bindings/scripts/dart_types.py b/engine/bindings/scripts/dart_types.py index 85f194261..72558db91 100644 --- a/engine/bindings/scripts/dart_types.py +++ b/engine/bindings/scripts/dart_types.py @@ -116,9 +116,13 @@ CPP_SPECIAL_CONVERSION_RULES = { 'unrestricted float': 'float', # Pass these by value, not pointer. 'Color': 'SkColor', + # These direct conversions appear to be working around + # dart_value_to_cpp_value using CPP_SPECIAL_CONVERSION_RULES directly + # instead of calling cpp_type. 'Float32List': 'Float32List', 'Point': 'Point', 'Rect': 'Rect', + 'MojoDataPipeConsumer': 'mojo::ScopedDataPipeConsumerHandle', 'TransferMode': 'SkXfermode::Mode', 'PaintingStyle': 'SkPaint::Style', } @@ -256,6 +260,7 @@ INCLUDES_FOR_TYPE = { 'NodeList': set(['sky/engine/core/dom/NodeList.h', 'sky/engine/core/dom/StaticNodeList.h']), 'DartValue': set(['sky/engine/tonic/dart_value.h']), + 'MojoDataPipeConsumer': set(['sky/engine/tonic/mojo_converter.h']), } @@ -370,6 +375,7 @@ DART_TO_CPP_VALUE = { 'Rect': pass_by_value_format('Rect'), 'TransferMode': pass_by_value_format('TransferMode'), 'PaintingStyle': pass_by_value_format('PaintingStyle'), + 'MojoDataPipeConsumer': pass_by_value_format('mojo::ScopedDataPipeConsumerHandle'), } def dart_value_to_cpp_value(idl_type, extended_attributes, variable_name, @@ -512,6 +518,7 @@ IDL_TO_DART_TYPE = { 'boolean': 'bool', 'void': 'void', 'unsigned long': 'int', + 'MojoDataPipeConsumer': 'int', } def idl_type_to_dart_type(idl_type): diff --git a/engine/core/core.gni b/engine/core/core.gni index e135e3ad6..80167a8b3 100644 --- a/engine/core/core.gni +++ b/engine/core/core.gni @@ -798,8 +798,8 @@ sky_core_files = [ "inspector/ScriptAsyncCallStack.h", "inspector/ScriptGCEventListener.h", "layout/LayoutCallback.h", - "loader/CanvasImageLoader.cpp", - "loader/CanvasImageLoader.h", + "loader/CanvasImageDecoder.cpp", + "loader/CanvasImageDecoder.h", "loader/DocumentLoadTiming.cpp", "loader/DocumentLoadTiming.h", "loader/EmptyClients.cpp", @@ -812,7 +812,7 @@ sky_core_files = [ "loader/FrameLoaderTypes.h", "loader/ImageLoader.cpp", "loader/ImageLoader.h", - "loader/ImageLoaderCallback.h", + "loader/ImageDecoderCallback.h", "loader/MojoLoader.cpp", "loader/MojoLoader.h", "loader/NavigationPolicy.h", @@ -1142,8 +1142,8 @@ core_idl_files = get_path_info([ "html/TextMetrics.idl", "html/VoidCallback.idl", "layout/LayoutCallback.idl", - "loader/ImageLoader.idl", - "loader/ImageLoaderCallback.idl", + "loader/ImageDecoder.idl", + "loader/ImageDecoderCallback.idl", "painting/Canvas.idl", "painting/ColorFilter.idl", "painting/DrawLooper.idl", diff --git a/engine/core/loader/CanvasImageLoader.cpp b/engine/core/loader/CanvasImageDecoder.cpp similarity index 60% rename from engine/core/loader/CanvasImageLoader.cpp rename to engine/core/loader/CanvasImageDecoder.cpp index a7175dc50..3776f95a9 100644 --- a/engine/core/loader/CanvasImageLoader.cpp +++ b/engine/core/loader/CanvasImageDecoder.cpp @@ -3,7 +3,7 @@ // found in the LICENSE file. #include "sky/engine/config.h" -#include "sky/engine/core/loader/CanvasImageLoader.h" +#include "sky/engine/core/loader/CanvasImageDecoder.h" #include "sky/engine/core/painting/CanvasImage.h" #include "sky/engine/core/script/dom_dart_state.h" #include "sky/engine/platform/SharedBuffer.h" @@ -11,30 +11,27 @@ namespace blink { -CanvasImageLoader::CanvasImageLoader(const String& src, PassOwnPtr callback) - : callback_(callback) { - KURL url = KURL(DOMDartState::Current()->url(), src); - fetcher_ = adoptPtr(new MojoFetcher(this, url)); +PassRefPtr CanvasImageDecoder::create( + mojo::ScopedDataPipeConsumerHandle handle, + PassOwnPtr callback) +{ + return adoptRef(new CanvasImageDecoder(handle.Pass(), callback)); } -CanvasImageLoader::~CanvasImageLoader() { +CanvasImageDecoder::CanvasImageDecoder(mojo::ScopedDataPipeConsumerHandle handle, PassOwnPtr callback) + : callback_(callback) { + buffer_ = SharedBuffer::create(); + drainer_ = adoptPtr(new mojo::common::DataPipeDrainer(this, handle.Pass())); } -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())); +CanvasImageDecoder::~CanvasImageDecoder() { } -void CanvasImageLoader::OnDataAvailable(const void* data, size_t num_bytes) { +void CanvasImageDecoder::OnDataAvailable(const void* data, size_t num_bytes) { buffer_->append(static_cast(data), num_bytes); } -void CanvasImageLoader::OnDataComplete() { +void CanvasImageDecoder::OnDataComplete() { OwnPtr decoder = ImageDecoder::create(*buffer_.get(), ImageSource::AlphaPremultiplied, ImageSource::GammaAndColorProfileIgnored); diff --git a/engine/core/loader/CanvasImageLoader.h b/engine/core/loader/CanvasImageDecoder.h similarity index 52% rename from engine/core/loader/CanvasImageLoader.h rename to engine/core/loader/CanvasImageDecoder.h index a8f255b03..f0686ae9c 100644 --- a/engine/core/loader/CanvasImageLoader.h +++ b/engine/core/loader/CanvasImageDecoder.h @@ -7,41 +7,32 @@ #define SKY_ENGINE_CORE_LOADER_CANVASIMAGELOADER_H_ #include "mojo/common/data_pipe_drainer.h" -#include "sky/engine/core/loader/ImageLoaderCallback.h" +#include "sky/engine/core/loader/ImageDecoderCallback.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, - public DartWrappable { +class CanvasImageDecoder : public mojo::common::DataPipeDrainer::Client, + public RefCounted, + public DartWrappable { DEFINE_WRAPPERTYPEINFO(); public: - static PassRefPtr create(const String& src, PassOwnPtr callback) - { - return adoptRef(new CanvasImageLoader(src, callback)); - } - virtual ~CanvasImageLoader(); - - // MojoFetcher::Client - void OnReceivedResponse(mojo::URLResponsePtr) override; + static PassRefPtr create(mojo::ScopedDataPipeConsumerHandle handle, PassOwnPtr callback); + virtual ~CanvasImageDecoder(); // mojo::common::DataPipeDrainer::Client void OnDataAvailable(const void*, size_t) override; void OnDataComplete() override; private: - explicit CanvasImageLoader(const String& src, PassOwnPtr callback); + CanvasImageDecoder(mojo::ScopedDataPipeConsumerHandle handle, PassOwnPtr callback); - OwnPtr fetcher_; OwnPtr drainer_; RefPtr buffer_; - OwnPtr callback_; + OwnPtr callback_; }; } // namespace blink diff --git a/engine/core/loader/ImageLoader.idl b/engine/core/loader/ImageDecoder.idl similarity index 54% rename from engine/core/loader/ImageLoader.idl rename to engine/core/loader/ImageDecoder.idl index 3bf59d3bd..f68b4aba3 100644 --- a/engine/core/loader/ImageLoader.idl +++ b/engine/core/loader/ImageDecoder.idl @@ -3,7 +3,7 @@ // found in the LICENSE file. [ - Constructor(DOMString src, ImageLoaderCallback callback), - ImplementedAs=CanvasImageLoader, -] interface ImageLoader { + Constructor(MojoDataPipeConsumer consumer, ImageDecoderCallback callback), + ImplementedAs=CanvasImageDecoder, +] interface ImageDecoder { }; diff --git a/engine/core/loader/ImageLoaderCallback.h b/engine/core/loader/ImageDecoderCallback.h similarity index 87% rename from engine/core/loader/ImageLoaderCallback.h rename to engine/core/loader/ImageDecoderCallback.h index 036c20610..fab1a2fba 100644 --- a/engine/core/loader/ImageLoaderCallback.h +++ b/engine/core/loader/ImageDecoderCallback.h @@ -9,9 +9,9 @@ namespace blink { -class ImageLoaderCallback { +class ImageDecoderCallback { public: - virtual ~ImageLoaderCallback() {} + virtual ~ImageDecoderCallback() {} virtual void handleEvent(CanvasImage* result) = 0; }; diff --git a/engine/core/loader/ImageLoaderCallback.idl b/engine/core/loader/ImageDecoderCallback.idl similarity index 82% rename from engine/core/loader/ImageLoaderCallback.idl rename to engine/core/loader/ImageDecoderCallback.idl index a8463add7..e5f643dfa 100644 --- a/engine/core/loader/ImageLoaderCallback.idl +++ b/engine/core/loader/ImageDecoderCallback.idl @@ -2,6 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -callback interface ImageLoaderCallback { +callback interface ImageDecoderCallback { void handleEvent(Image result); }; diff --git a/engine/tonic/mojo_converter.h b/engine/tonic/mojo_converter.h index cfdd4b016..24705fae4 100644 --- a/engine/tonic/mojo_converter.h +++ b/engine/tonic/mojo_converter.h @@ -25,6 +25,20 @@ struct DartConverter> { static Dart_Handle ToDart(mojo::ScopedHandleBase mojo_handle) { return Dart_NewInteger(static_cast(mojo_handle.release().value())); } + + static mojo::ScopedHandleBase FromArgumentsWithNullCheck( + Dart_NativeArguments args, + int index, + Dart_Handle& exception, + bool auto_scope = true) { + int64_t mojo_handle64 = 0; + Dart_Handle result = Dart_GetNativeIntegerArgument(args, index, &mojo_handle64); + if (Dart_IsError(result) || !mojo_handle64) + return mojo::ScopedHandleBase(); + + HandleType mojo_handle(static_cast(mojo_handle64)); + return mojo::MakeScopedHandle(mojo_handle); + } }; } // namespace blink diff --git a/examples/stocks/lib/stock_data.dart b/examples/stocks/lib/stock_data.dart index 9bca149d2..39201714f 100644 --- a/examples/stocks/lib/stock_data.dart +++ b/examples/stocks/lib/stock_data.dart @@ -59,7 +59,7 @@ class StockDataFetcher { } void _fetchNextChunk() { - fetch('data/stock_data_${_currentChunk++}.json').then((Response response) { + fetchBody('data/stock_data_${_currentChunk++}.json').then((Response response) { String json = response.bodyAsString(); JsonDecoder decoder = new JsonDecoder(); diff --git a/examples/stocks2/lib/stock_data.dart b/examples/stocks2/lib/stock_data.dart index d69f44275..a77ca50a6 100644 --- a/examples/stocks2/lib/stock_data.dart +++ b/examples/stocks2/lib/stock_data.dart @@ -59,7 +59,7 @@ class StockDataFetcher { } void _fetchNextChunk() { - fetch('../data/stock_data_${_currentChunk++}.json').then((Response response) { + fetchBody('../data/stock_data_${_currentChunk++}.json').then((Response response) { String json = response.bodyAsString(); JsonDecoder decoder = new JsonDecoder(); diff --git a/sdk/README.md b/sdk/README.md index 987807a8d..39bacd510 100644 --- a/sdk/README.md +++ b/sdk/README.md @@ -109,7 +109,7 @@ ergonomic interface: import 'package:sky/framework/net/fetch.dart'; main() async { - Response response = await fetch('example.txt'); + Response response = await fetchBody('example.txt'); print(response.bodyAsString()); } ``` diff --git a/sdk/lib/framework/net/fetch.dart b/sdk/lib/framework/net/fetch.dart index 720ac4cbc..5872af30d 100644 --- a/sdk/lib/framework/net/fetch.dart +++ b/sdk/lib/framework/net/fetch.dart @@ -9,6 +9,7 @@ import 'package:mojo/core.dart' as core; import 'package:mojom/mojo/network_service.mojom.dart'; import 'package:mojom/mojo/url_loader.mojom.dart'; import 'package:mojom/mojo/url_request.mojom.dart'; +import 'package:mojom/mojo/url_response.mojom.dart'; class Response { ByteData body; @@ -20,23 +21,30 @@ class Response { } } -Future fetch(String relativeUrl) async { - String url = Uri.base.resolve(relativeUrl).toString(); - - var net = new NetworkServiceProxy.unbound(); +Future fetch(UrlRequest request) async { + NetworkServiceProxy net = new NetworkServiceProxy.unbound(); shell.requestService("mojo:authenticated_network_service", net); - var loader = new UrlLoaderProxy.unbound(); + UrlLoaderProxy loader = new UrlLoaderProxy.unbound(); net.ptr.createUrlLoader(loader); - var request = new UrlRequest() - ..url = url - ..autoFollowRedirects = true; - var response = (await loader.ptr.start(request)).response; + UrlResponse response = (await loader.ptr.start(request)).response; loader.close(); net.close(); + return response; +} + +Future fetchUrl(String relativeUrl) async { + String url = Uri.base.resolve(relativeUrl).toString(); + UrlRequest request = new UrlRequest() + ..url = url + ..autoFollowRedirects = true; + return fetch(request); +} +Future fetchBody(String relativeUrl) async { + UrlResponse response = await fetchMojo(relativeUrl); if (response.body == null) return new Response(null); ByteData data = await core.DataPipeDrainer.drainHandle(response.body); diff --git a/sdk/lib/framework/net/image_cache.dart b/sdk/lib/framework/net/image_cache.dart index dcb2ac2e9..ccd69996e 100644 --- a/sdk/lib/framework/net/image_cache.dart +++ b/sdk/lib/framework/net/image_cache.dart @@ -2,17 +2,19 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -import 'dart:sky' as sky; +import 'dart:sky'; import 'dart:collection'; +import 'fetch.dart'; +import 'package:mojom/mojo/url_response.mojom.dart'; -final HashMap> _pendingRequests = - new HashMap>(); +final HashMap> _pendingRequests = + new HashMap>(); -final HashMap _completedRequests = - new HashMap(); +final HashMap _completedRequests = + new HashMap(); -void load(String url, sky.ImageLoaderCallback callback) { - sky.Image result = _completedRequests[url]; +void load(String url, ImageDecoderCallback callback) { + Image result = _completedRequests[url]; if (result != null) { callback(_completedRequests[url]); return; @@ -21,13 +23,15 @@ void load(String url, sky.ImageLoaderCallback callback) { bool newRequest = false; _pendingRequests.putIfAbsent(url, () { newRequest = true; - return new List(); + return new List(); }).add(callback); if (newRequest) { - new sky.ImageLoader(url, (image) { - _completedRequests[url] = image; - _pendingRequests[url].forEach((c) => c(image)); - _pendingRequests.remove(url); + fetchUrl(url).then((UrlResponse response) { + new ImageDecoder(response.body.handle.h, (image) { + _completedRequests[url] = image; + _pendingRequests[url].forEach((c) => c(image)); + _pendingRequests.remove(url); + }); }); } } -- GitLab