Commit aef33f11 authored by Findit's avatar Findit

Revert "Make NGOffsetMapping available for non-LayoutNG LayoutBlockFlow"

This reverts commit b43c6fca.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 606292 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2I0M2M2ZmNhNDM0OWI3ZjJmZjUzYjQxOTI2ZjVkN2NiNDJlZTRlMGYM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium.linux/Cast%20Audio%20Linux/24981

Sample Failed Step: webkit_unit_tests

Original change's description:
> Make NGOffsetMapping available for non-LayoutNG LayoutBlockFlow
> 
> This patch allows computing NGOffsetMapping for legacy inline
> formatting context.
> 
> For an experimental project "Invisible DOM"[1][2] to support
> the find-in-page feature, it is needed to compute the text
> content with whitespace collapsing applied, along with the
> mapping to the DOM offset. This patch adds an API to compute
> NGOffsetMapping even when the LayoutBlockFlow is not laid out
> by LayoutNG for that purpose.
> 
> Note that the project is still in the early phase. We may
> revisit the design as it moves forward.
> 
> [1] https://github.com/rakina/searchable-invisible-dom
> [2] https://www.chromestatus.com/feature/5105291213406208
> 
> Bug: 636993, 873057
> Change-Id: I101b411960813a7b9b5c9c6e2db85d28737af882
> Reviewed-on: https://chromium-review.googlesource.com/c/1322184
> Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Reviewed-by: Yoshifumi Inoue <yosin@chromium.org>
> Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
> Commit-Queue: Koji Ishii <kojii@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#606292}

