Skip to content

Commit

Permalink
Add SkBitmap StructTraits for skia::mojo::Bitmap.
Browse files Browse the repository at this point in the history
Prerequisite (as per dcheng's request) for new mojo Bitmap use in:
  https://codereview.chromium.org/2014013002

Add StructTraits to convert between SkBitmap and skia::mojo::Bitmap.
(add profile enum conversion functions, BitmapBuffer utility struct)

Update skia::mojo::BitmapPtr users throughout codebase.
Remove the now unused TypeConverter for these types.

Add public_deps as needed for skia header usage/mapping.
Propagate public_deps necessitated by typemap implementations.

BUG=557405
TEST=No regressions for existing mojo bitmap users.
R=yzshen@chromium.org,tsepez@chromium.org,reed@chromium.org,sky@chromium.org,avi@chromium.org
TBR=ben@chromium.org

Review-Url: https://codereview.chromium.org/2014013002
Cr-Commit-Position: refs/heads/master@{#397263}
  • Loading branch information
msw authored and Commit bot committed Jun 1, 2016
1 parent 14569c8 commit b0123d1
Show file tree
Hide file tree
Showing 32 changed files with 387 additions and 261 deletions.
6 changes: 4 additions & 2 deletions ash/sysui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,10 @@ source_set("lib") {
"//components/mus/public/interfaces",
"//components/user_manager",
"//device/bluetooth",
"//mash/shelf/public/interfaces",
"//mojo/common:common_base",
"//services/catalog/public/cpp",
"//services/shell/public/cpp",
"//services/tracing/public/cpp",
"//skia/public",
"//ui/app_list/presenter",
"//ui/app_list/presenter:mojom",
"//ui/aura",
Expand All @@ -64,6 +62,10 @@ source_set("lib") {
"//ui/views/mus:for_mojo_application",
]

public_deps = [
"//mash/shelf/public/interfaces",
]

if (is_chromeos) {
deps += [ "//chromeos" ]
}
Expand Down
1 change: 0 additions & 1 deletion ash/sysui/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,5 @@ include_rules = [
"+services/catalog/public",
"+services/shell/public",
"+services/tracing/public",
"+skia/public",
"+third_party/khronos/GLES2/gl2.h",
]
7 changes: 3 additions & 4 deletions ash/sysui/shelf_delegate_mus.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
#include "components/mus/public/cpp/window_property.h"
#include "mojo/common/common_type_converters.h"
#include "services/shell/public/cpp/connector.h"
#include "skia/public/type_converters.h"
#include "ui/aura/mus/mus_util.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/gfx/image/image_skia.h"
Expand Down Expand Up @@ -314,7 +313,7 @@ void ShelfDelegateMus::PinItem(
ShelfItem shelf_item;
shelf_item.type = TYPE_APP_SHORTCUT;
shelf_item.status = STATUS_CLOSED;
shelf_item.image = GetShelfIconFromBitmap(item->image.To<SkBitmap>());
shelf_item.image = GetShelfIconFromBitmap(item->image);
model_->Add(shelf_item);

std::unique_ptr<ShelfItemDelegateMus> item_delegate(
Expand All @@ -341,14 +340,14 @@ void ShelfDelegateMus::UnpinItem(const mojo::String& app_id) {
}

void ShelfDelegateMus::SetItemImage(const mojo::String& app_id,
skia::mojom::BitmapPtr image) {
const SkBitmap& image) {
if (!app_id_to_shelf_id_.count(app_id.To<std::string>()))
return;
ShelfID shelf_id = app_id_to_shelf_id_[app_id.To<std::string>()];
int index = model_->ItemIndexByID(shelf_id);
DCHECK_GE(index, 0);
ShelfItem item = *model_->ItemByID(shelf_id);
item.image = GetShelfIconFromBitmap(image.To<SkBitmap>());
item.image = GetShelfIconFromBitmap(image);
model_->Set(index, item);
}

Expand Down
3 changes: 1 addition & 2 deletions ash/sysui/shelf_delegate_mus.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,7 @@ class ShelfDelegateMus : public ShelfDelegate,
mash::shelf::mojom::ShelfItemPtr item,
mash::shelf::mojom::ShelfItemDelegateAssociatedPtrInfo delegate) override;
void UnpinItem(const mojo::String& app_id) override;
void SetItemImage(const mojo::String& app_id,
skia::mojom::BitmapPtr image) override;
void SetItemImage(const mojo::String& app_id, const SkBitmap& image) override;

// mojom::UserWindowObserver:
void OnUserWindowObserverAdded(
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,6 @@ source_set("browser") {
"//net:extras",
"//net:net_with_v8",
"//services/shell/public/cpp",
"//skia/public",
"//storage/browser",
"//storage/common",
"//third_party/WebKit/public:image_resources",
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ include_rules = [
"+rlz",
"+sandbox/win/src", # The path doesn't say it, but this is the Windows sandbox.
"+skia/ext",
"+skia/public",
"+sync/api", # Sync API files.
"+sync/internal_api/public/attachments", # Needed for tests.
"+sync/internal_api/public/base",
Expand Down
16 changes: 6 additions & 10 deletions chrome/browser/image_decoder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/utility_process_host.h"
#include "content/public/common/service_registry.h"
#include "skia/public/type_converters.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/base/l10n/l10n_util.h"

Expand All @@ -32,16 +31,13 @@ const int kBatchModeTimeoutSeconds = 5;
void OnDecodeImageDone(
base::Callback<void(int)> fail_callback,
base::Callback<void(const SkBitmap&, int)> success_callback,
int request_id, skia::mojom::BitmapPtr image) {
int request_id,
const SkBitmap& image) {
DCHECK_CURRENTLY_ON(BrowserThread::IO);
if (image) {
SkBitmap bitmap = image.To<SkBitmap>();
if (!bitmap.empty()) {
success_callback.Run(bitmap, request_id);
return;
}
}
fail_callback.Run(request_id);
if (!image.isNull() && !image.empty())
success_callback.Run(image, request_id);
else
fail_callback.Run(request_id);
}

} // namespace
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,6 @@ source_set("ui") {
"//ash/strings",
"//components/user_manager",
"//mash/shelf/public/interfaces",
"//skia/public",
"//ui/app_list/presenter",
"//ui/app_list/presenter:mojom",
"//ui/keyboard:mojom",
Expand Down
1 change: 0 additions & 1 deletion chrome/browser/ui/ash/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,4 @@ include_rules = [
"+components/user_manager",
"+mash/shelf/public/interfaces",
"+media",
"+skia/public",
]
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "extensions/grit/extensions_browser_resources.h"
#include "mojo/common/common_type_converters.h"
#include "services/shell/public/cpp/connector.h"
#include "skia/public/type_converters.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/display/display.h"
#include "ui/display/screen.h"
Expand Down Expand Up @@ -125,7 +124,7 @@ void ChromeMashShelfController::PinAppsFromPrefs() {
item->app_title = mojo::String::From(helper_.GetAppTitle(profile, app));
ResourceBundle& rb = ResourceBundle::GetSharedInstance();
const gfx::Image& image = rb.GetImageNamed(IDR_APP_DEFAULT_ICON);
item->image = skia::mojom::Bitmap::From(*image.ToSkBitmap());
item->image = *image.ToSkBitmap();
std::unique_ptr<ChromeShelfItemDelegate> delegate(
new ChromeShelfItemDelegate(app));
shelf_controller_->PinItem(std::move(item),
Expand Down Expand Up @@ -169,6 +168,5 @@ void ChromeMashShelfController::OnAutoHideBehaviorChanged(

void ChromeMashShelfController::OnAppImageUpdated(const std::string& app_id,
const gfx::ImageSkia& image) {
shelf_controller_->SetItemImage(app_id,
skia::mojom::Bitmap::From(*image.bitmap()));
shelf_controller_->SetItemImage(app_id, *image.bitmap());
}
31 changes: 23 additions & 8 deletions chrome/chrome_common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -649,20 +649,35 @@
},
'includes': [ '../build/protoc.gypi' ],
},
{
'target_name': 'common_mojo_bindings_mojom',
'type': 'none',
'variables': {
'mojom_files': [
'common/image_decoder.mojom',
'common/resource_usage_reporter.mojom',
],
'mojom_typemaps': [
'../skia/public/interfaces/skbitmap.typemap',
],
},
'dependencies': [
'../mojo/mojo_public.gyp:mojo_cpp_bindings',
'../skia/skia.gyp:skia_mojo',
],
'includes': [ '../mojo/mojom_bindings_generator_explicit.gypi' ],
},
{
# GN version: //chrome/common:mojo_bindings
'target_name': 'common_mojo_bindings',
'type': 'static_library',
'includes': [
'../mojo/mojom_bindings_generator.gypi'
],
'sources': [
'common/image_decoder.mojom',
'common/resource_usage_reporter.mojom',
],
'dependencies': [
'common_mojo_bindings_mojom',
'../mojo/mojo_public.gyp:mojo_cpp_bindings',
'../skia/skia.gyp:skia_mojo',
'../skia/skia.gyp:skia',
],
'export_dependent_settings': [
'../skia/skia.gyp:skia',
],
},
],
Expand Down
1 change: 0 additions & 1 deletion chrome/utility/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ static_library("utility") {
"//media",
"//net:net_with_v8",
"//skia",
"//skia/public",
"//sql",
"//third_party/libxml",
]
Expand Down
1 change: 0 additions & 1 deletion chrome/utility/DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ include_rules = [
"+extensions/common",
"+media",
"+skia/ext",
"+skia/public",
"+third_party/libxml",
"+third_party/zlib/google",
]
10 changes: 3 additions & 7 deletions chrome/utility/image_decoder_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "content/public/child/image_decoder_utils.h"
#include "ipc/ipc_channel.h"
#include "skia/ext/image_operations.h"
#include "skia/public/type_converters.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/geometry/size.h"

Expand Down Expand Up @@ -42,9 +41,9 @@ void ImageDecoderImpl::DecodeImage(
mojo::Array<uint8_t> encoded_data,
mojom::ImageCodec codec,
bool shrink_to_fit,
const mojo::Callback<void(skia::mojom::BitmapPtr)>& callback) {
const mojo::Callback<void(const SkBitmap&)>& callback) {
if (encoded_data.size() == 0) {
callback.Run(nullptr);
callback.Run(SkBitmap());
return;
}

Expand Down Expand Up @@ -99,8 +98,5 @@ void ImageDecoderImpl::DecodeImage(
}
}

if (decoded_image.isNull())
callback.Run(nullptr);
else
callback.Run(skia::mojom::Bitmap::From(decoded_image));
callback.Run(decoded_image);
}
2 changes: 1 addition & 1 deletion chrome/utility/image_decoder_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class ImageDecoderImpl : public mojom::ImageDecoder {
mojo::Array<uint8_t> encoded_data,
mojom::ImageCodec codec,
bool shrink_to_fit,
const mojo::Callback<void(skia::mojom::BitmapPtr)>& callback) override;
const mojo::Callback<void(const SkBitmap&)>& callback) override;

private:
int64_t max_message_size_;
Expand Down
22 changes: 9 additions & 13 deletions chrome/utility/image_decoder_impl_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

#include "base/bind.h"
#include "ipc/ipc_channel.h"
#include "skia/public/type_converters.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/codec/jpeg_codec.h"
Expand Down Expand Up @@ -46,15 +45,13 @@ class Request {
shrink, base::Bind(&Request::OnRequestDone, base::Unretained(this)));
}

const skia::mojom::BitmapPtr& bitmap() const { return bitmap_; }
const SkBitmap& bitmap() const { return bitmap_; }

private:
void OnRequestDone(skia::mojom::BitmapPtr result_image) {
bitmap_ = std::move(result_image);
}
void OnRequestDone(const SkBitmap& result_image) { bitmap_ = result_image; }

ImageDecoderImpl* decoder_;
skia::mojom::BitmapPtr bitmap_;
SkBitmap bitmap_;
};

} // namespace
Expand Down Expand Up @@ -83,24 +80,23 @@ TEST(ImageDecoderImplTest, DecodeImageSizeLimit) {

Request request(&decoder);
request.DecodeImage(jpg, true);
ASSERT_FALSE(request.bitmap().is_null());
SkBitmap bitmap = request.bitmap().To<SkBitmap>();
ASSERT_FALSE(request.bitmap().isNull());

// Check that image has been shrunk appropriately
EXPECT_LT(bitmap.computeSize64() + base_msg_size,
EXPECT_LT(request.bitmap().computeSize64() + base_msg_size,
static_cast<int64_t>(kTestMessageSize));
// Android does its own image shrinking for memory conservation deeper in
// the decode, so more specific tests here won't work.
#if !defined(OS_ANDROID)
EXPECT_EQ(widths[i] >> i, bitmap.width());
EXPECT_EQ(heights[i] >> i, bitmap.height());
EXPECT_EQ(widths[i] >> i, request.bitmap().width());
EXPECT_EQ(heights[i] >> i, request.bitmap().height());

// Check that if resize not requested and image exceeds IPC size limit,
// an empty image is returned
if (heights[i] > max_height_for_msg) {
Request request(&decoder);
request.DecodeImage(jpg, false);
EXPECT_TRUE(request.bitmap().is_null());
EXPECT_TRUE(request.bitmap().isNull());
}
#endif
}
Expand All @@ -116,7 +112,7 @@ TEST(ImageDecoderImplTest, DecodeImageFailed) {

Request request(&decoder);
request.DecodeImage(jpg, false);
EXPECT_TRUE(request.bitmap().is_null());
EXPECT_TRUE(request.bitmap().isNull());
}

} // namespace mojom
1 change: 0 additions & 1 deletion content/browser/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ source_set("browser") {
"//services/user/public/cpp",
"//services/user/public/interfaces",
"//skia",
"//skia/public",
"//sql",
"//storage/browser",
"//storage/common",
Expand Down
3 changes: 1 addition & 2 deletions content/browser/web_contents/web_contents_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@
#include "net/http/http_transaction_factory.h"
#include "net/url_request/url_request_context.h"
#include "net/url_request/url_request_context_getter.h"
#include "skia/public/type_converters.h"
#include "third_party/WebKit/public/web/WebSandboxFlags.h"
#include "third_party/skia/include/core/SkBitmap.h"
#include "ui/accessibility/ax_tree_combiner.h"
Expand Down Expand Up @@ -4846,7 +4845,7 @@ void WebContentsImpl::OnDidDownloadImage(
int id,
const GURL& image_url,
int32_t http_status_code,
mojo::Array<skia::mojom::BitmapPtr> images,
mojo::Array<SkBitmap> images,
mojo::Array<gfx::Size> original_image_sizes) {
const std::vector<SkBitmap> bitmaps = images.To<std::vector<SkBitmap>>();

Expand Down
2 changes: 1 addition & 1 deletion content/browser/web_contents/web_contents_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,7 @@ class CONTENT_EXPORT WebContentsImpl
int id,
const GURL& image_url,
int32_t http_status_code,
mojo::Array<skia::mojom::BitmapPtr> images,
mojo::Array<SkBitmap> images,
mojo::Array<gfx::Size> original_image_sizes);

// Callback function when showing JavaScript dialogs. Takes in a routing ID
Expand Down
3 changes: 3 additions & 0 deletions content/content_common_mojo_bindings.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
'common/storage_partition_service.mojom',
],
'mojom_typemaps': [
'../skia/public/interfaces/skbitmap.typemap',
'../ui/gfx/geometry/mojo/geometry.typemap',
'../url/mojo/gurl.typemap',
'../url/mojo/origin.typemap',
Expand All @@ -43,9 +44,11 @@
},
'dependencies': [
'../url/url.gyp:url_mojom',
'../skia/skia.gyp:skia',
'content_common_mojo_bindings_mojom',
],
'export_dependent_settings': [
'../skia/skia.gyp:skia',
'../url/url.gyp:url_mojom',
],
},
Expand Down
1 change: 0 additions & 1 deletion content/renderer/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ source_set("renderer") {
"//services/shell/public/cpp",
"//services/shell/public/interfaces",
"//skia",
"//skia/public",
"//storage/common",
"//third_party/WebKit/public:blink",
"//third_party/WebKit/public:mojo_bindings",
Expand Down
Loading

0 comments on commit b0123d1

Please sign in to comment.