Fix layout test failures for fragment box for SPAN |
||
Issue descriptionThis is a tracking bug when we generate fragment box for SPAN[1]. [1] http://crrev.com/c/790177 Create NGPhysicalFragment for SPAN element
,
Dec 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/813190a7045c2226aa6f1a4c9156888038bd9c48 commit 813190a7045c2226aa6f1a4c9156888038bd9c48 Author: Yoshifumi Inoue <yosin@chromium.org> Date: Fri Dec 01 05:51:41 2017 Create NGPhysicalFragment for every SPAN element for LayoutNGPaintFragments runtime feature enabled This patch changes |ShouldCreateBoxFragment()| to return |true| for producing box fragments for plain SPAN when |LayoutNGPaintFragments| runtime feature enabled, to locate |NGPhysicalFragment| associated to |LayoutInlinline| for computing bounding box and other kinds rects. Without this patch, we need to traverse fragment tree rooted by |LayoutBlockFlow| containing |LayoutInline|. In legacy layout tree, we don't generate |InlineBox| for |LayoutInline| for performance optimization and called "Culled InlineBox". To compute rects in legacy layout tree, we traverse layout tree and line boxes in |LayoutInline|. This code makes |LayoutInline| implementation complicated. Note: There is a flag disabling "Culled InlineBox" as |AlwaysCreateLineBoxesForLayoutInline|. Once LayoutNG is stable enough to have profiling, we investigate whether generating fragment for every SPAN causes performance regression or not. [1] http://crrev.com/c/763188: ONE [LayoutNG] Utilize DCHECK(CanUseInlineBox()) in core/layout/ Bug: 789390 Change-Id: Ife87fc9ee487397891ae4515d1bcaea1afcf9c04 Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng Reviewed-on: https://chromium-review.googlesource.com/790177 Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/master@{#520865} [modify] https://crrev.com/813190a7045c2226aa6f1a4c9156888038bd9c48/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/813190a7045c2226aa6f1a4c9156888038bd9c48/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc
,
Dec 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d32934233b5f4470896217439d39922cefc99cf6 commit d32934233b5f4470896217439d39922cefc99cf6 Author: Koji Ishii <kojii@chromium.org> Date: Tue Dec 05 11:44:56 2017 Revert "Create NGPhysicalFragment for every SPAN element for LayoutNGPaintFragments runtime feature enabled" This reverts commit 813190a7045c2226aa6f1a4c9156888038bd9c48. Reason for revert: It turned out that this change regressed 387 tests when NGPaintFragment is enabled. Local test here: https://chromium-review.googlesource.com/c/chromium/src/+/802840 The assumption was that this change fixes several failures without any additional failures (after bidi was fixed in CL:802673.) Since the assumption didn't stand, revert this once and we'll revisit again when we have more insights on what's happening. Original change's description: > Create NGPhysicalFragment for every SPAN element for LayoutNGPaintFragments runtime feature enabled > > This patch changes |ShouldCreateBoxFragment()| to return |true| for producing > box fragments for plain SPAN when |LayoutNGPaintFragments| runtime feature > enabled, to locate |NGPhysicalFragment| associated to |LayoutInlinline| for > computing bounding box and other kinds rects. > > Without this patch, we need to traverse fragment tree rooted by > |LayoutBlockFlow| containing |LayoutInline|. > > In legacy layout tree, we don't generate |InlineBox| for |LayoutInline| for > performance optimization and called "Culled InlineBox". To compute rects in > legacy layout tree, we traverse layout tree and line boxes in |LayoutInline|. > This code makes |LayoutInline| implementation complicated. Note: There is a > flag disabling "Culled InlineBox" as |AlwaysCreateLineBoxesForLayoutInline|. > > Once LayoutNG is stable enough to have profiling, we investigate whether > generating fragment for every SPAN causes performance regression or not. > > > [1] http://crrev.com/c/763188: ONE [LayoutNG] Utilize DCHECK(CanUseInlineBox()) > in core/layout/ > > Bug: 789390 > Change-Id: Ife87fc9ee487397891ae4515d1bcaea1afcf9c04 > Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng > Reviewed-on: https://chromium-review.googlesource.com/790177 > Commit-Queue: Yoshifumi Inoue <yosin@chromium.org> > Reviewed-by: Koji Ishii <kojii@chromium.org> > Cr-Commit-Position: refs/heads/master@{#520865} TBR=yosin@chromium.org,kojii@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 789390 Change-Id: I7cdbb2ca7ab9b07718a2ce6b7f998f4b38803f3b Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng Reviewed-on: https://chromium-review.googlesource.com/807426 Commit-Queue: Koji Ishii <kojii@chromium.org> Reviewed-by: Koji Ishii <kojii@chromium.org> Reviewed-by: yosin (OOO Dec 11 to Jan 8) <yosin@chromium.org> Cr-Commit-Position: refs/heads/master@{#521669} [modify] https://crrev.com/d32934233b5f4470896217439d39922cefc99cf6/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/d32934233b5f4470896217439d39922cefc99cf6/third_party/WebKit/Source/core/layout/LayoutInlineTest.cpp [modify] https://crrev.com/d32934233b5f4470896217439d39922cefc99cf6/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc
,
Oct 26
|
||
►
Sign in to add a comment |
||
Comment 1 by yosin@chromium.org
, Nov 29 2017Blocking: 591099