New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocked on:
issue 689777
issue 927694

Blocking:
issue 419087



Sign in to add a comment
link

Issue 912193: Split WebView and WebWidget implementations

Reported by danakj@chromium.org, Dec 5 Project Member

Issue description

WebViewImpl is both today, but we should give them separate objects and lifetimes. WebWidget should only exist when using compositing. It should be tied to lifetime of the main frame, almost certainly (and one per frame). At this point WebViewFrameWidget could go away, I beleive, replaced with the split-out WebWidget impl.

Subframes can continue to use WebFrameWidgetImpl, but not clear if that could also be used for the main frame as the impl of WebWidget. Right now, no, but these two things (view and main widget) are tangled up. Maybe if we moved all the view specific things to WebView appropriately it could.

re WebViewTest.SetBaseBackgroundColorBeforeMainFrame:
- BackgroundColor() getter should be a view thing
- WidgetMsg_SetBackgroundOpaque should be a ViewMsg
- RenderWidget::OnSetBackgroundOpaque but it only acts for main frames and seems like it wants to persist across main frames like its more like a page prop

re focus vs active
- WidgetMsg_SetActive should be a ViewMsg
- We have SetFocus going to input handler posting to main thread RenderWidget
- We also have page level SetActive
- WebView::SetIsActive vs WebWidget::SetFocused
- WebWidget::SetFocused only sets active on the page, not inactive ??? But always sets SetFocused().
- Can we make a single signal/state instead of 2 ? Is one per-widget and one per-page ??
- Tests do `web_view_->GetPage()->GetFocusController().SetActive(true);` instead of going through the WebView or WebWidget. Does other code do this too ?? oh god. WebPagePopupImpl does. WebFrameWidgetImpl does.
- WebView also has SetInitialFocus() which acts on the main frame. Should possibly be part of WebWidget, though also interacts with Page.
- child local roots don't know about focus correctly, which probably means main frame widgets won't either if we separate them from the view enough. we likely fix both at once. https://bugs.chromium.org/p/chromium/issues/detail?id=689777

re Close
- Close() is on WebWidget, but also closes the WebView
- They should probably each have their own Close once they split.

re Resizing
- Resize() is a WebWidget method, but
- ResizeWithBrowserControls is a WebView method
- Should these both be on the same API? The first is implemented by calling the second.
- WebFrameWidgetImpl Resize code has some "MainFrame" stuff in its names that seems to be incorrect, as it actually just acts on its frame which can be (always is atm) a subframe.

* re popups
* - GetPagePopup() is part of WebWidget but is only ever called for the main frame, and as per its name, is related to the Page not the Widget.
* - Should move GetPagePopup() to WebView.
* - As an extra treat WebViewImpl overrides this but changes the return type of GetPagePopup to WebPagePopupImpl - an internal blink type.

re visual viewport
- ResizeVisualViewport() is part of WebWidget but only modifies values on Page, should be WebView?

re cursor visibility
- WebViewImpl::SetCursorVisibilityState() changes page state only but is a WebWidget method.
- It is called from a RenderWidget IPC

* = done
 

Comment 1 by danakj@chromium.org, Dec 5

Blocking: 419087

Comment 2 by danakj@chromium.org, Dec 5

re WebViewTest.SetBaseBackgroundColorBeforeMainFrame:
- BackgroundColor() getter should be a view thing
- WidgetMsg_SetBackgroundOpaque should be a ViewMsg
- RenderWidget::OnSetBackgroundOpaque but it only acts for main frames and seems like it wants to persist across main frames like its more like a page prop

re focus vs active
- WidgetMsg_SetActive should be a ViewMsg
- We have SetFocus going to input handler posting to main thread RenderWidget
- We also have page level SetActive
- WebView::SetIsActive vs WebWidget::SetFocused
- WebWidget::SetFocused only sets active on the page, not inactive ??? But always sets SetFocused().
- Can we make a single signal/state instead of 2 ? Is one per-widget and one per-page ??
- Tests do `web_view_->GetPage()->GetFocusController().SetActive(true);` instead of going through the WebView or WebWidget. Does other code do this too ?? oh god. WebPagePopupImpl does. WebFrameWidgetImpl does.

Comment 3 by danakj@chromium.org, Dec 5

Cc: a...@chromium.org enne@chromium.org piman@chromium.org

Comment 4 by danakj@chromium.org, Dec 5

re focus con't
- WebView also has SetInitialFocus() which acts on the main frame. Should possibly be part of WebWidget, though also interacts with Page.

re Close
- Close() is on WebWidget, but also closes the WebView
- They should probably each have their own Close once they split.

re Resizing
- Resize() is a WebWidget method, but
- ResizeWithBrowserControls is a WebView method
- Should these both be on the same API? The first is implemented by calling the second.
- WebFrameWidgetImpl Resize code has some "MainFrame" stuff in its names that seems to be incorrect, as it actually just acts on its frame which can be (always is atm) a subframe.

Comment 5 by danakj@chromium.org, Dec 5

re popups
- GetPagePopup() is part of WebWidget but is only ever called for the main frame, and as per its name, is related to the Page not the Widget.
- Should move GetPagePopup() to WebView.
- As an extra treat WebViewImpl overrides this but changes the return type of GetPagePopup to WebPagePopupImpl - an internal blink type.

re visual viewport
- ResizeVisualViewport() is part of WebWidget but only modifies values on Page, should be WebView?

Comment 6 by bugdroid1@chromium.org, Dec 7

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/497a616eccb04d08841535091b1ab4c9c3599a72

commit 497a616eccb04d08841535091b1ab4c9c3599a72
Author: danakj <danakj@chromium.org>
Date: Thu Dec 06 20:35:48 2018

Make WebWidget methods on WebView private.

Callers should go through WebView's MainFrameWidget() to get to them.
While WebViewImpl inherits privately from WebWidget, the overrides of
its methods were all public. This moves them to the private section
with the single exception of GetPagePopup(). This method probably
belongs on WebView, but WebViewImpl changes its return type, so it's
harder to simply move mechanically. A TODO is left for that one.

This change is mechanical and not opinionated. All callers that failed
to compile are simply switched to call MainFrameWidget()-> in order to
get to the methods.

R=dcheng@chromium.org

Change-Id: I44cb270a97f3a4034f00c00631dad8631678ef53
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1363855
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614479}
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/public/web/web_view.h
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/animation/compositor_animations_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/editing/finder/text_finder_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/editing/link_selection_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/events/web_input_event_conversion_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/exported/web_frame_serializer_sanitization_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/exported/web_layer_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/exported/web_node_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/exported/web_plugin_container_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/exported/web_view_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/frame/browser_controls_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/frame/document_loading_rendering_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/frame/frame_overlay_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/frame/frame_test_helpers.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/frame/fullscreen_controller.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/frame/rotation_viewport_anchor_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/frame/visual_viewport_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/html/anchor_element_metrics_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/html/forms/external_popup_menu_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/html/image_document_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/html/imports/html_import_sheets_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/html/lazy_load_frame_observer_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/html/media_element_filling_viewport_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/input/event_handler_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/input/ime_on_focus_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/input/overscroll_behavior_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/input/pointer_event_manager_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/input/scroll_snap_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/input/touch_action_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/input/touch_event_manager_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/intersection_observer/intersection_observer_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/layout/scrollbars_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/loader/programmatic_scroll_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/page/autoscroll_controller_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/page/chrome_client_impl.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/page/drag_controller_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/page/scrolling/scroll_into_view_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/page/scrolling/scroll_metrics_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/page/viewport_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/paint/link_highlight_impl_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/scheduler/frame_throttling_test.cc
[modify] https://crrev.com/497a616eccb04d08841535091b1ab4c9c3599a72/third_party/blink/renderer/core/testing/sim/sim_compositor.cc

Comment 7 by bugdroid1@chromium.org, Dec 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53f46b89a44013ec4ffece80fe3f1f044e4a4f49

commit 53f46b89a44013ec4ffece80fe3f1f044e4a4f49
Author: danakj <danakj@chromium.org>
Date: Tue Dec 11 20:55:55 2018

Remove ScheduleAnimation plumbing in WebFrameWidget/WebView/LayerTreeView.

Have ChromeClient go directly to WebWidgetClient instead of through the
WebFrameWidget. The WebFrameWidgetImpl would go either to the compositor
(which always exists) or to WebWidgetClient. The latter path was dead.
Similar in WebViewImpl.

Then when ChromeClient (via plumbing..) would ping the compositor, it
would callback to RenderWidget to say RequestScheduleAnimation() for
single-thread-no-scheduler tests. This would redirect to
WebWidgetClient (empty for RenderWidget) in order to pick up test
overrides.

Instead, ChromeClient goes to WebWidetClient which is RenderWidget (or
test overrides). RenderWidget goes to the compositor.

Then we don't need the callback from the compositor anymore, so remove
it and change tests to watch ScheduleAnimation() as needed directly
instead.

WebLayerTreeView also has an equivalent of ScheduleAnimation() which
would get to the test harness by round-tripping through cc. Instead of
re-routing it, remove that API and have callers go through the
WebWidgetClient to ScheduleAnimation().

R=dcheng@chromium.org
TBR=dcheng for components/

Change-Id: I3c1a5a4c2c6dfc8008067891531b35e5d845b4ba
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1369032
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615656}
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/cc/test/stub_layer_tree_host_single_thread_client.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/cc/trees/layer_tree_host.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/cc/trees/layer_tree_host.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/cc/trees/layer_tree_host_single_thread_client.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/cc/trees/layer_tree_host_unittest.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/cc/trees/single_thread_proxy.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/components/plugins/renderer/webview_plugin.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/renderer/compositor/layer_tree_view.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/renderer/compositor/layer_tree_view.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/renderer/compositor/layer_tree_view_delegate.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/renderer/render_view_impl.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/renderer/render_view_impl.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/renderer/render_widget.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/renderer/render_widget.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/renderer/render_widget_owner_delegate.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/renderer/render_widget_unittest.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/shell/test_runner/web_view_test_proxy.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/shell/test_runner/web_view_test_proxy.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/shell/test_runner/web_widget_test_client.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/shell/test_runner/web_widget_test_proxy.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/content/test/stub_layer_tree_view_delegate.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/public/platform/web_layer_tree_view.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/public/web/web_widget_client.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/exported/web_page_popup_impl.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/exported/web_view_test.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/frame/frame_test_helpers.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/frame/web_frame_widget_base.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/frame/web_frame_widget_impl.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/frame/web_view_frame_widget.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/frame/web_view_frame_widget.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/page/chrome_client_impl.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/testing/sim/sim_compositor.cc
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/testing/sim/sim_compositor.h
[modify] https://crrev.com/53f46b89a44013ec4ffece80fe3f1f044e4a4f49/third_party/blink/renderer/core/testing/sim/sim_test.cc

Comment 8 by bugdroid1@chromium.org, Dec 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3788017f9d2c2904fb37b9df12e3134490ca98c2

commit 3788017f9d2c2904fb37b9df12e3134490ca98c2
Author: danakj <danakj@chromium.org>
Date: Tue Dec 11 21:33:01 2018

Split TestWebViewClient and TestWebWidgetClient.

WebViewClient and WebWidgetClient no longer inherit each other, do the
same with these test overrides.

WebViewHelper takes a TestWebViewClient and if null makes one itself.
Similarly TestWebViewClient takes a TestWebWidgetClient, and if null
makes one itself. Then WebViewHelper gets the WebWidgetClient from
the WebViewClient, avoiding us having 2 channels for this object
through WebViewHelper.

R=dcheng@chromium.org

Change-Id: If9ba127bafc19a8a881c971aefdb25c201656d51
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1370720
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615669}
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/css/style_engine_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/exported/web_layer_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/exported/web_view_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/frame/browser_controls_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/frame/frame_serializer_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/frame/frame_test_helpers.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/frame/frame_test_helpers.h
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/frame/visual_viewport_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/input/ime_on_focus_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/input/touch_action_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/layout/scrollbars_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/page/chrome_client_impl_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/page/spatial_navigation_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/page/viewport_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/testing/sim/sim_compositor.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/testing/sim/sim_compositor.h
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/testing/sim/sim_test.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/testing/sim/sim_test.h
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/testing/sim/sim_web_view_client.cc
[modify] https://crrev.com/3788017f9d2c2904fb37b9df12e3134490ca98c2/third_party/blink/renderer/core/testing/sim/sim_web_view_client.h

Comment 9 by bugdroid1@chromium.org, Dec 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/573f46751824b8dcfbec675291eacc8c581ae441

commit 573f46751824b8dcfbec675291eacc8c581ae441
Author: danakj <danakj@chromium.org>
Date: Tue Dec 11 22:50:40 2018

Remove code to store the last popup transiently in WebFrameWidgetImpl

This code was copied over from WebViewImpl when adding the HidePopups
call in order to have a pinch zoom or scroll tap outside the current
popup in a non-main-frame-local-rooted iframe hide the current popup.

This is modifying state on WebViewImpl meant to track between TapDown
and Tap, to avoid the Tap showing a popup that was hidden by TapDown.

WebFrameWidgetImpl does not show popups on Tap, and if it did, it
should track this transient state locally not on WebViewImpl.

Also remove the undocumented FALLTHROUGH which was not explained
in 671732 as it now goes to nothing.

R=dcheng@chromium.org

Change-Id: I9d67539cf0afbb9db239b73bf83418e1a877e6db
Bug: 912193, 671732
Reviewed-on: https://chromium-review.googlesource.com/c/1372539
Reviewed-by: James MacLean <wjmaclean@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615708}
[modify] https://crrev.com/573f46751824b8dcfbec675291eacc8c581ae441/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc

Comment 10 by bugdroid1@chromium.org, Dec 12

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc76bd1252ad56309ecad13729305c94b1f57ba7

commit dc76bd1252ad56309ecad13729305c94b1f57ba7
Author: Alice Boxhall <aboxhall@chromium.org>
Date: Wed Dec 12 02:11:12 2018

Revert "Remove code to store the last popup transiently in WebFrameWidgetImpl"

This reverts commit 573f46751824b8dcfbec675291eacc8c581ae441.

Reason for revert: Seems to be causing EffectiveTouchActionPropagatesAcrossNestedFrames to fail https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8927388648425983952/+/steps/content_browsertests_on_Intel_GPU_on_Mac_on_Mac-10.12.6/0/logs/SitePerProcessBrowserTouchActionTest.EffectiveTouchActionPropagatesAcrossNestedFrames/0

Original change's description:
> Remove code to store the last popup transiently in WebFrameWidgetImpl
> 
> This code was copied over from WebViewImpl when adding the HidePopups
> call in order to have a pinch zoom or scroll tap outside the current
> popup in a non-main-frame-local-rooted iframe hide the current popup.
> 
> This is modifying state on WebViewImpl meant to track between TapDown
> and Tap, to avoid the Tap showing a popup that was hidden by TapDown.
> 
> WebFrameWidgetImpl does not show popups on Tap, and if it did, it
> should track this transient state locally not on WebViewImpl.
> 
> Also remove the undocumented FALLTHROUGH which was not explained
> in 671732 as it now goes to nothing.
> 
> R=‚Äčdcheng@chromium.org
> 
> Change-Id: I9d67539cf0afbb9db239b73bf83418e1a877e6db
> Bug: 912193, 671732
> Reviewed-on: https://chromium-review.googlesource.com/c/1372539
> Reviewed-by: James MacLean <wjmaclean@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Commit-Queue: danakj <danakj@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#615708}

TBR=danakj@chromium.org,dcheng@chromium.org,wjmaclean@chromium.org

Change-Id: Ifddbb605fc6165505774105d5618d57b62e3159c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 912193, 671732
Reviewed-on: https://chromium-review.googlesource.com/c/1372110
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Commit-Queue: Alice Boxhall <aboxhall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615783}
[modify] https://crrev.com/dc76bd1252ad56309ecad13729305c94b1f57ba7/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc

Comment 11 by danakj@chromium.org, Dec 12

Blockedon: 689777
re focus con't: https://bugs.chromium.org/p/chromium/issues/detail?id=689777

I'm going to smush all my comments up into #0

Comment 12 by danakj@chromium.org, Dec 12

Description: Show this description

Comment 13 by danakj@chromium.org, Dec 12

Description: Show this description

Comment 14 by danakj@chromium.org, Dec 12

Description: Show this description

Comment 15 by danakj@chromium.org, Dec 12

Description: Show this description

Comment 16 by bugdroid1@chromium.org, Dec 12

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/72ac928f90e123b0a0fd54e3be0f6ab1fb79812d

commit 72ac928f90e123b0a0fd54e3be0f6ab1fb79812d
Author: danakj <danakj@chromium.org>
Date: Wed Dec 12 16:53:02 2018

Reland "Remove code to store the last popup transiently in WebFrameWidgetImpl"

This is a reland of 573f46751824b8dcfbec675291eacc8c581ae441

Was reverted because SitePerProcessBrowserTouchActionTest.EffectiveTouchActionPropagatesAcrossNestedFrames
failed when it landed. But the test went back to green for a few runs before
this was reverted, so I don't think it was at fault. We'll see if it starts
failing again on reland, and I'll watch the mac bots here in the CQ.

TBR=dcheng

Original change's description:
> Remove code to store the last popup transiently in WebFrameWidgetImpl
>
> This code was copied over from WebViewImpl when adding the HidePopups
> call in order to have a pinch zoom or scroll tap outside the current
> popup in a non-main-frame-local-rooted iframe hide the current popup.
>
> This is modifying state on WebViewImpl meant to track between TapDown
> and Tap, to avoid the Tap showing a popup that was hidden by TapDown.
>
> WebFrameWidgetImpl does not show popups on Tap, and if it did, it
> should track this transient state locally not on WebViewImpl.
>
> Also remove the undocumented FALLTHROUGH which was not explained
> in 671732 as it now goes to nothing.
>
> R=dcheng@chromium.org
>
> Change-Id: I9d67539cf0afbb9db239b73bf83418e1a877e6db
> Bug: 912193, 671732
> Reviewed-on: https://chromium-review.googlesource.com/c/1372539
> Reviewed-by: James MacLean <wjmaclean@chromium.org>
> Reviewed-by: Daniel Cheng <dcheng@chromium.org>
> Commit-Queue: danakj <danakj@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#615708}

Bug: 912193, 671732
Change-Id: I18e8e146b5b525239d1667e267d44fa8a295599d
Reviewed-on: https://chromium-review.googlesource.com/c/1374193
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615925}
[modify] https://crrev.com/72ac928f90e123b0a0fd54e3be0f6ab1fb79812d/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc

Comment 17 by bugdroid1@chromium.org, Dec 12

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/89ae4532e2e5a3d32a205c15b2e17e74da6617ad

commit 89ae4532e2e5a3d32a205c15b2e17e74da6617ad
Author: danakj <danakj@chromium.org>
Date: Wed Dec 12 22:52:55 2018

Clean up RenderWidget type logic for main- vs sub-frames.

Remove RenderWidget::WidgetType, replace it with for_frame(), which can
derive an answer from the presence of owner_delegate_ (for a main frame)
and for_child_local_root_frame_ (for sub frames).

This changes pepper plugin widgets from being considered frames to not,
which seems okay. One piece of code wanted to run only for popup widgets
but its in SetWindowRect which is not used on the pepper widget.

Remove use of IsWebView() and IsWebFrameWidget() on the WebWidget from
RenderWidget. Instead use for_frame() or its derivative parts.

We want to stop checking for the presence of a WebFrameWidget and drop
IPCs early when a RenderWidget is frozen, but there's one bug that
prevents that still as the Frame (and FrameWidget) can be detached
without freezing the RenderWidget (crbug.com/906340#c7).

R=ajwong@chromium.org, avi@chromium.org

Change-Id: I83589e2b0cf5a4c68dde77eaae87404ba1de4cc0
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1373887
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616093}
[modify] https://crrev.com/89ae4532e2e5a3d32a205c15b2e17e74da6617ad/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/89ae4532e2e5a3d32a205c15b2e17e74da6617ad/content/renderer/render_frame_impl.h
[modify] https://crrev.com/89ae4532e2e5a3d32a205c15b2e17e74da6617ad/content/renderer/render_view_impl.cc
[modify] https://crrev.com/89ae4532e2e5a3d32a205c15b2e17e74da6617ad/content/renderer/render_widget.cc
[modify] https://crrev.com/89ae4532e2e5a3d32a205c15b2e17e74da6617ad/content/renderer/render_widget.h
[modify] https://crrev.com/89ae4532e2e5a3d32a205c15b2e17e74da6617ad/content/renderer/render_widget_fullscreen_pepper.cc
[modify] https://crrev.com/89ae4532e2e5a3d32a205c15b2e17e74da6617ad/content/renderer/render_widget_unittest.cc
[modify] https://crrev.com/89ae4532e2e5a3d32a205c15b2e17e74da6617ad/content/test/web_test_support.cc

Comment 18 by bugdroid1@chromium.org, Dec 13

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610

commit a5c39d5f43575a87c7f6b6c858b3ee5c3be24610
Author: danakj <danakj@chromium.org>
Date: Thu Dec 13 22:20:56 2018

Clean up WebPagePopup APIs and move them from WebWidget to WebView.

This moves WebWidget::GetPagePopup() to WebView, as there is only 1
popup per view, not per widget. WebTest code was grabbing the popup
off the WebWidget and is changed to grab it from the WebView instead
when the WebWidget is for the main frame (which it always is right now
but we add conditions for correctness regardless).

WebViewImpl had a bunch of ways to close the current popup, and we
remove most of the redundant ways: HideSelectPopup() is removed.
HidePopups() is removed, and CancelPagePopup() is changed to replace
it.

SetLastHiddenPagePopup() is removed from the public API of WebViewImpl
as it is no longer used externally, and code inside changed to use the
variable directly.

WebPagePopupClient::ClosePopup() is renamed to CancelPopup() to give
consistent naming throughout the Cancel code.

WebViewTestProxy changed to defer WidgetClient() through to the
WebViewTestClient, like it does for other methods, and we pass the
ProxyWebWidgetClient to the WebViewTestClient for it to hand out,
instead of giving it null.

R=dcheng@chromium.org
TBR=

Change-Id: Ibaddec943ead28153290cd03a3d94790ac5d4fe0
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1372657
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616455}
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/android_webview/renderer/aw_render_frame_ext.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/renderer/render_view_impl.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/renderer/web_test/web_test_render_frame_observer.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/event_sender.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/layout_and_paint_async_then.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/layout_and_paint_async_then.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/pixel_dump.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/test_runner_for_specific_view.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/web_test_interfaces.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/web_test_interfaces.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/web_view_test_proxy.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/web_view_test_proxy.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/web_widget_test_client.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/web_widget_test_client.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/content/shell/test_runner/web_widget_test_proxy.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/public/web/web_view.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/public/web/web_widget.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/exported/web_page_popup_impl.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/frame/web_view_frame_widget.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/frame/web_view_frame_widget.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/html/forms/color_chooser_popup_ui_controller.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/html/forms/color_chooser_popup_ui_controller.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/html/forms/date_time_chooser_impl.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/html/forms/date_time_chooser_impl.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/html/forms/internal_popup_menu.cc
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/html/forms/internal_popup_menu.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/page/page_popup_client.h
[modify] https://crrev.com/a5c39d5f43575a87c7f6b6c858b3ee5c3be24610/third_party/blink/renderer/core/page/page_popup_controller.cc

Comment 19 by mstensho@chromium.org, Dec 14

EffectiveTouchActionPropagatesAcrossNestedFrames failed again:

https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/linux-chromeos-rel/17272

Comment 20 by danakj@chromium.org, Dec 14

From https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=content_browsertests&tests=EffectiveTouchActionPropagatesAcrossNestedFrames it looks like EffectiveTouchActionPropagatesAcrossNestedFrames is consistently flaky, but has been flaky while my CL was not in the tree too.

It failed in this run (pass on retry): https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.11%20Tests/31680

Which was at 615821.
My popup CL landed 615708, reverted 615783, reland 615925.
So this flake was between revert and reland.

Comment 21 by danakj@chromium.org, Dec 14

I filed 915270 for this test.

Comment 22 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c497a559cf4e56d4ca95cdf87071e1555916bc77

commit c497a559cf4e56d4ca95cdf87071e1555916bc77
Author: danakj <danakj@chromium.org>
Date: Fri Dec 14 17:29:35 2018

Remove use of WebWidget's IsWebView() and IsWebFrameWidget().

Use is-main-frame logic instead, which will remain true as we convert
the WebWidget for the main frame away from a WebView to a
WebFrameWidget.

This removes a dependency on WebWidget being-a WebView.

R=avi@chromium.org, dcheng@chromium.org

Change-Id: I8af02679ba0aaff8ef638757e04a533b84ce364d
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1374463
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616725}
[modify] https://crrev.com/c497a559cf4e56d4ca95cdf87071e1555916bc77/content/shell/test_runner/web_view_test_proxy.cc
[modify] https://crrev.com/c497a559cf4e56d4ca95cdf87071e1555916bc77/content/shell/test_runner/web_widget_test_client.cc
[modify] https://crrev.com/c497a559cf4e56d4ca95cdf87071e1555916bc77/content/shell/test_runner/web_widget_test_client.h
[modify] https://crrev.com/c497a559cf4e56d4ca95cdf87071e1555916bc77/content/shell/test_runner/web_widget_test_proxy.cc
[modify] https://crrev.com/c497a559cf4e56d4ca95cdf87071e1555916bc77/content/shell/test_runner/web_widget_test_proxy.h
[modify] https://crrev.com/c497a559cf4e56d4ca95cdf87071e1555916bc77/content/test/web_test_support.cc

Comment 23 by bugdroid1@chromium.org, Dec 14

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/19f3e51301b570e9edbd6c8b56b20e7c0466ac58

commit 19f3e51301b570e9edbd6c8b56b20e7c0466ac58
Author: danakj <danakj@chromium.org>
Date: Fri Dec 14 21:02:05 2018

Remove WebWidget::IsPagePopup() and IsWebView().

Neither is used anymore after this CL. IsPagePopup was only used to
verify a cast from WebWidget to WebPagePopupImpl. We can change the
type from WebWidget to WebPagePopup though, in which case it's not
needed and we get more clear code in the process :)

IsWebView() can not be true if the WebViewImpl is no longer also a
WebWidget, so remove it since we are going to make it always false.

R=dcheng@chromium.org

Change-Id: I040f5d62f1b88d825c8587104acdf0155bcef7f7
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1376092
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616817}
[modify] https://crrev.com/19f3e51301b570e9edbd6c8b56b20e7c0466ac58/content/renderer/render_view_impl.cc
[modify] https://crrev.com/19f3e51301b570e9edbd6c8b56b20e7c0466ac58/content/renderer/render_view_impl.h
[modify] https://crrev.com/19f3e51301b570e9edbd6c8b56b20e7c0466ac58/content/renderer/render_widget.h
[modify] https://crrev.com/19f3e51301b570e9edbd6c8b56b20e7c0466ac58/third_party/blink/public/web/web_view_client.h
[modify] https://crrev.com/19f3e51301b570e9edbd6c8b56b20e7c0466ac58/third_party/blink/public/web/web_widget.h
[modify] https://crrev.com/19f3e51301b570e9edbd6c8b56b20e7c0466ac58/third_party/blink/renderer/core/exported/web_page_popup_impl.h
[modify] https://crrev.com/19f3e51301b570e9edbd6c8b56b20e7c0466ac58/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/19f3e51301b570e9edbd6c8b56b20e7c0466ac58/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/19f3e51301b570e9edbd6c8b56b20e7c0466ac58/third_party/blink/renderer/core/frame/web_view_frame_widget.h

Comment 24 by bugdroid1@chromium.org, Dec 21

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a

commit d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a
Author: danakj <danakj@chromium.org>
Date: Fri Dec 21 23:06:09 2018

Move BackgroundColor from widgets and child frames to the WebView.

SetBackgroundOpaque() is a concept that applies only to WebViews, not to
WebWidgets. It is set by ChromeOS UI pieces, which each have their own
WebView/Page. It is also set by webview tag, which has its own WebView
contained inside another outer one. So we move the SetBackgroundOpaque()
methods and IPCs from RenderWidget to RenderView.

In the renderer, the WebWidget::BackgroundColor() is not used except
internally, so remove it from the WebWidget API. The WebFrameWidgetImpl
is only used for child frames, and during Initialize(), it will set its
background color to be overridden to transparent, preventing any future
colors to matter, unless the override was cleared. But since clearing
overrides only happens by IPC to the main frame widget (now to the
RenderView) that clearing can't happen. So remove all background color
code from WebFrameWidgetImpl, and just set the compositor's background
color to transparent during startup.

Android WebView and ChromeCast code was also setting the base background
color via the WebFrameWidget, but both are in code that corresponds to a
view, not a widget, so pass the value to WebViewImpl directly
instead by adding SetBaseBackgroundColor to the WebView API. Then we
remove all background color APIs from WebFrameWidget as well.

R=ajwong@chromium.org, avi@chromium.org, dcheng@chromium.org
TBR=dcheng

Change-Id: I3d83064d13836f286f8ebc89118b990bbdbb80d3
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1382861
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Luke Halliwell <halliwell@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618653}
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/android_webview/renderer/aw_render_frame_ext.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/chromecast/renderer/cast_content_renderer_client.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_view_host_impl.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_view_host_impl.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_view_host_unittest.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_widget_host_owner_delegate.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_widget_host_view_child_frame.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/browser/renderer_host/render_widget_host_view_mac_unittest.mm
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/common/view_messages.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/common/widget_messages.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/renderer/render_view_impl.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/renderer/render_view_impl.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/renderer/render_widget.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/renderer/render_widget.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/test/BUILD.gn
[add] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/test/stub_render_widget_host_owner_delegate.cc
[add] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/content/test/stub_render_widget_host_owner_delegate.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/third_party/blink/public/web/web_frame_widget.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/third_party/blink/public/web/web_view.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/third_party/blink/public/web/web_widget.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/third_party/blink/renderer/core/exported/web_view_test.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/third_party/blink/renderer/core/frame/web_frame_widget_impl.h
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/third_party/blink/renderer/core/frame/web_view_frame_widget.cc
[modify] https://crrev.com/d51fd53d74e3db5f790bf3f226fb9ba02c39bd8a/third_party/blink/renderer/core/frame/web_view_frame_widget.h

Comment 25 by bugdroid1@chromium.org, Dec 21

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1eb4333cdffa000ccdedc8570259a4125a7b4f0c

commit 1eb4333cdffa000ccdedc8570259a4125a7b4f0c
Author: danakj <danakj@chromium.org>
Date: Fri Dec 21 23:29:15 2018

Add structs to WebViewImpl to hold members.

One struct holds things that will stay in the eventual WebViewImpl, and
the other holds things that will move to WebWidgetImpl once they are
split.

This will allow us to DCHECK that you're not inside a WebWidget API call
when you access ViewData, or vice versa. And it makes users clear within
the WebViewImpl class which one they are acting on, as we make a
decision where each variable belongs. It also allows us to do that
incrementally.

R=dcheng@chromium.org

Change-Id: I255cf0de838928e3f88c805563017702067e0d48
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1388920
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618666}
[modify] https://crrev.com/1eb4333cdffa000ccdedc8570259a4125a7b4f0c/third_party/blink/public/web/web_view_client.h
[modify] https://crrev.com/1eb4333cdffa000ccdedc8570259a4125a7b4f0c/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/1eb4333cdffa000ccdedc8570259a4125a7b4f0c/third_party/blink/renderer/core/exported/web_view_impl.h

Comment 26 by bugdroid1@chromium.org, Jan 7

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5b84fad446f70aa6d48b0e3047c4388e2a8d638c

commit 5b84fad446f70aa6d48b0e3047c4388e2a8d638c
Author: danakj <danakj@chromium.org>
Date: Mon Jan 07 18:12:42 2019

Remove GetRenderViewHost() from the RenderWidgetHostOwnerDelegate.

For each caller of GetRenderViewHost(), which used to be a caller of
RenderViewHost::From(RenderWidgetHost*), add an appropriate method to
the owner delegate and call that directly instead.

Mostly this is pretty straightforward, and the code that called
GetRenderViewHost() to get something moves into RenderViewHostImpl (as
the impl of the owner delegate) which returns that something to the
RenderWidgetHost code.

One exception is that on mac we punch a hole in DEPS to allow use of
WebContents directly from the RenderWidgetHostView. In order to not
extend DEPS of web_contents.h beyond those files into the
RenderViewHostImpl implementation, we
a) Put GetWebContentsForWidget() behind #ifdef OS_MACOSX so the API
does not exist elsewhere.
b) Put the implementation of this into its own .cc file, which has
the appropriate DEPS hole punching applied to it.

This is all pretty sad looking to me, but it seems that the
RenderWidgetHostViewMac code should really be going through a delegate
and it is not, and this is the result.

R=asvitkine@chromium.org, avi@chromium.org
TBR=dcheng

Change-Id: I9a812381126f049a2f52e420bddde9d00e72f028
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1385467
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620379}
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/browser/renderer_host/render_view_host_impl.cc
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/browser/renderer_host/render_view_host_impl.h
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/browser/renderer_host/render_widget_host_impl.h
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/browser/renderer_host/render_widget_host_owner_delegate.h
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/browser/renderer_host/render_widget_host_view_cocoa.mm
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/browser/renderer_host/render_widget_host_view_mac.h
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/browser/renderer_host/render_widget_host_view_mac.mm
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/common/render_widget_host_ns_view.mojom
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/test/stub_render_widget_host_owner_delegate.cc
[modify] https://crrev.com/5b84fad446f70aa6d48b0e3047c4388e2a8d638c/content/test/stub_render_widget_host_owner_delegate.h

Comment 27 by bugdroid1@chromium.org, Jan 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1ff1e7479fb4b4cf22c684d5663567ef5def75f

commit a1ff1e7479fb4b4cf22c684d5663567ef5def75f
Author: danakj <danakj@chromium.org>
Date: Tue Jan 08 16:27:34 2019

Remove WebViewClient::WidgetClient()

There need not be a WebWidget{Client} for a WebView in the future, so
we must ensure code that is for views does not rely on there being a
widget. To aid this you can no longer get a WebWidgetClient from a
WebViewClient.

Most of this change is for frame_test_helpers to pass in these two
things separately again. And then move the LayerTreeView from the
TestWebViewClient to the TestWebWidgetClient.

R=dcheng@chromium.org
TBR=avi

Change-Id: I9cac9dae2ca8abc98a71c603bdacdbf62129a222
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1389320
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620748}
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/components/plugins/renderer/webview_plugin.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/components/plugins/renderer/webview_plugin.h
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/components/printing/renderer/print_render_frame_helper.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/content/renderer/render_view_impl.h
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/content/shell/test_runner/web_view_test_client.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/content/shell/test_runner/web_view_test_client.h
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/content/shell/test_runner/web_view_test_proxy.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/content/shell/test_runner/web_view_test_proxy.h
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/public/web/web_view_client.h
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/css/style_engine_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/exported/web_layer_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/exported/web_view_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/frame/browser_controls_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/frame/frame_serializer_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/frame/frame_test_helpers.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/frame/frame_test_helpers.h
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/frame/visual_viewport_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/input/ime_on_focus_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/input/touch_action_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/layout/scrollbars_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/page/scrolling/main_thread_scrolling_reasons_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/page/viewport_test.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/testing/sim/sim_compositor.cc
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/testing/sim/sim_compositor.h
[modify] https://crrev.com/a1ff1e7479fb4b4cf22c684d5663567ef5def75f/third_party/blink/renderer/core/testing/sim/sim_test.cc

Comment 28 by bugdroid1@chromium.org, Jan 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d0928c6372f3d55866f4c0822a783c11c0b9004e

commit d0928c6372f3d55866f4c0822a783c11c0b9004e
Author: danakj <danakj@chromium.org>
Date: Tue Jan 08 16:28:13 2019

Remove PaintContent's Page dependency and PaintContentIgnoringCompositing

PaintContentIgnoringCompositing is no longer used so remove it.

Meanwhile PaintContent was passing along the Page in order to get a
deprecated device scale from it. Instead grab the scale the recommended
way from the LocalFrameView.

R=dcheng@chromium.org

Change-Id: I83def77e5b58d2ef8eff337eb8817086208df770
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1399519
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620749}
[modify] https://crrev.com/d0928c6372f3d55866f4c0822a783c11c0b9004e/third_party/blink/public/web/web_widget.h
[modify] https://crrev.com/d0928c6372f3d55866f4c0822a783c11c0b9004e/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/d0928c6372f3d55866f4c0822a783c11c0b9004e/third_party/blink/renderer/core/exported/web_page_popup_impl.cc
[modify] https://crrev.com/d0928c6372f3d55866f4c0822a783c11c0b9004e/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/d0928c6372f3d55866f4c0822a783c11c0b9004e/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/d0928c6372f3d55866f4c0822a783c11c0b9004e/third_party/blink/renderer/core/page/page_widget_delegate.cc
[modify] https://crrev.com/d0928c6372f3d55866f4c0822a783c11c0b9004e/third_party/blink/renderer/core/page/page_widget_delegate.h

Comment 29 by danakj@chromium.org, Jan 8

Description: Show this description

Comment 30 by bugdroid1@chromium.org, Jan 9

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f456aeefda399d09d0641f7670ebeabdad24e609

commit f456aeefda399d09d0641f7670ebeabdad24e609
Author: danakj <danakj@chromium.org>
Date: Wed Jan 09 20:34:46 2019

Move Page into the WebView portion of WebViewImpl.

Accessors of it grab it through AsView(). After this we will start to
add guards to prevent AsView() calls inside WebWidget API methods.

1st open question is if WebWidget APIs should be able to access the
main frame and if so if it should be through the WebView/Page or
elseways.

2nd open question is if WebView APIs using the Page should be allowed
to touch the frames (and frame views) at all.

R=dcheng@chromium.org

Change-Id: I141e5660d48ae5b3227aed8525e6f6999c6f6413
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1399742
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621292}
[modify] https://crrev.com/f456aeefda399d09d0641f7670ebeabdad24e609/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/f456aeefda399d09d0641f7670ebeabdad24e609/third_party/blink/renderer/core/exported/web_view_impl.h

Comment 31 by bugdroid1@chromium.org, Jan 11

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/505476798f697b46fad4b8e7572b23e7e4dfeb67

commit 505476798f697b46fad4b8e7572b23e7e4dfeb67
Author: danakj <danakj@chromium.org>
Date: Fri Jan 11 23:53:46 2019

Set the display mode from the CreateViewParams not from RenderWidget.

We want to not use the RenderWidget before it's initialized, and we
want to move initialization to where there is a main frame already
(or part of making the main frame). So stop using the RenderWidget
to grab the display mode when setting it on the WebView, since we
have it in the CreateViewParams anyways.

R=avi@chromium.org

Change-Id: Ia9d5a1ecf9dc222a662bfc74de94764f50143ab3
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1407635
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622226}
[modify] https://crrev.com/505476798f697b46fad4b8e7572b23e7e4dfeb67/content/renderer/render_view_impl.cc

Comment 32 by bugdroid1@chromium.org, Jan 16

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed946461469e9dc33302b1b7e9ebe69aeaac0f84

commit ed946461469e9dc33302b1b7e9ebe69aeaac0f84
Author: danakj <danakj@chromium.org>
Date: Wed Jan 16 00:00:59 2019

Move RenderViewImpl code that configure WebSettings out to helper method

There's a bunch of code to parse CommandLine flags and set values on
WebSettings when initializing the RenderViewImpl and the WebView. To
make things more clear, put all of these out to a helper method that
accepts a WebSettings* so it's clear no use of WebView itself (or use
of the RenderWidget) is happening in them.

R=avi@chromium.org

Change-Id: I8dcf9d683c401fcdea5f814808dba795179f2254
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1407444
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622888}
[modify] https://crrev.com/ed946461469e9dc33302b1b7e9ebe69aeaac0f84/content/renderer/render_view_impl.cc

Comment 33 by bugdroid1@chromium.org, Jan 16

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed946461469e9dc33302b1b7e9ebe69aeaac0f84

commit ed946461469e9dc33302b1b7e9ebe69aeaac0f84
Author: danakj <danakj@chromium.org>
Date: Wed Jan 16 00:00:59 2019

Move RenderViewImpl code that configure WebSettings out to helper method

There's a bunch of code to parse CommandLine flags and set values on
WebSettings when initializing the RenderViewImpl and the WebView. To
make things more clear, put all of these out to a helper method that
accepts a WebSettings* so it's clear no use of WebView itself (or use
of the RenderWidget) is happening in them.

R=avi@chromium.org

Change-Id: I8dcf9d683c401fcdea5f814808dba795179f2254
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1407444
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622888}
[modify] https://crrev.com/ed946461469e9dc33302b1b7e9ebe69aeaac0f84/content/renderer/render_view_impl.cc

Comment 34 by bugdroid1@chromium.org, Jan 16

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/284d557d42d9179990b260046c5a25e1ffd25277

commit 284d557d42d9179990b260046c5a25e1ffd25277
Author: danakj <danakj@chromium.org>
Date: Wed Jan 16 17:00:26 2019

Split WebWidgetClient setting out of WebView::Create().

Add WebView::SetWebWidgetClient() that is called before setting up the
main frame and attaching it to the view. TODOs are sprinkled around as
what we should do in follow-ups.

Callers other than RenderViewImpl move the SetWebWidgetClient to be
called after initializing the WebView, but before making the main
frame. Next should be to remove SetWebWidgetClient calls from all
these places and move it to where RenderViewImpl attaches the
WebFrameWidget to the RenderWidget.

Then, RenderWidget::Init() should be moved to be after the attachment
of the main frame, and at that point RenderWidget will always have a
WebFrameWidget (instead of WebView as a WebWidget), except when the
RenderWidget is frozen (the main frame is remote).

Last, we then want to remove the SetWebWidgetClient calls (or reset
it to null) when the main frame is remote, but CloseWidgetSoon goes
through WebWidgetClient and should go through WebViewClient instead,
so a TODO is added. There may be more cases like this.

R=dcheng@chromium.org
TBR=dcheng

Change-Id: Id6b9d85501c4bf5a3cdfc3106b6bb18656a7c11f
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1406195
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623264}
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/components/plugins/renderer/webview_plugin.cc
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/components/printing/renderer/print_render_frame_helper.cc
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/content/renderer/render_view_impl.cc
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/extensions/renderer/scoped_web_frame.cc
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/media/blink/webmediaplayer_impl_unittest.cc
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/third_party/blink/public/web/web_view.h
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/third_party/blink/public/web/web_widget_client.h
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/third_party/blink/renderer/core/exported/web_view_test.cc
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/third_party/blink/renderer/core/exported/worker_shadow_page.cc
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/third_party/blink/renderer/core/frame/frame_test_helpers.cc
[modify] https://crrev.com/284d557d42d9179990b260046c5a25e1ffd25277/third_party/blink/renderer/core/frame/frame_test_helpers.h

Comment 35 by bugdroid1@chromium.org, Jan 16

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/efcb843fdd5fefaf4f0c106f04f308ed1aba7919

commit efcb843fdd5fefaf4f0c106f04f308ed1aba7919
Author: danakj <danakj@chromium.org>
Date: Wed Jan 16 18:47:14 2019

Don't use WebView as a proxy to show FPS counter in the compositor.

1. Move the API from WebLayerTreeView to WebWidgetClient
2. Have RenderViewImpl set the value each time a new WebFrameWidget
is attached, as in the future each one will have a new WebWidgetClient.
3. Have inspector go through the WebFrameWidget's client instead of
through WebView (and WebLayerTreeView).

Also removes the WebSetting for this, which was not used before this
CL.

R=enne@chromium.org

Change-Id: Ide83b5372033574cb965e973d50ac43d44f2e8fa
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1407633
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623313}
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/content/renderer/compositor/layer_tree_view.cc
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/content/renderer/compositor/layer_tree_view.h
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/content/renderer/render_view_impl.cc
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/content/renderer/render_widget.cc
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/content/renderer/render_widget.h
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/third_party/blink/public/platform/web_layer_tree_view.h
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/third_party/blink/public/web/web_settings.h
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/third_party/blink/public/web/web_view.h
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/third_party/blink/public/web/web_widget_client.h
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/third_party/blink/renderer/core/exported/web_settings_impl.cc
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/third_party/blink/renderer/core/exported/web_settings_impl.h
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/efcb843fdd5fefaf4f0c106f04f308ed1aba7919/third_party/blink/renderer/core/inspector/inspector_overlay_agent.cc

Comment 36 by bugdroid1@chromium.org, Jan 16

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b08b371a00245bc09c6d0bb6189289fe8da218fb

commit b08b371a00245bc09c6d0bb6189289fe8da218fb
Author: danakj <danakj@chromium.org>
Date: Wed Jan 16 18:59:56 2019

Set the WebWidgetClient from WebViewFrameWidget.

When making a WebViewFrameWidget have it set the WebWidgetClient on the
WebViewImpl. This delays setting a client until the main frame is
created.

Conservatively, if making a remote main frame, have RenderViewImpl
explictly set the WebWidgetClient on the WebViewImpl directly even
though there's no WebFrameClient. We'll work to remove that as there
should be no WebWidgetClient when there's no local main frame and
WebWidget.

When the WebViewFrameWidget closes it removes the WebWidgetClient,
however to be conservative.. RenderViewImpl immediately puts the
WebWidgetClient back similar to the remote main frame case during
init.

R=dcheng@chromium.org
TBR=dcheng

Change-Id: I4b0f04a2d5c74d012f5f71b4b72fc573cef45f8c
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1407630
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623315}
[modify] https://crrev.com/b08b371a00245bc09c6d0bb6189289fe8da218fb/components/plugins/renderer/webview_plugin.cc
[modify] https://crrev.com/b08b371a00245bc09c6d0bb6189289fe8da218fb/components/printing/renderer/print_render_frame_helper.cc
[modify] https://crrev.com/b08b371a00245bc09c6d0bb6189289fe8da218fb/content/public/test/render_view_test.cc
[modify] https://crrev.com/b08b371a00245bc09c6d0bb6189289fe8da218fb/content/renderer/render_view_browsertest.cc
[modify] https://crrev.com/b08b371a00245bc09c6d0bb6189289fe8da218fb/content/renderer/render_view_impl.cc
[modify] https://crrev.com/b08b371a00245bc09c6d0bb6189289fe8da218fb/third_party/blink/public/web/web_view.h
[modify] https://crrev.com/b08b371a00245bc09c6d0bb6189289fe8da218fb/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/b08b371a00245bc09c6d0bb6189289fe8da218fb/third_party/blink/renderer/core/frame/frame_test_helpers.cc
[modify] https://crrev.com/b08b371a00245bc09c6d0bb6189289fe8da218fb/third_party/blink/renderer/core/frame/web_view_frame_widget.cc

Comment 37 by bugdroid1@chromium.org, Jan 16

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e691e85e0e55cee7518dc8a8b23c1981705f3e9e

commit e691e85e0e55cee7518dc8a8b23c1981705f3e9e
Author: danakj <danakj@chromium.org>
Date: Wed Jan 16 22:49:04 2019

Stop going through WebView to set debug flags on the compositor

Instead of going through WebView (which may or may not have a main
frame being composited), go through the client of the frame's
WebFrameWidget.

For each debug property:
1. Move the API from WebLayerTreeView to WebWidgetClient
2. Have inspector go through the WebFrameWidget's client instead of
through WebView (and WebLayerTreeView).

Also removes the WebSetting for these, which were not used before this
CL.

R=avi@chromium.org, enne@chromium.org, pdr@chromium.org

Change-Id: I7c615878fe7f481d589548470d3c8878cf0b36e9
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1407438
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623417}
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/content/renderer/compositor/layer_tree_view.cc
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/content/renderer/compositor/layer_tree_view.h
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/content/renderer/render_widget.cc
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/content/renderer/render_widget.h
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/third_party/blink/public/platform/web_layer_tree_view.h
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/third_party/blink/public/web/web_settings.h
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/third_party/blink/public/web/web_view.h
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/third_party/blink/public/web/web_widget_client.h
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/third_party/blink/renderer/core/exported/web_settings_impl.cc
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/third_party/blink/renderer/core/exported/web_settings_impl.h
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/e691e85e0e55cee7518dc8a8b23c1981705f3e9e/third_party/blink/renderer/core/inspector/inspector_overlay_agent.cc

Comment 38 by bugdroid1@chromium.org, Jan 18

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ff8b4bf8d5ecb2c1c4c647302cb4ddaa575a5135

commit ff8b4bf8d5ecb2c1c4c647302cb4ddaa575a5135
Author: danakj <danakj@chromium.org>
Date: Fri Jan 18 20:52:37 2019

Remove gpu raster statefulness from WebViewImpl

This state only exists for unit tests, and they can just look at the
side effects on LayerTreeHost directly. This removes a widget-only
state from WebViewImpl, so we don't have to worry about setting it
correctly if it changed while the RenderWidget was frozen.

R=enne@chromium.org

Change-Id: I360da46f62f7754e4b79af33ff09f0e115051863
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1418837
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624272}
[modify] https://crrev.com/ff8b4bf8d5ecb2c1c4c647302cb4ddaa575a5135/cc/trees/layer_tree_host.h
[modify] https://crrev.com/ff8b4bf8d5ecb2c1c4c647302cb4ddaa575a5135/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/ff8b4bf8d5ecb2c1c4c647302cb4ddaa575a5135/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/ff8b4bf8d5ecb2c1c4c647302cb4ddaa575a5135/third_party/blink/renderer/core/page/viewport_test.cc

Comment 39 by bugdroid1@chromium.org, Jan 21

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/84bbf551ed771ef0f5a3b6924442804ccb21a95c

commit 84bbf551ed771ef0f5a3b6924442804ccb21a95c
Author: danakj <danakj@chromium.org>
Date: Mon Jan 21 21:33:31 2019

Rename ApplyNewSizeForWidget() to CancelPagePopupForWidget()

Make the API clear about it's intent, and only call it from RenderWidget
when the size changes, so the RenderView code is now just a pass-through
to the WebView.

R=avi@chromium.org

Change-Id: I6941e7d6bc152b09d8bf47ab392a312b67c215d6
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1426082
Commit-Queue: danakj <danakj@chromium.org>
Commit-Queue: Avi Drissman <avi@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Auto-Submit: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624679}
[modify] https://crrev.com/84bbf551ed771ef0f5a3b6924442804ccb21a95c/content/renderer/render_view_impl.cc
[modify] https://crrev.com/84bbf551ed771ef0f5a3b6924442804ccb21a95c/content/renderer/render_view_impl.h
[modify] https://crrev.com/84bbf551ed771ef0f5a3b6924442804ccb21a95c/content/renderer/render_widget.cc
[modify] https://crrev.com/84bbf551ed771ef0f5a3b6924442804ccb21a95c/content/renderer/render_widget_delegate.h
[modify] https://crrev.com/84bbf551ed771ef0f5a3b6924442804ccb21a95c/content/renderer/render_widget_unittest.cc

Comment 40 by bugdroid, Jan 23

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/20575e91a5024839400ccef1377c8ae147fcd96f

commit 20575e91a5024839400ccef1377c8ae147fcd96f
Author: danakj <danakj@chromium.org>
Date: Wed Jan 23 16:25:01 2019

Remove nullcheck from RenderWidget::SynchronizeVisualProperties

Sync visual properties is only called in response to an IPC. The close
IPC drops the IPC route before removing the WebWidget so IPCs can't
arrive that would SynchronizeVisualProperties() after the WebWidget is
gone.

The one exception is the screen metrics emulator which can call it from
its destructor, but it is also destroyed in CloseWebWidget() before the
WebWidget is destroyed precisely to avoid these sorts of things (with
a TODO about how to make that more clear..).

R=avi@chromium.org

Change-Id: I0621f4e3f4a708dd209b996129b9e17a765d6296
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1427881
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625219}

Comment 41 by bugdroid, Jan 23

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f928477b86e742995c5cd97c52178c592f72254d

commit f928477b86e742995c5cd97c52178c592f72254d
Author: danakj <danakj@chromium.org>
Date: Wed Jan 23 18:29:14 2019

Decouple fullscreen and widget/visual properties a bit.

Change fullscreen to be set through a setter method when receiving the
visual properties. This would allow tests to set it directly without
needing a full visual properties.

Remove a GetWebWidget() early out since visual properties only arrive
and are processed before the RenderWidget is closed.

Left a TODO to move it to the RenderView since it is a view-based
property on both the browser and renderer side.

Change-Id: Iecad0bd090b4c19aa260f6755c4b56e4232e0e63
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1425917
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Auto-Submit: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625270}

Comment 42 by bugdroid, Jan 23

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/30ad91688b7a9c9976cf00ead80a294157ff1796

commit 30ad91688b7a9c9976cf00ead80a294157ff1796
Author: danakj <danakj@chromium.org>
Date: Wed Jan 23 23:10:21 2019

Remove Redraw call from RenderWidgetScreenMetricsEmulator.

This redraw used to be

  if (params.needs_resize_ack)
    delegate_->Redraw();

Which was there to cause a paint (pre compositing) or a composite (post
compositing) in order to send an ack after a resize. We no longer need
to make this redraw happen to get the appropriate ACK as
664698a679a27ce4e converted us to ACKing to SynchronizeVisualProperties
(which replaced Resize) directly, and now ACKing is tied to surface
synchronization and local surface IDs.

R=avi@chromium.org

Change-Id: Id1c0e3e5534b41767ce36fb2298af56a639fa677
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1431213
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625398}

Comment 43 by bugdroid, Jan 23

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0246bba0cd7b7d6616e0aeac4a3f38464e9af232

commit 0246bba0cd7b7d6616e0aeac4a3f38464e9af232
Author: danakj <danakj@chromium.org>
Date: Wed Jan 23 23:12:27 2019

Move page scale factor out of RenderWidget::SynchronizeVisualProperties.

The SynchronizeVisualProperties() method is called from the IPC handler
to apply some of the IPC message. The code for page scale factor is
moved out to the IPC handler to act on it directly.

It is also called by a number of methods for tests, which apply various
properties to the RenderWidget. Page scale is not ever changed by these
test methods, so they do not depend on it being inside
SynchronizeVisualProperties().

It is also also called from the screen metrics emulator, but since the
emulator does not change the page scale factor, it also does not depend
on this code being in SynchronizeVisualProperties().

The notification of the page scale factor change to each
RenderFrameProxy is dramatically reordered with respect to the other
properties of RenderWidget changing. This is okay because the
notification is only to pass the value over IPC up to the browser and
down to the child RenderWidget, so it does not depend on the ordering.

This reduces the test-only method dependency footprint on the
VisualProperties structure. Left a TODO about changing the routing
of this page scale property to go

blink->RenderView->RenderViewHost->{All RenderWidgets}

instead of the current

blink->RenderView->RenderWidget->
  {All RenderFrameProxy->RenderWidgetHost->RenderWidgets}

Other page-level properties could benefit from this path as well
but are much trickier since they
a) do not go through RenderFrameProxy today and instead seem to
propagate through the browser code, which is good, except
b) are treated identically in child RenderWidgets as in the main
frame,

whereas the page scale factor has clear behaviour differences for
child RenderWidgets. So this could be useful for exploring and
establishing this pattern.

R=ajwong@chromium.org
TBR=avi

Change-Id: I4af9b114e729c26e8356e69ecb9569337d653cb3
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1427466
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625401}

Comment 44 by bugdroid, Jan 24

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2ae8e4805cf3528619e05806e836a78d2c740e0b

commit 2ae8e4805cf3528619e05806e836a78d2c740e0b
Author: danakj <danakj@chromium.org>
Date: Thu Jan 24 01:32:09 2019

Remove conditional use of LayerTreeView from RenderWidget.

The LayerTreeView is always present since we always composite
RenderWidgets now (for many years ^_^).

However, the LayerTreeView *is* removed during shutdown. Most places
will not need to worry about this because OnClose() drops the IPC
channel and most uses of LayerTreeView are in response to other IPCs.

However a few cases involve calls from Blink which could still be
active in between OnClose() and tearing down the RenderWidget+Blink,
so in these cases early out if |closing_| which represents better
conceptually what we're testing.

TBR=avi@chromium.org

Change-Id: Iff9133d9c69b327495f9290afcb471739124018b
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1432132
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625473}

Comment 45 by bugdroid, Jan 24

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bef2b64696776bdac2ea14124da97a7e3b2c91da

commit bef2b64696776bdac2ea14124da97a7e3b2c91da
Author: Kinuko Yasuda <kinuko@chromium.org>
Date: Thu Jan 24 04:53:33 2019

Revert "Remove conditional use of LayerTreeView from RenderWidget."

This reverts commit 2ae8e4805cf3528619e05806e836a78d2c740e0b.

Reason for revert: some layout tests started to crash on CHECK(closing_)

https://test-results.appspot.com/data/layout_results/Mac10_13_Tests/9201/webkit_layout_tests/layout-test-results/results.html

 external/wpt/trusted-types/block-string-assignment-to-Window-open.tentative.html
 external/wpt/url/failure.html

Original change's description:
> Remove conditional use of LayerTreeView from RenderWidget.
> 
> The LayerTreeView is always present since we always composite
> RenderWidgets now (for many years ^_^).
> 
> However, the LayerTreeView *is* removed during shutdown. Most places
> will not need to worry about this because OnClose() drops the IPC
> channel and most uses of LayerTreeView are in response to other IPCs.
> 
> However a few cases involve calls from Blink which could still be
> active in between OnClose() and tearing down the RenderWidget+Blink,
> so in these cases early out if |closing_| which represents better
> conceptually what we're testing.
> 
> TBR=avi@chromium.org
> 
> Change-Id: Iff9133d9c69b327495f9290afcb471739124018b
> Bug: 912193
> Reviewed-on: https://chromium-review.googlesource.com/c/1432132
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: Albert J. Wong <ajwong@chromium.org>
> Commit-Queue: danakj <danakj@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#625473}

TBR=avi@chromium.org,ajwong@chromium.org,danakj@chromium.org

Change-Id: Ide47f156f8f5d2af32f7442bb659599356c3f6c3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1433379
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Commit-Queue: Kinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625506}

Comment 46 by bugdroid, Jan 24

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/125a29e7472bde5781656cee767ccb21603d95cb

commit 125a29e7472bde5781656cee767ccb21603d95cb
Author: danakj <danakj@chromium.org>
Date: Thu Jan 24 23:25:29 2019

Reland "Remove conditional use of LayerTreeView from RenderWidget."

This is a reland of 2ae8e4805cf3528619e05806e836a78d2c740e0b

We found that RenderWidget::DidNavigate can occur after OnClose()
runs, so we should early out if |closing_| at that point.

content::RenderWidget::DidNavigate() + 68
content::RenderFrameImpl::DidCommitProvisionalLoad(blink::WebHistoryItem const&, blink::WebHistoryCommitType, mojo::ScopedHandleBase<mojo::MessagePipeHandle>) + 301
blink::LocalFrameClientImpl::DispatchDidCommitLoad(blink::HistoryItem*, blink::WebHistoryCommitType, blink::WebGlobalObjectReusePolicy) + 265
blink::DocumentLoader::DidCommitNavigation(blink::WebGlobalObjectReusePolicy) + 268
blink::DocumentLoader::InstallNewDocument(blink::KURL const&, scoped_refptr<blink::SecurityOrigin const>, blink::Document*, blink::WebGlobalObjectReusePolicy, WTF::AtomicString const&, WTF::AtomicString const&, blink::DocumentLoader::InstallNewDocumentReason, blink::ParserSynchronizationPolicy, blink::KURL const&) + 691
blink::DocumentLoader::CommitNavigation(WTF::AtomicString const&, blink::KURL const&) + 446
blink::DocumentLoader::CommitData(char const*, unsigned long) + 70
blink::DocumentLoader::DataReceived(blink::Resource*, char const*, unsigned long) + 107
blink::Resource::NotifyDataReceived(char const*, unsigned long) + 118
blink::Resource::AppendData(char const*, unsigned long) + 70
blink::ResourceLoader::DidReceiveData(char const*, int) + 164
content::WebURLLoaderImpl::Context::OnReceivedData(std::__1::unique_ptr<content::RequestPeer::ReceivedData, std::__1::default_delete<content::RequestPeer::ReceivedData> >) + 87
content::WebURLLoaderImpl::RequestPeerImpl::OnReceivedData(std::__1::unique_ptr<content::RequestPeer::ReceivedData, std::__1::default_delete<content::RequestPeer::ReceivedData> >) + 42Original change's description:

Also don't remove the MainThreadEventQueue client until the RenderWidget
is destroyed to avoid dropping the mojo reply callback.

> Remove conditional use of LayerTreeView from RenderWidget.
>
> The LayerTreeView is always present since we always composite
> RenderWidgets now (for many years ^_^).
>
> However, the LayerTreeView *is* removed during shutdown. Most places
> will not need to worry about this because OnClose() drops the IPC
> channel and most uses of LayerTreeView are in response to other IPCs.
>
> However a few cases involve calls from Blink which could still be
> active in between OnClose() and tearing down the RenderWidget+Blink,
> so in these cases early out if |closing_| which represents better
> conceptually what we're testing.
>
> TBR=avi@chromium.org
>
> Change-Id: Iff9133d9c69b327495f9290afcb471739124018b
> Bug: 912193
> Reviewed-on: https://chromium-review.googlesource.com/c/1432132
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: Albert J. Wong <ajwong@chromium.org>
> Commit-Queue: danakj <danakj@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#625473}

Bug: 912193
Change-Id: I148c56da1aa682daa84ccdaef3f836514f744e01
Reviewed-on: https://chromium-review.googlesource.com/c/1434975
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625865}

Comment 47 by bugdroid, Jan 25

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/04d128a63ae6cc270749a0069aaea89790463635

commit 04d128a63ae6cc270749a0069aaea89790463635
Author: danakj <danakj@chromium.org>
Date: Fri Jan 25 02:10:20 2019

Update color space in tests without SynchronizeVisualProperties.

Remove the requirement to store and load all the possible visual
properties in order to change the color space in RenderWidget.

R=avi@chromium.org

Change-Id: Ie37a76f356de43ade7c5e61a32d64de8574719a9
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1433072
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#625947}

Comment 48 by bugdroid, Jan 26

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/327a4fe16110817e02d1a564ec63ca38908678cc

commit 327a4fe16110817e02d1a564ec63ca38908678cc
Author: danakj <danakj@chromium.org>
Date: Sat Jan 26 00:00:35 2019

Push event handling failures up the stack, clear the client early.

Have RenderWidget clear itself as the MainThreadEventQueue client when
it is closing, in OnClose(). The queue already deals with trying to
handle an event when the client is gone.

However, it does not deal with the fact that when not handling the event
it must still run the mojo reply callback or suffer the wrath of a
DCHECK.

RenderWidget does deal with this fact already. But instead of dealing
with this callback behaviour at multiple levels, have the handle
methods return a boolean back up to Dispatch(), where if it gets
false it can run HandledEvent() with the failure args from a single
place.

Remove the closing early outs for MainThreadEventQueue methods in
RenderWidget.

R=bokan@chromium.org

Change-Id: I0f836ed4df801ce470afb10894c9c3caa70c5711
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1435520
Reviewed-by: Navid Zolghadr <nzolghadr@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: David Bokan <bokan@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626269}

Comment 49 by bugdroid, Jan 28

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/01474dd60d44f38ffc079e9180a5d3c092eb22f4

commit 01474dd60d44f38ffc079e9180a5d3c092eb22f4
Author: danakj <danakj@chromium.org>
Date: Mon Jan 28 10:46:04 2019

Clarify use of device_scale_factor_for_testing_.

Instead of applying it after using the device scale factor, move the
application to the top of OnSynchronizeVisualProperties() so that it
is used unconditionally and throughout the entire method.

R=dcheng@chromium.org

Change-Id: Icf3fc4b5af3141671f49bf35916469e98cec06c8
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1436490
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626488}

Comment 50 by bugdroid, Jan 30

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4cec1010bcd67c50c662b12aa3fef1370d98535

commit b4cec1010bcd67c50c662b12aa3fef1370d98535
Author: danakj <danakj@chromium.org>
Date: Wed Jan 30 19:22:13 2019

Update device scale factor in RenderWidget from web tests directly.

Instead of faking a SynchronizeVisualProperties IPC, which requires
building a full VisualProperties structure, have the test-only method to
set the device scale factor set the various values on RenderWidget and
WebWidget directly.

This avoids up needing to store unrelated VisualProperties on the
RenderWidget just to build the structure for web tests.

R=ajwong@chromium.org

Change-Id: I554ba43d744be5302462dfac3fc8f5c49b3ae649
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1437521
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627531}
[modify] https://crrev.com/b4cec1010bcd67c50c662b12aa3fef1370d98535/content/renderer/render_widget.cc
[modify] https://crrev.com/b4cec1010bcd67c50c662b12aa3fef1370d98535/content/renderer/render_widget.h

Comment 51 by bugdroid, Jan 30

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/97b87d124d1ce3e64bf18ffdb6be24a3bdd09ef8

commit 97b87d124d1ce3e64bf18ffdb6be24a3bdd09ef8
Author: Daniel Cheng <dcheng@chromium.org>
Date: Wed Jan 30 22:01:03 2019

Prepare page scale code for compositor being removed when frozen.

When the main frame is remote and the RenderWidget is frozen, we'd like
blink to have its WebWidgetClient and LayerTreeView removed so in the
future the RenderWidget can be destroyed.

This recovers from freezing by setting the page scale factor on the
"new" compositor. We also clean up the page scale factor code with
naming changes.

The setting of page scale was weirdly tied to DidInvalidateRect in
the WebWidgetClient for some historical reasons, and we detangle this.
a) When layout says DidInvalidateRect() for WebViewPlugin, we don't need
to update page scale, since it's not related and WebViewPlugin isn't
composited and thus doesn't use page scale.
b) When updating page scale we don't need to also DidInvalidateRect()
on the WebWidgetClient. This does nothing for RenderWidget anymore, and
we already call it each frame during layout for WebViewPlugin elsewhere.

Oh and some dead code removal in RenderViewImpl cuz why not.

R=dcheng@chromium.org

Change-Id: Ieab66be71a5ae98108244e16f5efdf45d8693d75
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1416450
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#627608}
[modify] https://crrev.com/97b87d124d1ce3e64bf18ffdb6be24a3bdd09ef8/content/renderer/render_view_impl.cc
[modify] https://crrev.com/97b87d124d1ce3e64bf18ffdb6be24a3bdd09ef8/content/renderer/render_view_impl.h
[modify] https://crrev.com/97b87d124d1ce3e64bf18ffdb6be24a3bdd09ef8/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/97b87d124d1ce3e64bf18ffdb6be24a3bdd09ef8/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/97b87d124d1ce3e64bf18ffdb6be24a3bdd09ef8/third_party/blink/renderer/core/frame/frame_test_helpers.cc

Comment 52 by bugdroid, Jan 31

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c000f9169e1071a5ff646983c22e8ca5e326c527

commit c000f9169e1071a5ff646983c22e8ca5e326c527
Author: danakj <danakj@chromium.org>
Date: Thu Jan 31 17:39:03 2019

Clean up browser controls code in the browser side.

tl;dr:
  Behaviour Before (Aura):
  RWHI->RWHVAura->RWHDelegate->WebContentsImpl->WebContentsDelegate
  Behaviour Before (Android):
  RWHI->RWHVAndroid->RVHDelegateView->WebContentsDelegate and
  RWHI->RWHVAndroid->RVHDelegateView->Interstitials
  Behaviour After (Aura):
  RWHI->RVHDelegateView->WebContentsDelegate
  Behaviour After (Android):
  RWHI->RVHDelegateView->WebContentsDelegate and
  RWHI->RVHDelegateView->Interstitials

  APIs Before:
  RenderWidgetHostDelegate::GetTopControlsHeight()
  RenderViewHostDelegateView::GetTopControlsHeight()
  WebContentsDelegate::GetTopControlsHeight()
  APIs After:
  RenderViewHostDelegateView::GetTopControlsHeight()
  WebContentsDelegate::GetTopControlsHeight()

The RenderViewHostDelegateView API gives the browser controls state.
A pointer to this API is provided by both RenderWidgetHostDelegate and
by RenderViewHostDelegate. These both are implemented in
WebContentsImpl, where the RenderViewHostDelegateView is held.

However RenderWidgetHostDelegate *also* gives the browser controls
state. So we have getters in WebContentsImpl, which redirect to the
WebContentsDelegate. Then (on Android) the RenderViewHostDelegateView
goes back through the WebContentsDelegate to get the value from the
same place as the RenderWidgetHostDelegate getters - except in the
case of android interstitials.

We remove the redundant APIs on WebContentsImpl,
RenderWidgetHostDelegate and RenderViewHostDelegate. And have code
always ask through the RenderViewHostDelegateView.

Secondly, RenderWidgetHostImpl asks the RenderWidgetHostView for the
browser controls state. The Android override goes to the
RenderViewHostDelegateView to return the values. Aura goes through
the RenderWidgetHostDelegate. RenderWidgetHostImpl already has access
to these APIs.

So remove all methods in RenderWidgetHostView to get browser controls
state, and have RenderWidgetHostImpl go directly to its delegate to
get the RenderViewHostDelegateView and get the values. Then have the
platform-specific behaviour via RenderViewHostDelegateView with
Aura and Android-non-interstitials asking the WebContentsDelegate.

Last, there is an if condition around the device scale factor being 0
that should not be possible, so remove it. It was introduced to be
cautious without understanding this can't happen (at least that's my
belief, a 0 DSF would mean all pixels are infinitely small :)).

Working toward making browser control state be a RenderView/WebView
state, since there is only 1 value for the whole page, and in blink
code querying this value is currently broken for OOPIFs (for eg in
FillsViewport() of root_scroller_controller.cc) since it tries to
get these values from the main frame, which would be a proxy. These
values should become clear WebView state instead of per-frame/widget.

R=avi@chromium.org

Change-Id: Ieda5556a32a7a2c8baed1b2291f77ed3665c05a6
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1437796
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628004}
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/renderer_host/render_widget_host_delegate.cc
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/renderer_host/render_widget_host_delegate.h
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/renderer_host/render_widget_host_unittest.cc
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/renderer_host/render_widget_host_view_android.cc
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/renderer_host/render_widget_host_view_android.h
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/renderer_host/render_widget_host_view_aura.cc
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/renderer_host/render_widget_host_view_aura.h
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/renderer_host/render_widget_host_view_base.cc
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/renderer_host/render_widget_host_view_base.h
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/web_contents/web_contents_impl.h
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/web_contents/web_contents_view_aura.cc
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/browser/web_contents/web_contents_view_aura.h
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/test/BUILD.gn
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/test/mock_render_widget_host_delegate.cc
[modify] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/test/mock_render_widget_host_delegate.h
[add] https://crrev.com/c000f9169e1071a5ff646983c22e8ca5e326c527/content/test/stub_render_view_host_delegate_view.h

Comment 53 by bugdroid, Jan 31

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7602f4f4603fb3f992c0deb9594f1f57e40695f8

commit 7602f4f4603fb3f992c0deb9594f1f57e40695f8
Author: danakj <danakj@chromium.org>
Date: Thu Jan 31 18:03:23 2019

