New issue
Advanced search Search tips

Issue 870422 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 30
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 1
Type: Bug

Blocked on:
issue 874445

Blocking:
issue 836884



Sign in to add a comment

Inspector crashes with --enable-blink-gen-property-trees

Project Member Reported by pdr@chromium.org, Aug 2

Issue description

layer_tree_host.cc(774) Check failed: property_trees_.effect_tree.Node(layer->effect_tree_index()). 
0 base::debug::StackTrace::StackTrace(unsigned long) + 83
1 base::debug::StackTrace::StackTrace(unsigned long) + 29
2 base::debug::StackTrace::StackTrace() + 28
3 logging::LogMessage::~LogMessage() + 315
4 logging::LogMessage::~LogMessage() + 21
5 cc::LayerTreeHost::DoUpdateLayers(cc::Layer*) + 2755
6 cc::LayerTreeHost::UpdateLayers() + 286

I think we are missing an effect node for the inspector overlay.
 
Blockedon: 874445
Cc: chaopeng@chromium.org
This crash is initially due to missing property tree nodes for the elastic overscroll layer. Applying Chaopeng's patch from https://chromium-review.googlesource.com/c/chromium/src/+/1180561 fixes that.

The real issue is that the inspector overlay layer is added which invalidates the cc::Layer property tree nodes. These should be rebuilt in the next paint but are not which leads to a crash.
Cc: bokan@chromium.org pdr@chromium.org
 Issue 854202  has been merged into this issue.
Project Member

Comment 3 by bugdroid1@chromium.org, Aug 28

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

commit 1ec93db6c7ca9b7c052e830cbe37fd8088ea3182
Author: Philip Rogers <pdr@chromium.org>
Date: Tue Aug 28 13:49:46 2018

[BlinkGenPropertyTrees] Move validation messages into blink lifecycle

Validation messages are used for form controls when an invalid value is
entered. For historical reasons, these were integrated outside the
blink lifecycle update but they can be moved into blink entirely now.
This patch moves validation messages into blink and removes the
CORE_EXPORT on ValidationMessageClientImpl.

Bug:  870422 

Change-Id: Ieb3fc83b3a5f1c636320e3044a0c5b6c83da11d1
Reviewed-on: https://chromium-review.googlesource.com/1191802
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586679}
[modify] https://crrev.com/1ec93db6c7ca9b7c052e830cbe37fd8088ea3182/third_party/blink/renderer/core/dom/document_test.cc
[modify] https://crrev.com/1ec93db6c7ca9b7c052e830cbe37fd8088ea3182/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/1ec93db6c7ca9b7c052e830cbe37fd8088ea3182/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/1ec93db6c7ca9b7c052e830cbe37fd8088ea3182/third_party/blink/renderer/core/frame/local_frame_view.cc
[modify] https://crrev.com/1ec93db6c7ca9b7c052e830cbe37fd8088ea3182/third_party/blink/renderer/core/html/forms/html_form_control_element_test.cc
[modify] https://crrev.com/1ec93db6c7ca9b7c052e830cbe37fd8088ea3182/third_party/blink/renderer/core/page/page.cc
[modify] https://crrev.com/1ec93db6c7ca9b7c052e830cbe37fd8088ea3182/third_party/blink/renderer/core/page/page.h
[modify] https://crrev.com/1ec93db6c7ca9b7c052e830cbe37fd8088ea3182/third_party/blink/renderer/core/page/page_animator.cc
[modify] https://crrev.com/1ec93db6c7ca9b7c052e830cbe37fd8088ea3182/third_party/blink/renderer/core/page/validation_message_client_impl.cc
[modify] https://crrev.com/1ec93db6c7ca9b7c052e830cbe37fd8088ea3182/third_party/blink/renderer/core/page/validation_message_client_impl.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 28

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

commit 9df683a0c2fe6e6c3b960e51346c72710f5b3e5f
Author: Philip Rogers <pdr@chromium.org>
Date: Tue Aug 28 18:08:34 2018

[BlinkGenPropertyTrees] Move page color overlay into blink lifecycle

Page color overlays are used for extension "deemphasized" effects. These
were integrated outside the blink lifecycle update but they can be moved
entirely into blink now. This patch moves the page color overlay from
WebViewImpl to Page and puts the update and paint calls in the blink
document lifecycle.