Change-Id: I7942801d9268078959619af61363c3a769fc0a53
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 636993, 873057
Reviewed-on: https://chromium-review.googlesource.com/c/1325350
Cr-Commit-Position: refs/heads/master@{#606316}
parent 78d31c19
...@@ -282,69 +282,32 @@ const NGOffsetMapping* NGInlineNode::ComputeOffsetMappingIfNeeded() { ...@@ -282,69 +282,32 @@ const NGOffsetMapping* NGInlineNode::ComputeOffsetMappingIfNeeded() {
NGInlineNodeData* data = MutableData(); NGInlineNodeData* data = MutableData();
if (!data->offset_mapping) { if (!data->offset_mapping) {
DCHECK(!data->text_content.IsNull()); // TODO(xiaochengh): ComputeOffsetMappingIfNeeded() discards the
ComputeOffsetMapping(GetLayoutBlockFlow(), data); // NGInlineItems and text content built by |builder|, because they are
DCHECK(data->offset_mapping); // already there in NGInlineNodeData. For efficiency, we should make
// |builder| not construct items and text content.
Vector<NGInlineItem> items;
items.ReserveCapacity(EstimateInlineItemsCount(*GetLayoutBlockFlow()));
NGInlineItemsBuilderForOffsetMapping builder(&items);
builder.GetOffsetMappingBuilder().ReserveCapacity(
EstimateOffsetMappingItemsCount(*GetLayoutBlockFlow()));
const bool update_layout = false;
CollectInlinesInternal(GetLayoutBlockFlow(), &builder, nullptr,
update_layout);
String text = builder.ToString();
DCHECK_EQ(data->text_content, text);
// TODO(xiaochengh): This doesn't compute offset mapping correctly when
// text-transform CSS property changes text length.
NGOffsetMappingBuilder& mapping_builder = builder.GetOffsetMappingBuilder();
mapping_builder.SetDestinationString(data->text_content);
data->offset_mapping =
std::make_unique<NGOffsetMapping>(mapping_builder.Build());
} }
return data->offset_mapping.get(); return data->offset_mapping.get();
} }
void NGInlineNode::ComputeOffsetMapping(LayoutBlockFlow* layout_block_flow,
NGInlineNodeData* data) {
DCHECK(!data->offset_mapping);
DCHECK(!layout_block_flow->GetDocument().NeedsLayoutTreeUpdate());
// TODO(xiaochengh): ComputeOffsetMappingIfNeeded() discards the
// NGInlineItems and text content built by |builder|, because they are
// already there in NGInlineNodeData. For efficiency, we should make
// |builder| not construct items and text content.
Vector<NGInlineItem> items;
items.ReserveCapacity(EstimateInlineItemsCount(*layout_block_flow));
NGInlineItemsBuilderForOffsetMapping builder(&items);
builder.GetOffsetMappingBuilder().ReserveCapacity(
EstimateOffsetMappingItemsCount(*layout_block_flow));
const bool update_layout = false;
CollectInlinesInternal(layout_block_flow, &builder, nullptr, update_layout);
if (data->text_content.IsNull())
data->text_content = builder.ToString();
else
DCHECK_EQ(data->text_content, builder.ToString());
// TODO(xiaochengh): This doesn't compute offset mapping correctly when
// text-transform CSS property changes text length.
NGOffsetMappingBuilder& mapping_builder = builder.GetOffsetMappingBuilder();
mapping_builder.SetDestinationString(data->text_content);
data->offset_mapping =
std::make_unique<NGOffsetMapping>(mapping_builder.Build());
DCHECK(data->offset_mapping);
}
const NGOffsetMapping* NGInlineNode::GetOffsetMapping(
LayoutBlockFlow* layout_block_flow,
std::unique_ptr<NGOffsetMapping>* storage) {
DCHECK(!layout_block_flow->GetDocument().NeedsLayoutTreeUpdate());
// If |layout_block_flow| is LayoutNG, compute from |NGInlineNode|.
if (layout_block_flow->IsLayoutNGMixin()) {
NGInlineNode node(layout_block_flow);
if (node.IsPrepareLayoutFinished())
return node.ComputeOffsetMappingIfNeeded();
// When this is not laid out yet, compute each time it is requested.
// TODO(kojii): We could still keep the result for later uses but it would
// add more states. Reconsider if this turned out to be needed.
}
// If this is not LayoutNG, compute the offset mapping and store in |storage|.
// The caller is responsible to keep |storage| for the life cycle.
NGInlineNodeData data;
ComputeOffsetMapping(layout_block_flow, &data);
*storage = std::move(data.offset_mapping);
DCHECK(*storage);
return storage->get();
}
// Depth-first-scan of all LayoutInline and LayoutText nodes that make up this // Depth-first-scan of all LayoutInline and LayoutText nodes that make up this
// NGInlineNode object. Collects LayoutText items, merging them up into the // NGInlineNode object. Collects LayoutText items, merging them up into the
// parent LayoutInline where possible, and joining all text content in a single // parent LayoutInline where possible, and joining all text content in a single
......
...@@ -70,17 +70,6 @@ class CORE_EXPORT NGInlineNode : public NGLayoutInputNode { ...@@ -70,17 +70,6 @@ class CORE_EXPORT NGInlineNode : public NGLayoutInputNode {
// This funciton must be called with clean layout. // This funciton must be called with clean layout.
const NGOffsetMapping* ComputeOffsetMappingIfNeeded(); const NGOffsetMapping* ComputeOffsetMappingIfNeeded();
// Get |NGOffsetMapping| for the |layout_block_flow|. If |layout_block_flow|
// is LayoutNG and it is already laid out, this function is the same as
// |ComputeOffsetMappingIfNeeded|. |storage| is not used in this case.
//
// Otherwise, this function computes |NGOffsetMapping| and store in |storage|
// as well as returning the pointer. The caller is responsible for keeping
// |storage| for the life cycle of the returned |NGOffsetMapping|.
static const NGOffsetMapping* GetOffsetMapping(
LayoutBlockFlow* layout_block_flow,
std::unique_ptr<NGOffsetMapping>* storage);
bool IsBidiEnabled() const { return Data().is_bidi_enabled_; } bool IsBidiEnabled() const { return Data().is_bidi_enabled_; }
TextDirection BaseDirection() const { return Data().BaseDirection(); } TextDirection BaseDirection() const { return Data().BaseDirection(); }
...@@ -132,9 +121,6 @@ class CORE_EXPORT NGInlineNode : public NGLayoutInputNode { ...@@ -132,9 +121,6 @@ class CORE_EXPORT NGInlineNode : public NGLayoutInputNode {
} }
const NGInlineNodeData& EnsureData(); const NGInlineNodeData& EnsureData();
static void ComputeOffsetMapping(LayoutBlockFlow* layout_block_flow,
NGInlineNodeData* data);
friend class NGLineBreakerTest; friend class NGLineBreakerTest;
friend class NGInlineNodeLegacy; friend class NGInlineNodeLegacy;
}; };
......
...@@ -1149,40 +1149,4 @@ TEST_F(NGOffsetMappingTest, TextOverflowEllipsis) { ...@@ -1149,40 +1149,4 @@ TEST_F(NGOffsetMappingTest, TextOverflowEllipsis) {
TEST_RANGE(mapping.GetRanges(), text, 0u, 1u); TEST_RANGE(mapping.GetRanges(), text, 0u, 1u);
} }
// Test |GetOffsetMapping| which is available both for LayoutNG and for legacy.
class NGOffsetMappingGetterTest : public RenderingTest,
public testing::WithParamInterface<bool>,
private ScopedLayoutNGForTest {
public:
NGOffsetMappingGetterTest() : ScopedLayoutNGForTest(GetParam()) {}
};
INSTANTIATE_TEST_CASE_P(NGOffsetMappingTest,
NGOffsetMappingGetterTest,
testing::Bool());
TEST_P(NGOffsetMappingGetterTest, Get) {
SetBodyInnerHTML(R"HTML(
<div id=container>
Whitespaces in this text should be collapsed.
</div>
)HTML");
LayoutBlockFlow* layout_block_flow =
ToLayoutBlockFlow(GetLayoutObjectByElementId("container"));
DCHECK(layout_block_flow->ChildrenInline());
// For the purpose of this test, ensure this is laid out by each layout
// engine.
DCHECK_EQ(layout_block_flow->IsLayoutNGMixin(), GetParam());
std::unique_ptr<NGOffsetMapping> storage;
const NGOffsetMapping* mapping =
NGInlineNode::GetOffsetMapping(layout_block_flow, &storage);
EXPECT_TRUE(mapping);
EXPECT_EQ(!storage, GetParam()); // |storage| is used only for legacy.
const String& text_content = mapping->GetText();
EXPECT_EQ(text_content, "Whitespaces in this text should be collapsed.");
}
} // 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