New issue
Advanced search Search tips

Issue 820787 link

Starred by 1 user
Status: Assigned
Owner:
dcheng@chromium.org
Cc:
wfh@chromium.org
creis@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Create WebFrameWidget *before* creating the local root

Project Member Reported by dcheng@chromium.org, Mar 11 2018 
There are a lot of edge cases in code right now because we create it the other way. For example, when we create the initial empty document, that may trigger notifications to the WebFrameWidget's client...

... but it may not actually exist yet because we're still in the middle of frame creation, leading to null checks with unclear purposes.
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 17 2018 

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

commit 1a19c914bb8b9e5c2688c8b23ce67eda03072f1a
Author: Daniel Cheng <dcheng@chromium.org>
Date: Tue Apr 17 06:20:06 2018

[OOPIF] Refactor client and local root into WebFrameWidgetBase.

Extract some more common code into WebFrameWidgetBase. The end goal is
to completely merge WebFrameWidgetImpl and WebViewFrameWidget into
WebFrameWidgetBase. This is an intermediate step to make it easier to
create the frame widget before creating the local frame root: by sharing
storage in the base class, the followup patch to invert creation order
will only have to update pointers in one place.

Bug: 820787
Change-Id: I5f096c8a8d74abb6b6773ff36c75288b998d3880
Reviewed-on: https://chromium-review.googlesource.com/1014017
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551264}
[modify] https://crrev.com/1a19c914bb8b9e5c2688c8b23ce67eda03072f1a/third_party/blink/renderer/core/frame/web_frame_widget_base.cc
[modify] https://crrev.com/1a19c914bb8b9e5c2688c8b23ce67eda03072f1a/third_party/blink/renderer/core/frame/web_frame_widget_base.h
[modify] https://crrev.com/1a19c914bb8b9e5c2688c8b23ce67eda03072f1a/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
[modify] https://crrev.com/1a19c914bb8b9e5c2688c8b23ce67eda03072f1a/third_party/blink/renderer/core/frame/web_frame_widget_impl.h
[modify] https://crrev.com/1a19c914bb8b9e5c2688c8b23ce67eda03072f1a/third_party/blink/renderer/core/frame/web_view_frame_widget.cc
[modify] https://crrev.com/1a19c914bb8b9e5c2688c8b23ce67eda03072f1a/third_party/blink/renderer/core/frame/web_view_frame_widget.h
Project Member

Comment 2 by bugdroid1@chromium.org, Apr 22 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62089bc639bb3b5a5235583b7f19908f6d6ddc54

commit 62089bc639bb3b5a5235583b7f19908f6d6ddc54
Author: Daniel Cheng <dcheng@chromium.org>
Date: Sun Apr 22 07:00:53 2018

Remove redundant calls to WebSettings::SetAcceleratedCompositingEnabled(true).

The main motivation is to avoid WebFrameWidgetImpl::InitializeLayerTreeView's
dependency on GetPage() returning a non-null pointer: this becomes tricky in
followups when the initialization order of WebLocalFrame and WebFrameWidget
is inverted. As the default is true (outside test helpers for core), so just
remove the redundant calls to set it to true.

This also simplifies a few conditional branches in InitializeLayerTreeView(),
when the branches aren't needed.

Bug: 820787
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I9798b4d7923d61f4f30386e010e03beebbe589e7
Reviewed-on: https://chromium-review.googlesource.com/1020805
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Cr-Commit-Position: refs/heads/master@{#552602}
[modify] https://crrev.com/62089bc639bb3b5a5235583b7f19908f6d6ddc54/third_party/blink/renderer/core/animation/compositor_animations_test.cc
[modify] https://crrev.com/62089bc639bb3b5a5235583b7f19908f6d6ddc54/third_party/blink/renderer/core/exported/web_frame_test.cc
[modify] https://crrev.com/62089bc639bb3b5a5235583b7f19908f6d6ddc54/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/62089bc639bb3b5a5235583b7f19908f6d6ddc54/third_party/blink/renderer/core/exported/web_view_test.cc
[modify] https://crrev.com/62089bc639bb3b5a5235583b7f19908f6d6ddc54/third_party/blink/renderer/core/frame/browser_controls_test.cc
[modify] https://crrev.com/62089bc639bb3b5a5235583b7f19908f6d6ddc54/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
[modify] https://crrev.com/62089bc639bb3b5a5235583b7f19908f6d6ddc54/third_party/blink/renderer/core/page/scrolling/root_scroller_test.cc
[modify] https://crrev.com/62089bc639bb3b5a5235583b7f19908f6d6ddc54/third_party/blink/renderer/core/page/scrolling/scroll_metrics_test.cc
[modify] https://crrev.com/62089bc639bb3b5a5235583b7f19908f6d6ddc54/third_party/blink/renderer/core/page/scrolling/scrolling_coordinator_test.cc
[modify] https://crrev.com/62089bc639bb3b5a5235583b7f19908f6d6ddc54/third_party/blink/renderer/core/paint/html_canvas_painter_test.cc
[modify] https://crrev.com/62089bc639bb3b5a5235583b7f19908f6d6ddc54/third_party/blink/renderer/core/scheduler/frame_throttling_test.cc
►

Sign in to add a comment