Bug:  870422 
Change-Id: I92586ebeca29be24dfe35e033ca4099852d93ce4
Reviewed-on: https://chromium-review.googlesource.com/1192120
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586786}
[modify] https://crrev.com/9df683a0c2fe6e6c3b960e51346c72710f5b3e5f/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/9df683a0c2fe6e6c3b960e51346c72710f5b3e5f/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/9df683a0c2fe6e6c3b960e51346c72710f5b3e5f/third_party/blink/renderer/core/frame/local_frame_view.cc
[modify] https://crrev.com/9df683a0c2fe6e6c3b960e51346c72710f5b3e5f/third_party/blink/renderer/core/inspector/inspector_overlay_agent.cc
[modify] https://crrev.com/9df683a0c2fe6e6c3b960e51346c72710f5b3e5f/third_party/blink/renderer/core/page/page.cc
[modify] https://crrev.com/9df683a0c2fe6e6c3b960e51346c72710f5b3e5f/third_party/blink/renderer/core/page/page.h
[modify] https://crrev.com/9df683a0c2fe6e6c3b960e51346c72710f5b3e5f/third_party/blink/renderer/core/page/page_overlay.cc
[modify] https://crrev.com/9df683a0c2fe6e6c3b960e51346c72710f5b3e5f/third_party/blink/renderer/core/page/page_overlay.h
[modify] https://crrev.com/9df683a0c2fe6e6c3b960e51346c72710f5b3e5f/third_party/blink/renderer/core/page/page_overlay_test.cc
[modify] https://crrev.com/9df683a0c2fe6e6c3b960e51346c72710f5b3e5f/third_party/blink/renderer/core/page/validation_message_client_impl.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Aug 30

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

commit a0f906c1958b70a193181a394e7d31f8c683dbea
Author: Philip Rogers <pdr@chromium.org>
Date: Thu Aug 30 13:45:15 2018

[BlinkGenPropertyTrees] Integrate inspector into blink lifecycle

Before this patch, the inspector overlay was updated via two lifecycle hooks:
1) WebViewImpl::MainFrameLayoutUpdated would call PageOverlay::Update
which attached and sized the overlay graphics layer.
2) WebViewImpl::UpdateLifecycle (main frame) and
WebFrameWidgetImpl::UpdateLifecycle (local roots) would update the inspector
overlay after updating the main frame's lifecycle.

This approach is problematic for blink-gen-property-trees which needs all layers
to be attached and painted before layers are collected.

This patch unifies all of the inspector overlay updates into a single call:
WebLocalFrameImpl::UpdateDevToolsOverlays. Because WebLocalFrameImpl owns the
devtools agent, there is no need to have different paint codepaths for the
main frame and local frame, and WebViewImpl no longer needs to participate in
the devtools update at all. The unified devtools overlay update has been put
after the inspected page's paint phase completes but before layers are
collected. With this change, all inspector tests now pass with
blink-gen-property-trees.

Bug:  870422 

Cq-Include-Trybots: luci.chromium.try:linux-blink-gen-property-trees;luci.chromium.try:linux_layout_tests_slimming_paint_v2;master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ia99c10e374dbaf672dee2b461f3791ddd81a1555
Reviewed-on: https://chromium-review.googlesource.com/1188694
Commit-Queue: Philip Rogers <pdr@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587549}
[modify] https://crrev.com/a0f906c1958b70a193181a394e7d31f8c683dbea/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-gen-property-trees
[modify] https://crrev.com/a0f906c1958b70a193181a394e7d31f8c683dbea/third_party/blink/renderer/core/exported/web_dev_tools_agent_impl.cc
[modify] https://crrev.com/a0f906c1958b70a193181a394e7d31f8c683dbea/third_party/blink/renderer/core/exported/web_dev_tools_agent_impl.h
[modify] https://crrev.com/a0f906c1958b70a193181a394e7d31f8c683dbea/third_party/blink/renderer/core/exported/web_view_impl.cc
[modify] https://crrev.com/a0f906c1958b70a193181a394e7d31f8c683dbea/third_party/blink/renderer/core/exported/web_view_impl.h
[modify] https://crrev.com/a0f906c1958b70a193181a394e7d31f8c683dbea/third_party/blink/renderer/core/frame/local_frame_view.cc
[modify] https://crrev.com/a0f906c1958b70a193181a394e7d31f8c683dbea/third_party/blink/renderer/core/frame/web_frame_widget_impl.cc
[modify] https://crrev.com/a0f906c1958b70a193181a394e7d31f8c683dbea/third_party/blink/renderer/core/frame/web_local_frame_impl.cc
[modify] https://crrev.com/a0f906c1958b70a193181a394e7d31f8c683dbea/third_party/blink/renderer/core/frame/web_local_frame_impl.h
[modify] https://crrev.com/a0f906c1958b70a193181a394e7d31f8c683dbea/third_party/blink/renderer/core/inspector/inspector_overlay_agent.cc
[modify] https://crrev.com/a0f906c1958b70a193181a394e7d31f8c683dbea/third_party/blink/renderer/core/inspector/inspector_overlay_agent.h

Status: Fixed (was: Assigned)

Sign in to add a comment