Remove ResizingModeSelector and clarify sync resize code.

The sync resize mode, in which the renderer controls a RenderWidget's
size (always the main frame widget I am sure...) means that the
RenderWidget must ignore IPC-based control of its size from the
browser.

This mode was deprecated but later undeprecated, so we can expect it to
stay for ~forever.

In this CL we remove the ResizingModeSelector as it basically pulls
one if statement out of RenderWidget at the cost of much more difficulty
understanding the behaviour of RenderWidget, and forcing a different
public API on RenderWidget.

The ResizingModeSelector also has a method that is not used at all, so
it goes away in the process.

We add comments explaining the code's decisions and some TODOs as the
decisions look overly cautious for this limited testing scenario.

R=avi@chromium.org

Bug: 912193
Change-Id: I5def95e5e4a912f59263b6765b92ef656f62607c
Reviewed-on: https://chromium-review.googlesource.com/c/1447037
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628011}
[modify] https://crrev.com/7602f4f4603fb3f992c0deb9594f1f57e40695f8/content/renderer/BUILD.gn
[modify] https://crrev.com/7602f4f4603fb3f992c0deb9594f1f57e40695f8/content/renderer/render_view_impl.cc
[modify] https://crrev.com/7602f4f4603fb3f992c0deb9594f1f57e40695f8/content/renderer/render_widget.cc
[modify] https://crrev.com/7602f4f4603fb3f992c0deb9594f1f57e40695f8/content/renderer/render_widget.h
[delete] https://crrev.com/66236c640db16d71a5cd3f3e59638fb323d5f542/content/renderer/resizing_mode_selector.cc
[delete] https://crrev.com/66236c640db16d71a5cd3f3e59638fb323d5f542/content/renderer/resizing_mode_selector.h

Comment 54 by bugdroid, Feb 1

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec509adfa3ac85fab3cd51422b8aaf9cbb6b43cb

commit ec509adfa3ac85fab3cd51422b8aaf9cbb6b43cb
Author: danakj <danakj@chromium.org>
Date: Fri Feb 01 00:51:57 2019

Collect browser control data ignoring null-ness of |view_|.

Since the data comes from the |delegate_| of the RenderWidgetHostImpl,
it does not depend on a |view_| being non-null. This reduces the
number of early outs for the browser controls code.

TBR=ajwong@chromium.org

Change-Id: I17b4ac724708091a65cf8936ecb56ba5080da581
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1443530
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628173}
[modify] https://crrev.com/ec509adfa3ac85fab3cd51422b8aaf9cbb6b43cb/content/browser/renderer_host/render_widget_host_impl.cc

Comment 55 by bugdroid, Feb 1

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4658f3f5b86ab1da34311121251581c42c9ff31f

commit 4658f3f5b86ab1da34311121251581c42c9ff31f
Author: danakj <danakj@chromium.org>
Date: Fri Feb 01 15:57:01 2019

Remove null-check for |delegate_| in RWHI::GetVisualProperties().

RenderWidgetHostImpl already checks for |delegate_| before calling the
method. The other caller does so in the same stack that it uses the
WebContents, which is the delegate. The delegate is removed only when
the WebContents is destroyed, so we know it will be present in both
cases.

TBR=ajwong@chromium.org

Change-Id: Ia0fe71c00a87bb92b22ebeff545762fbcf72848d
Bug: 912193
Reviewed-on: https://chromium-review.googlesource.com/c/1443933
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628349}
[modify] https://crrev.com/4658f3f5b86ab1da34311121251581c42c9ff31f/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/4658f3f5b86ab1da34311121251581c42c9ff31f/content/browser/renderer_host/render_widget_host_impl.h

Comment 56 by danakj@chromium.org, Feb 4

Blockedon: 927694

Comment 57 by bugdroid, Feb 5

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d12ea212263ec55f33594eac4b222a6502e9dbcb

commit d12ea212263ec55f33594eac4b222a6502e9dbcb
Author: danakj <danakj@chromium.org>
Date: Tue Feb 05 19:06:08 2019

Get the WindowRect from each frame's WebWidgetClient.

The WindowRect given to each WebWidgetClient (aka RenderWidget) is the
same rect, representing the bounds of the top level window's web
content. Currently we always ask the WebView's top-level RenderWidget
for the value, but when it is frozen this is not correct. It happens
to work since we send PageMsg_UpdateWindowScreenRect to these frozen
RenderWidgets via their inactive RenderViewImpls.

Change ChromeClient to take a LocalFrame parameter and grab the
WindowRect from that LocalFrame's RenderWidget (from the frame's
local root technically) instead.

Also remove PageRect() since it just returns the WindowRect. And drop
RootWindowRect() from the WebViewClient API as it's no longer needed.

R=dcheng@chromium.org

Bug: 912193
Change-Id: I79340d7965260f6dd1aa2a7056c5c631b699a0b4
Reviewed-on: https://chromium-review.googlesource.com/c/1448747
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Auto-Submit: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629248}
[modify] https://crrev.com/d12ea212263ec55f33594eac4b222a6502e9dbcb/content/renderer/render_view_impl.cc
[modify] https://crrev.com/d12ea212263ec55f33594eac4b222a6502e9dbcb/content/renderer/render_view_impl.h
[modify] https://crrev.com/d12ea212263ec55f33594eac4b222a6502e9dbcb/content/test/web_test_support.cc
[modify] https://crrev.com/d12ea212263ec55f33594eac4b222a6502e9dbcb/third_party/blink/public/web/web_view_client.h
[modify] https://crrev.com/d12ea212263ec55f33594eac4b222a6502e9dbcb/third_party/blink/renderer/core/exported/web_page_popup_impl.cc
[modify] https://crrev.com/d12ea212263ec55f33594eac4b222a6502e9dbcb/third_party/blink/renderer/core/frame/local_dom_window.cc
[modify] https://crrev.com/d12ea212263ec55f33594eac4b222a6502e9dbcb/third_party/blink/renderer/core/loader/empty_clients.h
[modify] https://crrev.com/d12ea212263ec55f33594eac4b222a6502e9dbcb/third_party/blink/renderer/core/page/chrome_client.h
[modify] https://crrev.com/d12ea212263ec55f33594eac4b222a6502e9dbcb/third_party/blink/renderer/core/page/chrome_client_impl.cc
[modify] https://crrev.com/d12ea212263ec55f33594eac4b222a6502e9dbcb/third_party/blink/renderer/core/page/chrome_client_impl.h
[modify] https://crrev.com/d12ea212263ec55f33594eac4b222a6502e9dbcb/third_party/blink/renderer/core/page/create_window.cc

Comment 58 by bugdroid, Feb 5

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4df8a9d0aede6e1e7061941713c982fde674f0c0

commit 4df8a9d0aede6e1e7061941713c982fde674f0c0
Author: danakj <danakj@chromium.org>
Date: Tue Feb 05 20:36:59 2019

Remove redundant PageMsg_UpdateWindowScreenRect IPC path.

This path sends the window screen rect, through the RenderViewImpl
of the main frame, to the RenderWidget. But it does so immediately
after sending the window screen rect + widget screen rect to the
same RenderWidget.

There is an exception which is when an InterstitialPageImpl is present
in the WebContents. Then we would have sent the window screen rect to
that interstitial's main frame (via its RenderViewImpl). Instead,
walk the full frame tree of the InterstitialPageImpl (don't assume only
one widget) and go through the RenderWidgetHostImpl to send both
rects using the same code path as we would for a non-interstitial.

Then we remove the PageMsg_UpdateWindowScreenRect IPC and all of the
plumbing around it.

R=nasko@chromium.org

Bug: 912193
Change-Id: I6f2d21f40e548d3eb4d8104cea9427a5118a0f2b
Reviewed-on: https://chromium-review.googlesource.com/c/1448631
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Commit-Queue: danakj <danakj@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629291}
[modify] https://crrev.com/4df8a9d0aede6e1e7061941713c982fde674f0c0/content/browser/frame_host/interstitial_page_impl.h
[modify] https://crrev.com/4df8a9d0aede6e1e7061941713c982fde674f0c0/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/4df8a9d0aede6e1e7061941713c982fde674f0c0/content/common/page_messages.h
[modify] https://crrev.com/4df8a9d0aede6e1e7061941713c982fde674f0c0/content/renderer/devtools/render_widget_screen_metrics_emulator.cc
[modify] https://crrev.com/4df8a9d0aede6e1e7061941713c982fde674f0c0/content/renderer/devtools/render_widget_screen_metrics_emulator.h
[modify] https://crrev.com/4df8a9d0aede6e1e7061941713c982fde674f0c0/content/renderer/render_view_impl.cc
[modify] https://crrev.com/4df8a9d0aede6e1e7061941713c982fde674f0c0/content/renderer/render_view_impl.h
[modify] https://crrev.com/4df8a9d0aede6e1e7061941713c982fde674f0c0/content/renderer/render_widget.cc
[modify] https://crrev.com/4df8a9d0aede6e1e7061941713c982fde674f0c0/content/renderer/render_widget.h

Comment 59 by bugdroid, Feb 6

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/648d903b6923f7d4dac5f2e780fed04b383538e1

commit 648d903b6923f7d4dac5f2e780fed04b383538e1
Author: danakj <danakj@chromium.org>
Date: Wed Feb 06 16:25:33 2019

Null out the WebWidgetClient in WebViewImpl::Close().

We null away the WebViewClient already, but once the WebWidget parts
are closed we want to accept the RenderWidget (ie WebWidgetClient) being
destroyed. So drop the pointer to the WebWidgetClient when the main
frame is detached from Close().

R=dcheng@chromium.org

Bug: 912193
Change-Id: Ib4e0916c5e4cefaf49cb529135d38010809c3ea0
Reviewed-on: https://chromium-review.googlesource.com/c/1454135
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#629602}
[modify] https://crrev.com/648d903b6923f7d4dac5f2e780fed04b383538e1/third_party/blink/renderer/core/exported/web_view_impl.cc

Comment 60 by bugdroid, Feb 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7747362eabc23081af7c6d794241a01e80a811b2

commit 7747362eabc23081af7c6d794241a01e80a811b2
Author: danakj <danakj@chromium.org>
Date: Fri Feb 08 04:23:46 2019

Remove lots of DidInvalidateRect plumbing and APIs.

This API was for notifying the WebWidgetClient about invalidations back
when we did not use a compositor. Now it has only one use which is for
notifying WebViewPlugin of invalidation each frame.

The path that is used is:
ChromeClient->WebView->WebWidgetClient->WebViewPlugin

This removes other paths, documents it as WebViewPlugin-specific. And
removes Apis in fullscreen pepper that were for plumbing through to
the WebWidgetClient, since pepper is not used in WebViewPlugin.

Then we move the API from WebWidgetClient to WebViewClient since
WebViewPlugin only uses one widget and the API through ChromeClient
and WebViewImpl is all widget-agnostic.

TBR=dcheng@chromium.org

Bug: 912193
Change-Id: Ibd6f7ab3aee48bebf37a311dfd1f31609d406acc
Reviewed-on: https://chromium-review.googlesource.com/c/1456999
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630170}
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/components/plugins/renderer/webview_plugin.h
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/content/renderer/pepper/fullscreen_container.h
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/content/renderer/pepper/pepper_plugin_instance_impl.cc
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/content/renderer/render_widget_fullscreen_pepper.cc
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/content/renderer/render_widget_fullscreen_pepper.h
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/content/shell/test_runner/web_view_test_proxy.cc
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/content/shell/test_runner/web_view_test_proxy.h
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/third_party/blink/public/web/web_view_client.h
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/third_party/blink/public/web/web_widget.h
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/third_party/blink/public/web/web_widget_client.h
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/third_party/blink/renderer/core/exported/web_page_popup_impl.cc
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
[modify] https://crrev.com/7747362eabc23081af7c6d794241a01e80a811b2/third_party/blink/renderer/core/page/chrome_client.h

Comment 61 by bugdroid, Feb 8

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/26b9991aba5797ee7473794a7d616d8efa79a891

