Skip to content

GitLab

T
tangled
Commit 340dbd5c authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot
Browse files
Options

[LayoutNG] Support rewinding unpositioned floats 

This patch supports NGLineBreaker to rewind unpositioned
floats.

When rewinding floats occurs, LayoutNG either position them
incorrectly, or hit DCHECK in paint due to duplicated ID.
This patch fixes it for unpositioned foats by matching to
Edge and Gecko.

Blink considers such floats as if it were committed
(positioned in LayoutNG terminologies) and thus the position
is incorrect.

Bug: 636993
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: I884879b2034d2f63ae248f70de3edeb27d71cafe
Reviewed-on: https://chromium-review.googlesource.com/1136262
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#575357}
parent a9fab63e
Hide whitespace changes
Inline Side-by-side
Showing with 123 additions and 9 deletions
third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
View file @ 340dbd5c
...@@ -104,6 +104,7 @@ crbug.com/591099 external/wpt/WebCryptoAPI/generateKey/failures_RSASSA-PKCS1-v1_ ...@@ -104,6 +104,7 @@ crbug.com/591099 external/wpt/WebCryptoAPI/generateKey/failures_RSASSA-PKCS1-v1_
crbug.com/591099 external/wpt/WebCryptoAPI/generateKey/successes.worker.html [ Timeout ] crbug.com/591099 external/wpt/WebCryptoAPI/generateKey/successes.worker.html [ Timeout ]
crbug.com/591099 external/wpt/WebCryptoAPI/import_export/rsa_importKey.worker.html [ Timeout ] crbug.com/591099 external/wpt/WebCryptoAPI/import_export/rsa_importKey.worker.html [ Timeout ]
crbug.com/709227 external/wpt/WebCryptoAPI/import_export/symmetric_importKey.worker.html [ Failure ] crbug.com/709227 external/wpt/WebCryptoAPI/import_export/symmetric_importKey.worker.html [ Failure ]
crbug.com/591099 external/wpt/css/CSS2/floats/floats-line-wrap-shifted-001.html [ Pass ]
crbug.com/591099 external/wpt/css/CSS2/normal-flow/block-in-inline-empty-001.xht [ Pass ] crbug.com/591099 external/wpt/css/CSS2/normal-flow/block-in-inline-empty-001.xht [ Pass ]
crbug.com/591099 external/wpt/css/CSS2/normal-flow/block-in-inline-empty-004.xht [ Pass ] crbug.com/591099 external/wpt/css/CSS2/normal-flow/block-in-inline-empty-004.xht [ Pass ]
crbug.com/591099 external/wpt/css/CSS2/normal-flow/block-in-inline-insert-001e.xht [ Pass ] crbug.com/591099 external/wpt/css/CSS2/normal-flow/block-in-inline-insert-001e.xht [ Pass ]
......
third_party/WebKit/LayoutTests/TestExpectations
View file @ 340dbd5c
...@@ -172,6 +172,7 @@ crbug.com/849459 fragmentation/repeating-thead-under-repeating-thead.html [ Fail ...@@ -172,6 +172,7 @@ crbug.com/849459 fragmentation/repeating-thead-under-repeating-thead.html [ Fail
# ====== Layout team owned tests from here ====== # ====== Layout team owned tests from here ======
crbug.com/591099 external/wpt/css/CSS2/floats/floats-line-wrap-shifted-001.html [ Failure ]
crbug.com/711704 external/wpt/css/CSS2/floats/floats-rule3-outside-left-002.xht [ Failure ] crbug.com/711704 external/wpt/css/CSS2/floats/floats-rule3-outside-left-002.xht [ Failure ]
crbug.com/711704 external/wpt/css/CSS2/floats/floats-rule3-outside-right-002.xht [ Failure ] crbug.com/711704 external/wpt/css/CSS2/floats/floats-rule3-outside-right-002.xht [ Failure ]
crbug.com/711704 external/wpt/css/CSS2/floats/floats-rule7-outside-left-001.xht [ Failure ] crbug.com/711704 external/wpt/css/CSS2/floats/floats-rule7-outside-left-001.xht [ Failure ]
......
third_party/WebKit/LayoutTests/external/wpt/css/CSS2/floats/floats-line-wrap-shifted-001-ref.html 0 → 100644
View file @ 340dbd5c
<!DOCTYPE html>
<style>
div {
font-size: 10px;
width: 12ch;
line-height: 1;
background: yellow;
}
.float {
width: 12ch;
height: 1em;
background: orange;
}
</style>
<body>
<div>
1111<br>
2222 3333
<div class="float"></div>
</div>
</body>
third_party/WebKit/LayoutTests/external/wpt/css/CSS2/floats/floats-line-wrap-shifted-001.html 0 → 100644
View file @ 340dbd5c
<!DOCTYPE html>
<link rel="author" title="Koji Ishii" href="kojii@chromium.org">
<link rel="help" href="https://drafts.csswg.org/css2/visuren.html#float-position">
<link rel="match" href="floats-line-wrap-shifted-001-ref.html">
<meta name="flags" content="" />
<meta name="assert" content="Float may not be higher than line-box containing a box generated by an element earlier in the source document." />
<style>
div {
font-size: 10px;
width: 12ch;
line-height: 1;
background: yellow;
}
.float {
float: left;
width: 12ch;
height: 1em;
background: orange;
}
</style>
<body>
<div>
1111
<nobr>
2222
<!--
This float does not fit in the 1st line and thus shifted downward.
-->
<div class="float"></div>
<!--
The next word causes the 1st line to wrap.
The last break opportunity was before the float, and thus the float is also
wrapped to the next line.
According to the rule 6, the float should be below <nobr> box.
6. The outer top of an element's floating box may not be higher than the top
of any line-box containing a box generated by an element earlier in the
source document.
-->
3333
</nobr>
</div>
</body>
third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc
View file @ 340dbd5c
...@@ -1077,25 +1077,46 @@ NGLineBreaker::LineBreakState NGLineBreaker::HandleOverflow( ...@@ -1077,25 +1077,46 @@ NGLineBreaker::LineBreakState NGLineBreaker::HandleOverflow(
} }
void NGLineBreaker::Rewind(NGLineInfo* line_info, unsigned new_end) { void NGLineBreaker::Rewind(NGLineInfo* line_info, unsigned new_end) {
NGInlineItemResults* item_results = &line_info->Results(); NGInlineItemResults& item_results = line_info->Results();
DCHECK_LT(new_end, item_results->size()); DCHECK_LT(new_end, item_results.size());
// TODO(ikilpatrick): Add DCHECK that we never rewind past any floats. // Avoid rewinding floats if possible. They will be added back anyway while
// processing trailing items even when zero available width. Also this saves
// most cases where our support for rewinding positioned floats is not great
// yet (see below.)
while (item_results[new_end].item->Type() == NGInlineItem::kFloating) {
++new_end;
if (new_end == item_results.size()) {
UpdatePosition(*line_info);
return;
}
}
if (new_end) { if (new_end) {
// Use |results[new_end - 1].end_offset| because it may have been truncated // Use |results[new_end - 1].end_offset| because it may have been truncated
// and may not be equal to |results[new_end].start_offset|. // and may not be equal to |results[new_end].start_offset|.
MoveToNextOf((*item_results)[new_end - 1]); MoveToNextOf(item_results[new_end - 1]);
} else { } else {
// When rewinding all items, use |results[0].start_offset|. // When rewinding all items, use |results[0].start_offset|.
const NGInlineItemResult& first_remove = (*item_results)[new_end]; const NGInlineItemResult& first_remove = item_results[new_end];
item_index_ = first_remove.item_index; item_index_ = first_remove.item_index;
offset_ = first_remove.start_offset; offset_ = first_remove.start_offset;
} }
// TODO(kojii): Should we keep results for the next line? We don't need to // Remove unpositioned floats that are to be rewound.
// re-layout atomic inlines. // Note, items before |handled_floats_end_item_index_| are inserted outside of
item_results->Shrink(new_end); // the line breaker and that should not be removed when rewinding.
for (unsigned i = new_end; i < item_results.size(); i++) {
NGInlineItemResult& rewind = item_results[i];
if (rewind.item->Type() == NGInlineItem::kFloating) {
NGBlockNode float_node(ToLayoutBox(rewind.item->GetLayoutObject()));
RemoveUnpositionedFloat(unpositioned_floats_, float_node);
// TODO(kojii): Probably need to do something if this float was already
// positioned, but haven't got how to do this yet.
}
}
item_results.Shrink(new_end);
SetLineEndFragment(nullptr, line_info); SetLineEndFragment(nullptr, line_info);
UpdatePosition(*line_info); UpdatePosition(*line_info);
......
third_party/blink/renderer/core/layout/ng/ng_floats_utils.cc
View file @ 340dbd5c
...@@ -346,6 +346,10 @@ void AddUnpositionedFloat( ...@@ -346,6 +346,10 @@ void AddUnpositionedFloat(
Vector<scoped_refptr<NGUnpositionedFloat>>* unpositioned_floats, Vector<scoped_refptr<NGUnpositionedFloat>>* unpositioned_floats,
NGContainerFragmentBuilder* fragment_builder, NGContainerFragmentBuilder* fragment_builder,
scoped_refptr<NGUnpositionedFloat> unpositioned_float) { scoped_refptr<NGUnpositionedFloat> unpositioned_float) {
// The same float node should not be added more than once.
DCHECK(
!RemoveUnpositionedFloat(unpositioned_floats, unpositioned_float->node));
if (fragment_builder && !fragment_builder->BfcOffset()) { if (fragment_builder && !fragment_builder->BfcOffset()) {
fragment_builder->AddAdjoiningFloatTypes( fragment_builder->AddAdjoiningFloatTypes(
unpositioned_float->IsLeft() ? kFloatTypeLeft : kFloatTypeRight); unpositioned_float->IsLeft() ? kFloatTypeLeft : kFloatTypeRight);
...@@ -353,6 +357,19 @@ void AddUnpositionedFloat( ...@@ -353,6 +357,19 @@ void AddUnpositionedFloat(
unpositioned_floats->push_back(std::move(unpositioned_float)); unpositioned_floats->push_back(std::move(unpositioned_float));
} }
bool RemoveUnpositionedFloat(
Vector<scoped_refptr<NGUnpositionedFloat>>* unpositioned_floats,
NGBlockNode float_node) {
for (scoped_refptr<NGUnpositionedFloat>& unpositioned_float :
*unpositioned_floats) {
if (unpositioned_float->node == float_node) {
unpositioned_floats->erase(&unpositioned_float);
return true;
}
}
return false;
}
NGFloatTypes ToFloatTypes(EClear clear) { NGFloatTypes ToFloatTypes(EClear clear) {
switch (clear) { switch (clear) {
default: default:
......
third_party/blink/renderer/core/layout/ng/ng_floats_utils.h
View file @ 340dbd5c
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
namespace blink { namespace blink {
class NGBlockNode;
class NGConstraintSpace; class NGConstraintSpace;
class NGContainerFragmentBuilder; class NGContainerFragmentBuilder;
class NGExclusionSpace; class NGExclusionSpace;
...@@ -59,6 +60,11 @@ void AddUnpositionedFloat( ...@@ -59,6 +60,11 @@ void AddUnpositionedFloat(
NGContainerFragmentBuilder* fragment_builder, NGContainerFragmentBuilder* fragment_builder,
scoped_refptr<NGUnpositionedFloat> unpositioned_float); scoped_refptr<NGUnpositionedFloat> unpositioned_float);
// Remove a pending float from the list.
bool RemoveUnpositionedFloat(
Vector<scoped_refptr<NGUnpositionedFloat>>* unpositioned_floats,
NGBlockNode float_node);
NGFloatTypes ToFloatTypes(EClear clear); NGFloatTypes ToFloatTypes(EClear clear);
} // namespace blink } // namespace blink
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment