Commit aaaef4e3 authored by Greg Thompson's avatar Greg Thompson Committed by Commit Bot

Revert "Drop extensions use of legacy IPC serialization for SkBitmap, use mojo"

This reverts commit 553c38ae.

Reason for revert: extensions::ExtensionActionSetIconFunction::RunExtensionAction crash spike; see https://crbug.com/1087948.

Original change's description:
> Drop extensions use of legacy IPC serialization for SkBitmap, use mojo
>
> This converts to using the skia::mojom::Bitmap serialize functions to
> turn an SkBitmap into a string and back for putting into a base::Value
> and sending over IPC.
>
> TBR=senorblanco
>
> Bug: 1144462
> Change-Id: I10f28bff10aed703e59c500e7f4668f557b51785
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2521203
> Commit-Queue: danakj <danakj@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Reviewed-by: Reilly Grant <reillyg@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#825007}

TBR=danakj@chromium.org,dcheng@chromium.org,reillyg@chromium.org,rdevlin.cronin@chromium.org,senorblanco@chromium.org

Change-Id: Ie49c7fc1c114a4b1c4c97ac2e4f26e9a457b9528
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1144462
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2524480Reviewed-by: default avatarGreg Thompson <grt@chromium.org>
Commit-Queue: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#825185}
parent 18393545
...@@ -12,7 +12,6 @@ specific_include_rules = { ...@@ -12,7 +12,6 @@ specific_include_rules = {
"+chrome/browser/ui/views/frame", "+chrome/browser/ui/views/frame",
"+components/captive_portal", "+components/captive_portal",
"+components/web_package/test_support", "+components/web_package/test_support",
"+skia/public/mojom/bitmap.mojom.h",
], ],
"tls_socket_unittest\.cc": [ "tls_socket_unittest\.cc": [
"+services/network/network_context.h", "+services/network/network_context.h",
......
...@@ -24,11 +24,12 @@ ...@@ -24,11 +24,12 @@
#include "extensions/common/extension.h" #include "extensions/common/extension.h"
#include "extensions/common/extension_builder.h" #include "extensions/common/extension_builder.h"
#include "extensions/common/value_builder.h" #include "extensions/common/value_builder.h"
#include "skia/public/mojom/bitmap.mojom.h" #include "ipc/ipc_message_utils.h"
#include "testing/gmock/include/gmock/gmock.h" #include "testing/gmock/include/gmock/gmock.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkBitmap.h"
#include "ui/gfx/image/image.h" #include "ui/gfx/image/image.h"
#include "ui/gfx/ipc/skia/gfx_skia_param_traits.h"
namespace extensions { namespace extensions {
namespace { namespace {
...@@ -219,8 +220,12 @@ TEST(DeclarativeContentActionTest, SetIcon) { ...@@ -219,8 +220,12 @@ TEST(DeclarativeContentActionTest, SetIcon) {
EXPECT_TRUE(bitmap.tryAllocN32Pixels(19, 19)); EXPECT_TRUE(bitmap.tryAllocN32Pixels(19, 19));
// Fill the bitmap with red pixels. // Fill the bitmap with red pixels.
bitmap.eraseARGB(255, 255, 0, 0); bitmap.eraseARGB(255, 255, 0, 0);
std::string data64 = IPC::Message bitmap_pickle;
base::Base64Encode(skia::mojom::Bitmap::Serialize(&bitmap)); IPC::WriteParam(&bitmap_pickle, bitmap);
std::string binary_data = std::string(
static_cast<const char*>(bitmap_pickle.data()), bitmap_pickle.size());
std::string data64;
base::Base64Encode(binary_data, &data64);
std::unique_ptr<base::DictionaryValue> dict = std::unique_ptr<base::DictionaryValue> dict =
DictionaryBuilder() DictionaryBuilder()
...@@ -278,8 +283,12 @@ TEST(DeclarativeContentActionTest, SetInvisibleIcon) { ...@@ -278,8 +283,12 @@ TEST(DeclarativeContentActionTest, SetInvisibleIcon) {
uint32_t* pixels = bitmap.getAddr32(0, 0); uint32_t* pixels = bitmap.getAddr32(0, 0);
// Set a single pixel, which isn't enough to consider the icon visible. // Set a single pixel, which isn't enough to consider the icon visible.
pixels[0] = SkColorSetARGB(255, 255, 0, 0); pixels[0] = SkColorSetARGB(255, 255, 0, 0);
std::string data64 = IPC::Message bitmap_pickle;
base::Base64Encode(skia::mojom::Bitmap::Serialize(&bitmap)); IPC::WriteParam(&bitmap_pickle, bitmap);
std::string binary_data = std::string(
static_cast<const char*>(bitmap_pickle.data()), bitmap_pickle.size());
std::string data64;
base::Base64Encode(binary_data, &data64);
std::unique_ptr<base::DictionaryValue> dict = std::unique_ptr<base::DictionaryValue> dict =
DictionaryBuilder() DictionaryBuilder()
......
...@@ -18,7 +18,6 @@ include_rules = [ ...@@ -18,7 +18,6 @@ include_rules = [
"+extensions/test", "+extensions/test",
"+mojo/public", "+mojo/public",
"+services/service_manager/public", "+services/service_manager/public",
"+skia/public/mojom",
"+testing", "+testing",
"+third_party/blink/public/common/loader/url_loader_throttle.h", "+third_party/blink/public/common/loader/url_loader_throttle.h",
"+third_party/skia/include", "+third_party/skia/include",
......
...@@ -17,7 +17,8 @@ ...@@ -17,7 +17,8 @@
#include "extensions/common/extension_icon_set.h" #include "extensions/common/extension_icon_set.h"
#include "extensions/common/manifest_handlers/icons_handler.h" #include "extensions/common/manifest_handlers/icons_handler.h"
#include "extensions/grit/extensions_browser_resources.h" #include "extensions/grit/extensions_browser_resources.h"
#include "skia/public/mojom/bitmap.mojom.h" #include "ipc/ipc_message.h"
#include "ipc/ipc_message_utils.h"
#include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkBitmap.h"
#include "third_party/skia/include/core/SkCanvas.h" #include "third_party/skia/include/core/SkCanvas.h"
#include "third_party/skia/include/core/SkPaint.h" #include "third_party/skia/include/core/SkPaint.h"
...@@ -31,6 +32,7 @@ ...@@ -31,6 +32,7 @@
#include "ui/gfx/image/image.h" #include "ui/gfx/image/image.h"
#include "ui/gfx/image/image_skia.h" #include "ui/gfx/image/image_skia.h"
#include "ui/gfx/image/image_skia_source.h" #include "ui/gfx/image/image_skia_source.h"
#include "ui/gfx/ipc/skia/gfx_skia_param_traits.h"
#include "ui/gfx/skbitmap_operations.h" #include "ui/gfx/skbitmap_operations.h"
#include "url/gurl.h" #include "url/gurl.h"
...@@ -121,23 +123,23 @@ ExtensionAction::IconParseResult ExtensionAction::ParseIconFromCanvasDictionary( ...@@ -121,23 +123,23 @@ ExtensionAction::IconParseResult ExtensionAction::ParseIconFromCanvasDictionary(
gfx::ImageSkia* icon) { gfx::ImageSkia* icon) {
for (base::DictionaryValue::Iterator iter(dict); !iter.IsAtEnd(); for (base::DictionaryValue::Iterator iter(dict); !iter.IsAtEnd();
iter.Advance()) { iter.Advance()) {
std::string byte_string; std::string binary_string64;
std::string base64_string; IPC::Message pickle;
const void* bytes = nullptr;
size_t num_bytes = 0;
if (iter.value().is_blob()) { if (iter.value().is_blob()) {
bytes = iter.value().GetBlob().data(); pickle = IPC::Message(
num_bytes = iter.value().GetBlob().size(); reinterpret_cast<const char*>(iter.value().GetBlob().data()),
} else if (iter.value().GetAsString(&base64_string)) { iter.value().GetBlob().size());
if (!base::Base64Decode(base64_string, &byte_string)) } else if (iter.value().GetAsString(&binary_string64)) {
std::string binary_string;
if (!base::Base64Decode(binary_string64, &binary_string))
return IconParseResult::kDecodeFailure; return IconParseResult::kDecodeFailure;
bytes = byte_string.c_str(); pickle = IPC::Message(binary_string.c_str(), binary_string.length());
num_bytes = byte_string.length();
} else { } else {
continue; continue;
} }
base::PickleIterator pickle_iter(pickle);
SkBitmap bitmap; SkBitmap bitmap;
if (!skia::mojom::Bitmap::Deserialize(bytes, num_bytes, &bitmap)) if (!IPC::ReadParam(&pickle, &pickle_iter, &bitmap))
return IconParseResult::kUnpickleFailure; return IconParseResult::kUnpickleFailure;
CHECK(!bitmap.isNull()); CHECK(!bitmap.isNull());
......
...@@ -8,6 +8,8 @@ ...@@ -8,6 +8,8 @@
#include <tuple> #include <tuple>
#include <vector> #include <vector>
#include "ui/gfx/ipc/skia/gfx_skia_param_traits.h"
class SkBitmap; class SkBitmap;
namespace base { namespace base {
......
...@@ -13,9 +13,10 @@ ...@@ -13,9 +13,10 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "content/public/common/common_param_traits.h"
#include "extensions/renderer/script_context.h" #include "extensions/renderer/script_context.h"
#include "gin/data_object_builder.h" #include "gin/data_object_builder.h"
#include "skia/public/mojom/bitmap.mojom.h" #include "ipc/ipc_message_utils.h"
#include "third_party/blink/public/web/web_array_buffer_converter.h" #include "third_party/blink/public/web/web_array_buffer_converter.h"
#include "third_party/skia/include/core/SkBitmap.h" #include "third_party/skia/include/core/SkBitmap.h"
...@@ -143,9 +144,11 @@ bool SetIconNatives::ConvertImageDataToBitmapValue( ...@@ -143,9 +144,11 @@ bool SetIconNatives::ConvertImageDataToBitmapValue(
} }
// Construct the Value object. // Construct the Value object.
std::vector<uint8_t> s = skia::mojom::Bitmap::Serialize(&bitmap); IPC::Message bitmap_pickle;
blink::WebArrayBuffer buffer = blink::WebArrayBuffer::Create(s.size(), 1); IPC::WriteParam(&bitmap_pickle, bitmap);
memcpy(buffer.Data(), s.data(), s.size()); blink::WebArrayBuffer buffer =
blink::WebArrayBuffer::Create(bitmap_pickle.size(), 1);
memcpy(buffer.Data(), bitmap_pickle.data(), bitmap_pickle.size());
*image_data_bitmap = blink::WebArrayBufferConverter::ToV8Value( *image_data_bitmap = blink::WebArrayBufferConverter::ToV8Value(
&buffer, context()->v8_context()->Global(), isolate); &buffer, context()->v8_context()->Global(), isolate);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment