From cc838d48f43ee03e8b88b36de412e54c1f8cbfd1 Mon Sep 17 00:00:00 2001 From: danakj Date: Wed, 2 Oct 2019 23:18:32 +0000 Subject: [PATCH] Clarify emulator computation and don't rely on emulator state for it This splits apart the computation of each emulated field and makes more clear the intended outcome for desktop vs mobile cases. We remove the applied_widget_rect_ state, which can be computed from other inputs, and the scale_ state which is already stored in the emulation_params_. When the scale is not used, it used to store it as 0 and multiply the context menu coordinates by 0. Now we store a 1 when the scale is not used. R=avi@chromium.org TBR=caseq Bug: 1006052 Change-Id: I9c9aabca73d7d73f946e5889444004e00e181bec Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1834080 Commit-Queue: danakj Reviewed-by: Daniel Cheng Reviewed-by: Avi Drissman Cr-Commit-Position: refs/heads/master@{#702235} --- cc/test/pixel_test_utils.cc | 4 +- content/renderer/render_widget.cc | 8 +- .../render_widget_screen_metrics_emulator.cc | 167 +++++++++--------- .../render_widget_screen_metrics_emulator.h | 28 +-- headless/BUILD.gn | 1 + headless/lib/DEPS | 1 + .../lib/headless_web_contents_browsertest.cc | 23 ++- .../public/web/web_device_emulation_params.h | 7 +- 8 files changed, 128 insertions(+), 111 deletions(-) diff --git a/cc/test/pixel_test_utils.cc b/cc/test/pixel_test_utils.cc index 41ce88fd81e481..bd4372b16fbc44 100644 --- a/cc/test/pixel_test_utils.cc +++ b/cc/test/pixel_test_utils.cc @@ -72,8 +72,8 @@ bool MatchesBitmap(const SkBitmap& gen_bmp, std::string gen_bmp_data_url = GetPNGDataUrl(gen_bmp); std::string ref_bmp_data_url = GetPNGDataUrl(ref_bmp); LOG(ERROR) << "Pixels do not match!"; - LOG(ERROR) << "Actual: " << gen_bmp_data_url; - LOG(ERROR) << "Expected: " << ref_bmp_data_url; + LOG(ERROR) << "Actual pixels (open in browser):\n" << gen_bmp_data_url; + LOG(ERROR) << "Expected pixels (open in browser):\n" << ref_bmp_data_url; } return compare; } diff --git a/content/renderer/render_widget.cc b/content/renderer/render_widget.cc index 74cdb603158764..ec2fb8fe49a4df 100644 --- a/content/renderer/render_widget.cc +++ b/content/renderer/render_widget.cc @@ -2116,11 +2116,11 @@ void RenderWidget::ScreenRectToEmulatedIfNeeded(WebRect* window_rect) const { window_rect->x = emulator->ViewRectOrigin().x() + - (window_rect->x - emulator->original_screen_rect().origin().x()) / + (window_rect->x - emulator->original_view_rect().origin().x()) / emulator->scale(); window_rect->y = emulator->ViewRectOrigin().y() + - (window_rect->y - emulator->original_screen_rect().origin().y()) / + (window_rect->y - emulator->original_view_rect().origin().y()) / emulator->scale(); } @@ -2135,10 +2135,10 @@ void RenderWidget::EmulatedToScreenRectIfNeeded(WebRect* window_rect) const { return; window_rect->x = - emulator->original_screen_rect().origin().x() + + emulator->original_view_rect().origin().x() + (window_rect->x - emulator->ViewRectOrigin().x()) * emulator->scale(); window_rect->y = - emulator->original_screen_rect().origin().y() + + emulator->original_view_rect().origin().y() + (window_rect->y - emulator->ViewRectOrigin().y()) * emulator->scale(); } diff --git a/content/renderer/render_widget_screen_metrics_emulator.cc b/content/renderer/render_widget_screen_metrics_emulator.cc index bfd7efcb6b12a9..1af5a2afcf2289 100644 --- a/content/renderer/render_widget_screen_metrics_emulator.cc +++ b/content/renderer/render_widget_screen_metrics_emulator.cc @@ -41,108 +41,118 @@ void RenderWidgetScreenMetricsEmulator::ChangeEmulationParams( } gfx::Point RenderWidgetScreenMetricsEmulator::ViewRectOrigin() { - gfx::Point widget_pos = original_screen_rect().origin(); + gfx::Point widget_pos = original_view_rect().origin(); if (emulation_params_.view_position) widget_pos = *emulation_params_.view_position; - else if (emulation_params_.screen_position != - blink::WebDeviceEmulationParams::kDesktop) + else if (!emulating_desktop()) widget_pos = gfx::Point(); return widget_pos; } void RenderWidgetScreenMetricsEmulator::Apply() { - ScreenInfo screen_info = original_screen_info_; - - applied_widget_rect_.set_size(gfx::Size(emulation_params_.view_size)); + // The WidgetScreenRect gets derived from the widget size of the main frame + // widget, not from the original WidgetScreenRect. + gfx::Size widget_size = original_widget_size_; + // The WindowScreenRect gets derived from the original WindowScreenRect, + // though. + gfx::Size window_size = original_window_rect().size(); + + // If either the width or height are specified by the emulator, then we use + // that size, and assume that they have the scale pre-applied to them. + if (emulation_params_.view_size.width) { + widget_size.set_width(emulation_params_.view_size.width); + } else { + widget_size.set_width( + gfx::ToRoundedInt(widget_size.width() / emulation_params_.scale)); + } + if (emulation_params_.view_size.height) { + widget_size.set_height(emulation_params_.view_size.height); + } else { + widget_size.set_height( + gfx::ToRoundedInt(widget_size.height() / emulation_params_.scale)); + } - if (!applied_widget_rect_.width()) - applied_widget_rect_.set_width(original_size().width()); + // For mobile emulation, the window size is changed to match the widget size, + // as there are no window decorations around the widget. + if (!emulating_desktop()) + window_size = widget_size; - if (!applied_widget_rect_.height()) - applied_widget_rect_.set_height(original_size().height()); + gfx::Point widget_pos = original_view_rect().origin(); + gfx::Point window_pos = original_window_rect().origin(); - scale_ = emulation_params_.scale; - if (!emulation_params_.view_size.width && - !emulation_params_.view_size.height && scale_) { - applied_widget_rect_.set_size( - gfx::ScaleToRoundedSize(original_size(), 1.f / scale_)); + if (emulation_params_.view_position) { + // The emulated widget position overrides the widget and window positions. + widget_pos = *emulation_params_.view_position; + window_pos = widget_pos; + } else if (!emulating_desktop()) { + // For mobile emulation, the widget and window are moved to 0,0 if not + // explicitly specified. + widget_pos = gfx::Point(); + window_pos = widget_pos; } - gfx::Rect window_screen_rect; - if (emulation_params_.screen_position == - blink::WebDeviceEmulationParams::kDesktop) { - screen_info.rect = original_screen_info().rect; - screen_info.available_rect = original_screen_info().available_rect; - window_screen_rect = original_window_screen_rect_; - if (emulation_params_.view_position) { - applied_widget_rect_.set_origin(*emulation_params_.view_position); - window_screen_rect.set_origin(*emulation_params_.view_position); - } else { - applied_widget_rect_.set_origin(original_view_screen_rect_.origin()); - } - } else { - applied_widget_rect_.set_origin( - emulation_params_.view_position.value_or(blink::WebPoint())); - screen_info.rect = applied_widget_rect_; - screen_info.available_rect = applied_widget_rect_; - window_screen_rect = applied_widget_rect_; - } + gfx::Rect screen_rect = original_screen_info().rect; if (!emulation_params_.screen_size.IsEmpty()) { - gfx::Rect screen_rect = gfx::Rect(0, 0, emulation_params_.screen_size.width, - emulation_params_.screen_size.height); - screen_info.rect = screen_rect; - screen_info.available_rect = screen_rect; + // The emulated screen size overrides the real one, and moves the screen's + // origin to 0,0. + screen_rect = gfx::Rect(emulation_params_.screen_size); + } else if (!emulating_desktop()) { + // For mobile emulation, the screen is adjusted to match the position and + // size of the widget rect, if not explicitly specified. + screen_rect = gfx::Rect(widget_pos, widget_size); } - screen_info.device_scale_factor = - emulation_params_.device_scale_factor - ? emulation_params_.device_scale_factor - : original_screen_info().device_scale_factor; - - if (emulation_params_.screen_orientation_type != - blink::kWebScreenOrientationUndefined) { - switch (emulation_params_.screen_orientation_type) { - case blink::kWebScreenOrientationPortraitPrimary: - screen_info.orientation_type = - SCREEN_ORIENTATION_VALUES_PORTRAIT_PRIMARY; - break; - case blink::kWebScreenOrientationPortraitSecondary: - screen_info.orientation_type = - SCREEN_ORIENTATION_VALUES_PORTRAIT_SECONDARY; - break; - case blink::kWebScreenOrientationLandscapePrimary: - screen_info.orientation_type = - SCREEN_ORIENTATION_VALUES_LANDSCAPE_PRIMARY; - break; - case blink::kWebScreenOrientationLandscapeSecondary: - screen_info.orientation_type = - SCREEN_ORIENTATION_VALUES_LANDSCAPE_SECONDARY; - break; - default: - screen_info.orientation_type = SCREEN_ORIENTATION_VALUES_DEFAULT; - break; - } - screen_info.orientation_angle = emulation_params_.screen_orientation_angle; + float device_scale_factor = original_screen_info().device_scale_factor; + + if (emulation_params_.device_scale_factor) + device_scale_factor = emulation_params_.device_scale_factor; + + ScreenOrientationValues orientation_type = + original_screen_info().orientation_type; + uint16_t orientation_angle = original_screen_info().orientation_angle; + + switch (emulation_params_.screen_orientation_type) { + case blink::kWebScreenOrientationUndefined: + break; // Leave as the real value. + case blink::kWebScreenOrientationPortraitPrimary: + orientation_type = SCREEN_ORIENTATION_VALUES_PORTRAIT_PRIMARY; + orientation_angle = emulation_params_.screen_orientation_angle; + break; + case blink::kWebScreenOrientationPortraitSecondary: + orientation_type = SCREEN_ORIENTATION_VALUES_PORTRAIT_SECONDARY; + orientation_angle = emulation_params_.screen_orientation_angle; + break; + case blink::kWebScreenOrientationLandscapePrimary: + orientation_type = SCREEN_ORIENTATION_VALUES_LANDSCAPE_PRIMARY; + orientation_angle = emulation_params_.screen_orientation_angle; + break; + case blink::kWebScreenOrientationLandscapeSecondary: + orientation_type = SCREEN_ORIENTATION_VALUES_LANDSCAPE_SECONDARY; + orientation_angle = emulation_params_.screen_orientation_angle; + break; } // Pass three emulation parameters to the blink side: // - we keep the real device scale factor in compositor to produce sharp image // even when emulating different scale factor; - // - in order to fit into view, WebView applies offset and scale to the - // root layer. blink::WebDeviceEmulationParams modified_emulation_params = emulation_params_; modified_emulation_params.device_scale_factor = original_screen_info().device_scale_factor; - modified_emulation_params.scale = scale_; delegate_->SetScreenMetricsEmulationParameters(true, modified_emulation_params); - delegate_->SetScreenRects(applied_widget_rect_, window_screen_rect); - - delegate_->SetScreenInfoAndSize( - screen_info, /*widget_size=*/applied_widget_rect_.size(), - /*visible_viewport_size=*/applied_widget_rect_.size()); + delegate_->SetScreenRects(gfx::Rect(widget_pos, widget_size), + gfx::Rect(window_pos, window_size)); + + ScreenInfo screen_info = original_screen_info(); + screen_info.device_scale_factor = device_scale_factor; + screen_info.rect = screen_rect; + screen_info.available_rect = screen_rect; + screen_info.orientation_type = orientation_type; + screen_info.orientation_angle = orientation_angle; + delegate_->SetScreenInfoAndSize(screen_info, /*widget_size=*/widget_size, + /*visible_viewport_size=*/widget_size); } void RenderWidgetScreenMetricsEmulator::OnSynchronizeVisualProperties( @@ -160,16 +170,15 @@ void RenderWidgetScreenMetricsEmulator::OnUpdateScreenRects( const gfx::Rect& window_screen_rect) { original_view_screen_rect_ = view_screen_rect; original_window_screen_rect_ = window_screen_rect; - if (emulation_params_.screen_position == - blink::WebDeviceEmulationParams::kDesktop) { + if (emulating_desktop()) { Apply(); } } void RenderWidgetScreenMetricsEmulator::OnShowContextMenu( ContextMenuParams* params) { - params->x *= scale_; - params->y *= scale_; + params->x *= emulation_params_.scale; + params->y *= emulation_params_.scale; } } // namespace content diff --git a/content/renderer/render_widget_screen_metrics_emulator.h b/content/renderer/render_widget_screen_metrics_emulator.h index 54cac135181ae1..3533b615769113 100644 --- a/content/renderer/render_widget_screen_metrics_emulator.h +++ b/content/renderer/render_widget_screen_metrics_emulator.h @@ -32,16 +32,23 @@ class CONTENT_EXPORT RenderWidgetScreenMetricsEmulator { const gfx::Rect& window_screen_rect); ~RenderWidgetScreenMetricsEmulator(); - const gfx::Size& original_size() const { return original_widget_size_; } const ScreenInfo& original_screen_info() const { return original_screen_info_; } - const gfx::Rect& original_screen_rect() const { + // This rect is the WidgetScreenRect or ViewRect, which is the main frame + // widget's bounding box, not including OS window decor, in logical DIP screen + // coordinates. + const gfx::Rect& original_view_rect() const { return original_view_screen_rect_; } + // This rect is the WindowScreenRect or WindowRect, which is the bounding box + // of the main frame's top level window, including OS window decor, in logical + // DIP screen coordinates. + const gfx::Rect& original_window_rect() const { + return original_window_screen_rect_; + } - float scale() const { return scale_; } - const gfx::Rect& applied_widget_rect() const { return applied_widget_rect_; } + float scale() const { return emulation_params_.scale; } // Emulated position of the main frame widget (aka view) rect. gfx::Point ViewRectOrigin(); @@ -63,20 +70,19 @@ class CONTENT_EXPORT RenderWidgetScreenMetricsEmulator { void OnShowContextMenu(ContextMenuParams* params); private: - RenderWidgetScreenMetricsEmulatorDelegate* const delegate_; + bool emulating_desktop() const { + return emulation_params_.screen_position == + blink::WebDeviceEmulationParams::kDesktop; + } // Applies emulated values to the RenderWidget. void Apply(); + RenderWidgetScreenMetricsEmulatorDelegate* const delegate_; + // Parameters as passed by RenderWidget::EnableScreenMetricsEmulation. blink::WebDeviceEmulationParams emulation_params_; - // The computed scale and offset used to fit widget into browser window. - float scale_ = 1; - - // Widget rect as passed to webwidget. - gfx::Rect applied_widget_rect_; - // Original values to restore back after emulation ends. ScreenInfo original_screen_info_; gfx::Size original_widget_size_; diff --git a/headless/BUILD.gn b/headless/BUILD.gn index 5efc30d385f02b..9b23b9975c6d9d 100644 --- a/headless/BUILD.gn +++ b/headless/BUILD.gn @@ -757,6 +757,7 @@ test("headless_browsertests") { deps = [ ":headless_shell_lib", "//base", + "//cc:test_support", "//components/security_state/content", "//components/services/pdf_compositor/public/mojom", "//content/test:test_support", diff --git a/headless/lib/DEPS b/headless/lib/DEPS index b2c24395c7cb5a..ca032d51e78703 100644 --- a/headless/lib/DEPS +++ b/headless/lib/DEPS @@ -8,6 +8,7 @@ specific_include_rules = { ], "headless_web_contents_browsertest.cc": [ "+cc/base/switches.h", + "+cc/test", "+components/viz/common/features.h", "+components/viz/common/switches.h", "+pdf", diff --git a/headless/lib/headless_web_contents_browsertest.cc b/headless/lib/headless_web_contents_browsertest.cc index 50c33556c5848a..1aaddc97d3f107 100644 --- a/headless/lib/headless_web_contents_browsertest.cc +++ b/headless/lib/headless_web_contents_browsertest.cc @@ -14,6 +14,7 @@ #include "base/strings/stringprintf.h" #include "build/build_config.h" #include "cc/base/switches.h" +#include "cc/test/pixel_test_utils.h" #include "components/viz/common/switches.h" #include "content/public/browser/web_contents.h" #include "content/public/common/content_switches.h" @@ -804,20 +805,16 @@ class HeadlessWebContentsBeginFrameControlViewportTest SkBitmap result_bitmap; EXPECT_TRUE(DecodePNG(png_data, &result_bitmap)); - EXPECT_EQ(300, result_bitmap.width()); - EXPECT_EQ(300, result_bitmap.height()); - SkColor expected_color = SkColorSetRGB(0x00, 0x00, 0xff); + // Expext a 300x300 bitmap that is all blue. + SkBitmap expected_bitmap; + SkImageInfo info; + expected_bitmap.allocPixels( + SkImageInfo::MakeN32(300, 300, kOpaque_SkAlphaType), /*row_bytes=*/0); + expected_bitmap.eraseColor(SkColorSetRGB(0x00, 0x00, 0xff)); - SkColor actual_color = result_bitmap.getColor(100, 100); - EXPECT_EQ(expected_color, actual_color); - actual_color = result_bitmap.getColor(0, 0); - EXPECT_EQ(expected_color, actual_color); - actual_color = result_bitmap.getColor(0, 299); - EXPECT_EQ(expected_color, actual_color); - actual_color = result_bitmap.getColor(299, 0); - EXPECT_EQ(expected_color, actual_color); - actual_color = result_bitmap.getColor(299, 299); - EXPECT_EQ(expected_color, actual_color); + EXPECT_TRUE( + cc::MatchesBitmap(result_bitmap, expected_bitmap, + cc::ExactPixelComparator(/*discard_alpha=*/false))); } // Post completion to avoid deleting the WebContents on the same callstack diff --git a/third_party/blink/public/web/web_device_emulation_params.h b/third_party/blink/public/web/web_device_emulation_params.h index c11b505d5e3ff2..a9d6de84c48153 100644 --- a/third_party/blink/public/web/web_device_emulation_params.h +++ b/third_party/blink/public/web/web_device_emulation_params.h @@ -29,13 +29,16 @@ struct WebDeviceEmulationParams { // original one for kDesktop screen position, (0, 0) for kMobile. base::Optional view_position; - // Emulated view size. Empty size means no override. + // Emulated view size. A width or height of 0 means no override in that + // dimension, but the other can still be applied. When both are 0, then the + // |scale| will be applied to the view instead. WebSize view_size; // If zero, the original device scale factor is preserved. float device_scale_factor; - // Scale of emulated view inside available space, not in fit to view mode. + // Scale the contents of the main frame. The view's size will be scaled by + // this number when they are not specified in |view_size|. float scale; // Forced viewport offset for screenshots during emulation, (-1, -1) for