Commit f2d2b644 authored by danakj's avatar danakj Committed by Chromium LUCI CQ

Use skia.mojom.BitmapN32 in FrameWidget mojoms

These bitmaps are safer for transport from untrustworthy sources since
all bitmaps should be in N32 format and the browser can make bad
assumptions as a result.

Bug: 1144462
Change-Id: I9c4fde80a4124753c11845cbcc202cce2c3d31a4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2582928Reviewed-by: default avatarDaniel Cheng <dcheng@chromium.org>
Reviewed-by: default avatarAvi Drissman <avi@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#835684}
parent 25a6f2f8
...@@ -104,7 +104,6 @@ class MockDraggingRenderViewHostDelegateView ...@@ -104,7 +104,6 @@ class MockDraggingRenderViewHostDelegateView
RenderWidgetHostImpl* source_rwh) override { RenderWidgetHostImpl* source_rwh) override {
drag_url_ = drop_data.url; drag_url_ = drop_data.url;
html_base_url_ = drop_data.html_base_url; html_base_url_ = drop_data.html_base_url;
image_ = image;
} }
GURL drag_url() { GURL drag_url() {
...@@ -115,12 +114,9 @@ class MockDraggingRenderViewHostDelegateView ...@@ -115,12 +114,9 @@ class MockDraggingRenderViewHostDelegateView
return html_base_url_; return html_base_url_;
} }
const gfx::ImageSkia& image() { return image_; }
private: private:
GURL drag_url_; GURL drag_url_;
GURL html_base_url_; GURL html_base_url_;
gfx::ImageSkia image_;
}; };
TEST_F(RenderViewHostTest, StartDragging) { TEST_F(RenderViewHostTest, StartDragging) {
...@@ -163,37 +159,6 @@ TEST_F(RenderViewHostTest, StartDragging) { ...@@ -163,37 +159,6 @@ TEST_F(RenderViewHostTest, StartDragging) {
EXPECT_EQ(http_url, delegate_view.html_base_url()); EXPECT_EQ(http_url, delegate_view.html_base_url());
} }
TEST_F(RenderViewHostTest, StartDraggingWithInvalidBitmap) {
TestWebContents* web_contents = contents();
MockDraggingRenderViewHostDelegateView delegate_view;
web_contents->set_delegate_view(&delegate_view);
GURL http_url = GURL("http://www.domain.com/index.html");
DropData drop_data;
// If `html` is not populated, `html_base_url` won't be populated when
// converting to `DragData` with `DropDataToDragData`.
drop_data.html = base::string16();
drop_data.url = http_url;
drop_data.html_base_url = http_url;
SkBitmap badbitmap;
badbitmap.allocPixels(
SkImageInfo::Make(1, 1, kARGB_4444_SkColorType, kPremul_SkAlphaType));
badbitmap.eraseColor(SK_ColorGREEN);
SkBitmap n32bitmap;
EXPECT_TRUE(skia::SkBitmapToN32OpaqueOrPremul(badbitmap, &n32bitmap));
// An N32 bitmap is a valid drag image.
test_rvh()->TestStartDragging(drop_data, n32bitmap);
EXPECT_TRUE(gfx::BitmapsAreEqual(n32bitmap, *delegate_view.image().bitmap()));
// Other bitmap types are not, and are converted.
test_rvh()->TestStartDragging(drop_data, badbitmap);
EXPECT_TRUE(gfx::BitmapsAreEqual(n32bitmap, *delegate_view.image().bitmap()));
}
TEST_F(RenderViewHostTest, DragEnteredFileURLsStillBlocked) { TEST_F(RenderViewHostTest, DragEnteredFileURLsStillBlocked) {
DropData dropped_data; DropData dropped_data;
gfx::PointF client_point; gfx::PointF client_point;
......
...@@ -2549,7 +2549,7 @@ void RenderWidgetHostImpl::DidFirstVisuallyNonEmptyPaint() { ...@@ -2549,7 +2549,7 @@ void RenderWidgetHostImpl::DidFirstVisuallyNonEmptyPaint() {
void RenderWidgetHostImpl::StartDragging( void RenderWidgetHostImpl::StartDragging(
blink::mojom::DragDataPtr drag_data, blink::mojom::DragDataPtr drag_data,
blink::DragOperationsMask drag_operations_mask, blink::DragOperationsMask drag_operations_mask,
const SkBitmap& unsafe_bitmap, const SkBitmap& bitmap,
const gfx::Vector2d& bitmap_offset_in_dip, const gfx::Vector2d& bitmap_offset_in_dip,
blink::mojom::DragEventSourceInfoPtr event_info) { blink::mojom::DragEventSourceInfoPtr event_info) {
RenderViewHostDelegateView* view = delegate_->GetDelegateView(); RenderViewHostDelegateView* view = delegate_->GetDelegateView();
...@@ -2558,16 +2558,6 @@ void RenderWidgetHostImpl::StartDragging( ...@@ -2558,16 +2558,6 @@ void RenderWidgetHostImpl::StartDragging(
DragSourceSystemDragEnded(); DragSourceSystemDragEnded();
return; return;
} }
SkBitmap bitmap;
// On receipt of an arbitrary bitmap from the renderer, we convert to an N32
// 32bpp bitmap. Other pixel sizes can lead to out-of-bounds mistakes when
// transferring the pixels out of the/ bitmap into other buffers.
if (!skia::SkBitmapToN32OpaqueOrPremul(unsafe_bitmap, &bitmap)) {
NOTREACHED() << "Unable to convert bitmap for drag-and-drop";
// Need to clear drag and drop state in blink.
DragSourceSystemDragEnded();
return;
}
DropData drop_data = DragDataToDropData(*drag_data); DropData drop_data = DragDataToDropData(*drag_data);
DropData filtered_data(drop_data); DropData filtered_data(drop_data);
......
...@@ -190,7 +190,7 @@ interface FrameWidgetHost { ...@@ -190,7 +190,7 @@ interface FrameWidgetHost {
// session at the OS level. // session at the OS level.
StartDragging(DragData drag_data, StartDragging(DragData drag_data,
AllowedDragOperations operations_allowed, AllowedDragOperations operations_allowed,
skia.mojom.BitmapWithArbitraryBpp? image, skia.mojom.BitmapN32? image,
gfx.mojom.Vector2d bitmap_offset_in_dip, gfx.mojom.Vector2d bitmap_offset_in_dip,
DragEventSourceInfo event_info); DragEventSourceInfo event_info);
}; };
......
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