New issue
Advanced search Search tips

Issue 878309 link

Starred by 2 users

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Task


Show other hotlists

Hotlists containing this issue:
layoutng


Sign in to add a comment

[LayoutNG] Avoid getting the LayoutObject off NG structures when possible

Project Member Reported by mstensho@chromium.org, Aug 28

Issue description

NGPhysicalFragment, NGPaintFragment and NGLayoutInputNode all provide methods to return the associated LayoutObject, but those shouldn't be called carelessly, because:

1: One day in the future there'll be no LayoutObject
2: The life-span of a LayoutObject is quite different from that of a NGPhysicalFragment, and the layout object may be deleted before the fragment.

This bug is about minimizing the use of those getters. The solution to each case of "unwarranted" usage may vary. When possible, we should avoid involving the layout object at all. If that's not a viable option, the NG class should provide a method that does the thing, rather than having someone on the outside call GetLayoutObject() and do their thing on their own.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 28

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

commit dfc32c00b0235fd45bba9801dbe0ab51f5f722a2
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Tue Aug 28 12:14:19 2018

[LayoutNG] NGPhysicalFragment has IsFloatingOrOutOfFlowPositioned().

No need to query the layout object at all.

Bug: 878309
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I7fc86696321f305c5dfee8b98aacef7b20a7a267
Reviewed-on: https://chromium-review.googlesource.com/1193364
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Aleks Totic <atotic@chromium.org>
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586662}
[modify] https://crrev.com/dfc32c00b0235fd45bba9801dbe0ab51f5f722a2/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 28

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

commit 0527de47e7db6f6448483b88ff4be5da03e3698d
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Tue Aug 28 16:06:20 2018

[LayoutNG] Add NGLayoutInputNode::ListMarkerOccupiesWholeLine().

Bug: 878309
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I961a8c962ca3b77b86a4cca37fdda81333ad8a88
Reviewed-on: https://chromium-review.googlesource.com/1194028
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#586727}
[modify] https://crrev.com/0527de47e7db6f6448483b88ff4be5da03e3698d/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc
[modify] https://crrev.com/0527de47e7db6f6448483b88ff4be5da03e3698d/third_party/blink/renderer/core/layout/ng/ng_layout_input_node.cc
[modify] https://crrev.com/0527de47e7db6f6448483b88ff4be5da03e3698d/third_party/blink/renderer/core/layout/ng/ng_layout_input_node.h

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 29

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

commit 7719f6c789672cd6207fb20fa1912ca8fc4111a9
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Wed Aug 29 13:41:45 2018

[LayoutNG] Introduce a PrepareForLayout() stage in NGBlockNode.

Move setting up the list item marker text there, rather than accessing layout
objects directly from the algorithm. Also there was no point repeating this
operation on the container for every child.

Bug: 878309
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I678b0824a0a4b729e56e01aa680234b961dfc587
Reviewed-on: https://chromium-review.googlesource.com/1194070
Reviewed-by: cathie chen <cathiechen@tencent.com>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587096}
[modify] https://crrev.com/7719f6c789672cd6207fb20fa1912ca8fc4111a9/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc
[modify] https://crrev.com/7719f6c789672cd6207fb20fa1912ca8fc4111a9/third_party/blink/renderer/core/layout/ng/ng_block_node.cc
[modify] https://crrev.com/7719f6c789672cd6207fb20fa1912ca8fc4111a9/third_party/blink/renderer/core/layout/ng/ng_block_node.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 29

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

commit 7292da3ba1bc8bd6f491d89523fe9ae1f5d19ed7
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Wed Aug 29 14:15:56 2018

[LayoutNG] Implement NGPhysicalBoxFragment::ChildrenInline() as a flag.

Bug: 878309
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: If8a07e6f04a1b18fa08dcff74fcc32cb37a74022
Reviewed-on: https://chromium-review.googlesource.com/1193879
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587104}
[modify] https://crrev.com/7292da3ba1bc8bd6f491d89523fe9ae1f5d19ed7/third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.cc
[modify] https://crrev.com/7292da3ba1bc8bd6f491d89523fe9ae1f5d19ed7/third_party/blink/renderer/core/layout/ng/ng_physical_box_fragment.h
[modify] https://crrev.com/7292da3ba1bc8bd6f491d89523fe9ae1f5d19ed7/third_party/blink/renderer/core/layout/ng/ng_physical_fragment.h

Labels: -Type-Bug Type-Task
Status: Available (was: Untriaged)
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 29

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

commit a2d12f8cea155f9920f75520db8f10ad255b5395
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Wed Aug 29 20:40:35 2018

[LayoutNG] Implement NGPhysicalTextFragment::IsAnonymousText() with a flag.

Bug: 878309
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ie048813575b614d9692ac999932a22e42f405eef
Reviewed-on: https://chromium-review.googlesource.com/1193857
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587287}
[modify] https://crrev.com/a2d12f8cea155f9920f75520db8f10ad255b5395/third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.cc
[modify] https://crrev.com/a2d12f8cea155f9920f75520db8f10ad255b5395/third_party/blink/renderer/core/layout/ng/inline/ng_physical_text_fragment.h

Sign in to add a comment