New issue
Advanced search Search tips

Issue 813057 link

Starred by 1 user
Status: Fixed
Owner:
futhark@chromium.org
Closed: Apr 2018
Cc:
e...@chromium.org
chrishtr@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Squad: Make RecalcStyle work for re-attaching elements with HasCustomStyleCallbacks

Project Member Reported by futhark@chromium.org, Feb 16 2018 
The target of Squad was to calculate ComputedStyle for all nodes before (re-)attaching the layout tree. Currently we stop recalculating style in re-attach subtrees encountering nodes returning true for HasCustomStyleCallbacks(). The reason is that a couple of CustomStyleForLayoutObject() methods rely on ancestor elements being attached, so we delay StyleForLayoutObject until the AttachLayoutTree call.

Identified methods which needs to be fixed:

  SVGElement::CustomStyleForLayoutObject()
  TextControlInnerEditorElement::CustomStyleForLayoutObject()

The former looks straightforward, the latter seems to be a bit of work.
Project Member

Comment 1 by bugdroid1@chromium.org, Feb 16 2018 

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

commit 992c22b41e881a63cb61840f070778ff30d9e8db
Author: Rune Lillesveen <futhark@chromium.org>
Date: Fri Feb 16 14:56:45 2018

[Squad] Get parent ComputedStyle from node, not from SVG layout object.

We currently don't compute style for elements being reattached in the
style recalc step when the element has custom style callbacks. The
reason is that some CustomStyleForLayoutObject() implementations rely on
ancestor elements being attached. Here, we stop relying on SVG
corresponding elements having a LayoutObject by retrieving ComputedStyle
from the Node instead. If the parent element is being (re-)attached, we
will get the non-attached style through Node::GetComputedStyle().

Bug:  813057 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I18b6a41087836746869071a409841692fa168537
Reviewed-on: https://chromium-review.googlesource.com/923732
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#537306}
[modify] https://crrev.com/992c22b41e881a63cb61840f070778ff30d9e8db/third_party/WebKit/Source/core/svg/SVGElement.cpp
Project Member

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

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

commit 4ef836ad2e67ce25e263514bcda94f808bfb02d6
Author: Rune Lillesveen <futhark@chromium.org>
Date: Fri Apr 13 11:23:31 2018

[Squad] Make style of inner editor not rely on LayoutObject.

Move custom style computation to TextControlInnerEditorElement and make
sure its style does not rely on the input host LayoutObject. Necessary
to be able to compute all elements' style in the RecalcStyle pass.

Bug:  813057 
Change-Id: I823ca67b5ddaba08ce7adfc26a29f45f73d88b8f
Reviewed-on: https://chromium-review.googlesource.com/925701
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550589}
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/WebKit/LayoutTests/fast/forms/suggested-value-after-empty-suggested-value-expected.txt
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/css/html.css
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/css/resolver/style_adjuster.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/html/forms/text_control_element.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/html/forms/text_control_element.h
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/html/forms/text_control_inner_elements.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/html/forms/text_control_inner_elements.h
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control.h
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control_multi_line.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control_multi_line.h
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control_single_line.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control_single_line.h
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 16 2018 

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

commit 0c342738138a4b1ae2e53a6f25c7e5e8892006eb
Author: Rune Lillesveen <futhark@chromium.org>
Date: Mon Apr 16 16:52:40 2018

[Squad] Allow CustomStyleForLayoutObject() in reattach recalc.

We did not allow recalculating style for reattachment as part of
RecalcStyle when we had custom style hooks because some elements had
CustomStyleForLayoutObject() methods which depended on parent layout
objects. This is no longer true, so we can now always recalc style for
re-attachment. That means we'll also need to call custom style hooks
WillRecalcStyle/DidRecalcStyle for re-attacment styles as well.

Bug:  813057 
Change-Id: I403c30e0f9f6934bd96d3da84488a6b3c489a50e
Reviewed-on: https://chromium-review.googlesource.com/925702
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551011}
[modify] https://crrev.com/0c342738138a4b1ae2e53a6f25c7e5e8892006eb/third_party/blink/renderer/core/dom/element.cc
Project Member

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

Labels: merge-merged-testbranch
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4ef836ad2e67ce25e263514bcda94f808bfb02d6

commit 4ef836ad2e67ce25e263514bcda94f808bfb02d6
Author: Rune Lillesveen <futhark@chromium.org>
Date: Fri Apr 13 11:23:31 2018

[Squad] Make style of inner editor not rely on LayoutObject.

Move custom style computation to TextControlInnerEditorElement and make
sure its style does not rely on the input host LayoutObject. Necessary
to be able to compute all elements' style in the RecalcStyle pass.

Bug:  813057 
Change-Id: I823ca67b5ddaba08ce7adfc26a29f45f73d88b8f
Reviewed-on: https://chromium-review.googlesource.com/925701
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Reviewed-by: Kent Tamura <tkent@chromium.org>
Cr-Commit-Position: refs/heads/master@{#550589}
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/WebKit/LayoutTests/fast/forms/suggested-value-after-empty-suggested-value-expected.txt
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/css/html.css
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/css/resolver/style_adjuster.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/html/forms/text_control_element.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/html/forms/text_control_element.h
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/html/forms/text_control_inner_elements.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/html/forms/text_control_inner_elements.h
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control.h
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control_multi_line.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control_multi_line.h
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control_single_line.cc
[modify] https://crrev.com/4ef836ad2e67ce25e263514bcda94f808bfb02d6/third_party/blink/renderer/core/layout/layout_text_control_single_line.h
Project Member

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

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

commit 0c342738138a4b1ae2e53a6f25c7e5e8892006eb
Author: Rune Lillesveen <futhark@chromium.org>
Date: Mon Apr 16 16:52:40 2018

[Squad] Allow CustomStyleForLayoutObject() in reattach recalc.

We did not allow recalculating style for reattachment as part of
RecalcStyle when we had custom style hooks because some elements had
CustomStyleForLayoutObject() methods which depended on parent layout
objects. This is no longer true, so we can now always recalc style for
re-attachment. That means we'll also need to call custom style hooks
WillRecalcStyle/DidRecalcStyle for re-attacment styles as well.

Bug:  813057 
Change-Id: I403c30e0f9f6934bd96d3da84488a6b3c489a50e
Reviewed-on: https://chromium-review.googlesource.com/925702
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551011}
[modify] https://crrev.com/0c342738138a4b1ae2e53a6f25c7e5e8892006eb/third_party/blink/renderer/core/dom/element.cc

Comment 6 by futhark@chromium.org, Apr 18 2018

Status: Fixed (was: Started)
►

Sign in to add a comment