commit 26b9991aba5797ee7473794a7d616d8efa79a891
Author: danakj <danakj@chromium.org>
Date: Fri Feb 08 18:58:19 2019

Move WebLayerTreeView::SetRootLayer to WebWidgetClient. (1/n)

The WebLayerTreeView is a shortcut past the WebWidgetClient that is
not needed now that we always composite. This works toward eliminating
this extra API layer

More importantly this helps us Close/Restart the WebWidget parts of
WebViewImpl by not requiring plumbing 2 pointers with slightly different
lifetimes (WebLayerTreeView is created inside the Init of the
WebWidgetClient). This will help avoid a bunch of complexity in
creating WebFrameWidgets and other WebWidgets.

TBR=dcheng@chromium.org

Bug: 912193
Change-Id: Id9b496255f7f8bb67405f779903b6ba1826c3716
Reviewed-on: https://chromium-review.googlesource.com/c/1456205
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#630414}
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/content/renderer/compositor/layer_tree_view.cc
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/content/renderer/compositor/layer_tree_view.h
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/content/renderer/render_widget.cc
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/content/renderer/render_widget.h
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/content/renderer/render_widget_fullscreen_pepper.cc
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/content/shell/test_runner/web_view_test_proxy.cc
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/content/shell/test_runner/web_view_test_proxy.h
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/third_party/blink/public/platform/web_layer_tree_view.h
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/third_party/blink/public/web/web_widget_client.h
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/third_party/blink/renderer/core/exported/web_page_popup_impl.cc
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/third_party/blink/renderer/core/exported/web_page_popup_impl.h
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/third_party/blink/renderer/core/frame/frame_test_helpers.cc
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/third_party/blink/renderer/core/frame/frame_test_helpers.h
[modify] https://crrev.com/26b9991aba5797ee7473794a7d616d8efa79a891/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc

Comment 62 by bugdroid, Feb 13 (5 days ago)

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/04d55bb599cf00bc0e7493795add44fefff33721

commit 04d55bb599cf00bc0e7493795add44fefff33721
Author: danakj <danakj@chromium.org>
Date: Wed Feb 13 20:28:10 2019

Remove AttachCompositorAnimationTimeline from WebFrameWidget creation.

This branch in WebLocalFrameImpl::SetFrameWidget() to attach the
compositor timeline was added in f5af1abbbe19b919a claiming that
Document::Initialize() may be called for a LocalFrame that has not
attached a WebFrameWidget yet.

I claim that this does occur but only in cases where the LocalFrame
will /never/ have a WebFrameWidget so doing this later in that case
is pointless. And this ends up attempting to attach a compositor
timeline in the middle of creating WebFrameWidgetImpl - when it does
not have an initialized WebWidgetClient (aka RenderWidget) and thus
has no LayerTreeView or AnimationHost setup for it yet. Thus the code
path goes to ChromeClientImpl and simply returns due to null pointer.

So in both cases this is not needed. Further points below.

In the first case, we do see Document::Initialize() called for
LocalFrame before it is attached to a local root when
LocalFrame::ForceSynchronousDocumentInstall() is called. This is done
in cases where the LocalFrame hosts an orphan Page object, and is used
for painting into the "real" Document/Page, such as for SVG, inspector
overlays, validation overlays. Since these are orphan Pages, the frame
is never attached and WebLocalFrameImpl::SetFrameWidget() would not
occur.

ForceSynchronousDocumentInstall() is also used for WebPagePopupImpl
which is not a frame, so does not ever have a WebFrameWidget.

In the second case, Document::Initialize() is called from:
- CommitNavigation: This is after the local root frame has a
WebFrameWidget attached to it. The WebFrameWidget is attached inside
RenderFrameImpl::CreateFrame(), whereas CommitNavigation happens in
another call stack.
- ReplaceDocumentWhileExecutingJavaScriptURL: this is also done from
another stack, not from inside RenderFrameImpl::CreateFrame() between
the WebFrame being created and the WebFrameWidget.
- XSLTProcessor::CreateDocumentFromSource: this is called only if the
frame already exists and has a document - which was already
initialized. So we know it already satisfies having a WebFrameWidget
by induction.

R=pdr@chromium.org, wangxianzhu@chromium.org

Bug: 912193
Change-Id: I6c8e71c2710110f51a686f63cc6fe49b8adcadfb
Reviewed-on: https://chromium-review.googlesource.com/c/1468596
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#631875}
[modify] https://crrev.com/04d55bb599cf00bc0e7493795add44fefff33721/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/04d55bb599cf00bc0e7493795add44fefff33721/third_party/blink/renderer/core/dom/document.h
[modify] https://crrev.com/04d55bb599cf00bc0e7493795add44fefff33721/third_party/blink/renderer/core/frame/web_local_frame_impl.cc

Comment 63 by bugdroid, Feb 13 (5 days ago)

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d

commit e3bd126257e4f85efbbdf9b6f36a0fece04bde3d
Author: danakj <danakj@chromium.org>
Date: Wed Feb 13 21:54:16 2019

Remove blink::CompositorAnimationHost. (2/n)

We want to remove WebLayerTreeView and to do so we must remove the
CompositorAnimationHost() method from it. While doing so, instead of
adding a replacement method in WebWidgetClient, pass the AnimationHost
directly to the WebWidget::SetLayerTreeView(). This avoids an A->B->A
re-entrancy since SetLayerTreeView() would ask back for the
AnimationHost anyway.

While passing in a cc::AnimationHost to SetLayerTreeView(), drop the
blink wrapper type and use cc::AnimationHost directly everywhere. This
is mostly mechanical except:
- Make WebViewImpl always expect there to be an animation host, instead
of storing a null when IsThreadedAnimationEnabled() is off.
- Then have ChromeClientImpl check for the feature explicitly instead
of handling nullptrs.
- Rearrange some code in tests that was causing us to have a null
AnimationHost when we truly didn't expect to! This exposed some weird
ordering stuff but it is resolvable in the test harness.

R=haraken@chromium.org

Bug: 912193
Change-Id: Ide492ea5cf809f3be2bddf727db728d1d7e9f3d6
Reviewed-on: https://chromium-review.googlesource.com/c/1461503
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#631899}
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/content/renderer/compositor/layer_tree_view.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/content/renderer/compositor/layer_tree_view.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/content/renderer/render_widget.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/content/renderer/render_widget_fullscreen_pepper.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/content/renderer/render_widget_unittest.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/public/platform/web_layer_tree_view.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/public/web/web_widget.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/DEPS
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/animation/compositor_animations_test.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/animation/document_animations.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/exported/web_page_popup_impl.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/exported/web_page_popup_impl.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/frame_test_helpers.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/frame_test_helpers.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/link_highlights.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/link_highlights.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/local_frame_view.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/local_frame_view.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/visual_viewport.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/visual_viewport.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/web_frame_widget_base.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/web_frame_widget_impl.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/web_view_frame_widget.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/frame/web_view_frame_widget.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/page/chrome_client_impl.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/page/page.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/page/page.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_context.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_context.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/paint/paint_layer_scrollable_area.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/scroll/scroll_animator_compositor_coordinator.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/core/scroll/scrollable_area.h
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/platform/BUILD.gn
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/platform/animation/animated_layers_test.cc
[delete] https://crrev.com/07fa3dd71b0e62bd4a25872aa9c5fec68e820d72/third_party/blink/renderer/platform/animation/compositor_animation_host.cc
[delete] https://crrev.com/07fa3dd71b0e62bd4a25872aa9c5fec68e820d72/third_party/blink/renderer/platform/animation/compositor_animation_host.h
[delete] https://crrev.com/07fa3dd71b0e62bd4a25872aa9c5fec68e820d72/third_party/blink/renderer/platform/animation/compositor_animation_host_test.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/platform/animation/compositor_animation_timeline.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/renderer/platform/animation/compositor_animation_timeline_test.cc
[modify] https://crrev.com/e3bd126257e4f85efbbdf9b6f36a0fece04bde3d/third_party/blink/tools/blinkpy/presubmit/audit_non_blink_usage.py

Comment 64 by bugdroid, Feb 15 (3 days ago)

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2727e8a8d44244539b4d2002b451a59f8b4e40f3

commit 2727e8a8d44244539b4d2002b451a59f8b4e40f3
Author: Albert J. Wong <ajwong@chromium.org>
Date: Fri Feb 15 16:56:11 2019

Remove the Web{View,Frame,Widget}Test{Proxy[Base],Client} forest.

This CL builds on the huge amounts of ground work by danakj@ removing
templates and clarifying usage + relations for these classes.

This coding pattern of having proxy with a base class of common "fake"
functionality along with an extra "client" that could have specific
override of the base functionality was originally created to support
the Mandolin project. Mandolin would have added a second client to
blink in parallel to content. The structure of having a full proxy
that could select between multiple fake client implementations made
sense in this context. However, at this point, this forest of classes
is overkill.

For example, in the case of the View/Widget cluster of these
classes, each cluster was using 3 classes in a multiple-inheritance
and bridge configuration with lots of shell methods for the purpose
of faking the implementation for 5-6 virtual methods.

This CL merges all the Web{View,Frame,Widget}TestProxy classes with
their .*Base superclass. This removes 3 multi-inheritance hierarchies
which helps with code clarity and sanity in the debugger (it was easy
to accidentally cast a pointer incorrectly in gdb and crash the
debugger).

WebViewClient and WebWidgetClient were also merged into the Proxy
as the Proxies consisted mostly of forwarding logic. This again
removes another set of objects that can get mis-assigned in code
or mis-inspected in the debugger.

Lastly, WebViewTestProxy no longer inherits from WebWidgetTestProxy
mirroring the recent inheritance break between RenderView and
RenderWidget. This removes some duplicated code (ScheduleAnimation()
was nearly cut/paste between WebViewTestProxy and WebWidgetTestProxy)
and allows for a cleaner InstallCreateXXXHooks pattern.

See...deforestation isn't always bad.

Bug: 545684, 912193
Change-Id: I42a4ffc2d3bb55f7d73e3b758daaddf058128a04
Reviewed-on: https://chromium-review.googlesource.com/c/1437324
Commit-Queue: danakj <danakj@chromium.org>
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Auto-Submit: Albert J. Wong <ajwong@chromium.org>
Cr-Commit-Position: refs/heads/master@{#632648}
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/public/test/render_view_test.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/public/test/web_test_support.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/renderer/render_frame_impl.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/renderer/render_view_impl.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/renderer/render_view_impl.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/renderer/render_widget.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/renderer/render_widget.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/renderer/render_widget_unittest.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/renderer/web_test/blink_test_runner.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/renderer/web_test/blink_test_runner.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/renderer/web_test/web_test_content_renderer_client.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/BUILD.gn
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/accessibility_controller.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/accessibility_controller.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/event_sender.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/event_sender.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/test_interfaces.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/test_interfaces.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/test_runner.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/test_runner_for_specific_view.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/test_runner_for_specific_view.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/text_input_controller.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/text_input_controller.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/web_frame_test_client.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/web_frame_test_client.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/web_frame_test_proxy.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/web_test_delegate.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/web_test_interfaces.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/web_test_interfaces.h
[delete] https://crrev.com/1ad420dfa749b742a1fa72dbf8f02c102ecd94fd/content/shell/test_runner/web_view_test_client.cc
[delete] https://crrev.com/1ad420dfa749b742a1fa72dbf8f02c102ecd94fd/content/shell/test_runner/web_view_test_client.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/web_view_test_proxy.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/web_view_test_proxy.h
[delete] https://crrev.com/1ad420dfa749b742a1fa72dbf8f02c102ecd94fd/content/shell/test_runner/web_widget_test_client.cc
[delete] https://crrev.com/1ad420dfa749b742a1fa72dbf8f02c102ecd94fd/content/shell/test_runner/web_widget_test_client.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/web_widget_test_proxy.cc
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/shell/test_runner/web_widget_test_proxy.h
[modify] https://crrev.com/2727e8a8d44244539b4d2002b451a59f8b4e40f3/content/test/web_test_support.cc

Sign in to add a comment