Commit d12cfdf1 authored by Alice Boxhall's avatar Alice Boxhall Committed by Commit Bot

Revert "[LayoutNG] LocalVisualRect for inline"

This reverts commit a16415e1.

Reason for revert: Seems to have caused a leak: 
https://uberchromegw.corp.google.com/i/chromium.webkit/builders/WebKit%20Linux%20Trusty%20Leak/builds/11042

Original change's description:
> [LayoutNG] LocalVisualRect for inline
> 
> This patch implements LocalVisualRectIgnoringVisibility() for
> LayoutInline and LayoutText, that are used by PaintInvalidator.
> 
> This patch is still at the early iteration in a few ways:
> 1. Other similar functions (e.g., LocalSelectionRect()) are not
>    implemented.
> 2. The inline fragment iterator is supposed to be useful for
>    several other purposes. Making it more generic and applicable
>    for other purposes is planned after we collect how we use it.
> 3. Adds ~20 new failures, part because CurrentFragment() is not
>    available in some cases, part because overflow logic isn't
>    complete yet. I'll look into these failures in following patches.
> 
> Bug: 636993, 714962
> Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
> Change-Id: I55c607a40fcf705c46162577fd9a1255f3048da7
> Reviewed-on: https://chromium-review.googlesource.com/718637
> Commit-Queue: Koji Ishii <kojii@chromium.org>
> Reviewed-by: Emil A Eklund <eae@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#510020}

TBR=eae@chromium.org,kojii@chromium.org,xiaochengh@chromium.org

Change-Id: Idf7967b552c11dd92e022fc9e1e1668288f447fc
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 636993, 714962
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/729660Reviewed-by: default avatarAlice Boxhall <aboxhall@chromium.org>
Commit-Queue: Alice Boxhall <aboxhall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#510267}
parent e382a109
......@@ -347,8 +347,6 @@ blink_core_sources("layout") {
"ng/inline/ng_inline_box_state.h",
"ng/inline/ng_inline_break_token.cc",
"ng/inline/ng_inline_break_token.h",
"ng/inline/ng_inline_fragment_iterator.cc",
"ng/inline/ng_inline_fragment_iterator.h",
"ng/inline/ng_inline_item.cc",
"ng/inline/ng_inline_item.h",
"ng/inline/ng_inline_item_result.cc",
......
......@@ -34,7 +34,6 @@
#include "core/layout/LayoutView.h"
#include "core/layout/api/LineLayoutBoxModel.h"
#include "core/layout/line/InlineTextBox.h"
#include "core/layout/ng/layout_ng_block_flow.h"
#include "core/paint/BoxPainter.h"
#include "core/paint/InlinePainter.h"
#include "core/paint/ObjectPainter.h"
......@@ -1158,12 +1157,6 @@ LayoutRect LayoutInline::AbsoluteVisualRect() const {
}
LayoutRect LayoutInline::LocalVisualRectIgnoringVisibility() const {
if (RuntimeEnabledFeatures::LayoutNGEnabled()) {
NGPhysicalOffsetRect visual_rect;
if (LayoutNGBlockFlow::LocalVisualRectFor(this, &visual_rect))
return visual_rect.ToLayoutRect();
}
// If we don't create line boxes, we don't have any invalidations to do.
if (!AlwaysCreateLineBoxes())
return LayoutRect();
......
......@@ -1852,12 +1852,6 @@ LayoutRect LayoutText::VisualOverflowRect() const {
}
LayoutRect LayoutText::LocalVisualRectIgnoringVisibility() const {
if (RuntimeEnabledFeatures::LayoutNGEnabled()) {
NGPhysicalOffsetRect visual_rect;
if (LayoutNGBlockFlow::LocalVisualRectFor(this, &visual_rect))
return visual_rect.ToLayoutRect();
}
return UnionRect(VisualOverflowRect(), LocalSelectionRect());
}
......
......@@ -13,11 +13,6 @@ bool NGPhysicalOffsetRect::operator==(const NGPhysicalOffsetRect& other) const {
return other.offset == offset && other.size == size;
}
NGPhysicalOffsetRect NGPhysicalOffsetRect::operator+(
const NGPhysicalOffset& offset) const {
return {this->offset + offset, size};
}
void NGPhysicalOffsetRect::Unite(const NGPhysicalOffsetRect& other) {
if (other.IsEmpty())
return;
......
......@@ -31,8 +31,6 @@ struct CORE_EXPORT NGPhysicalOffsetRect {
bool operator==(const NGPhysicalOffsetRect& other) const;
NGPhysicalOffsetRect operator+(const NGPhysicalOffset&) const;
void Unite(const NGPhysicalOffsetRect&);
// Conversions from/to existing code. New code prefers type safety for
......
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "core/layout/ng/inline/ng_inline_fragment_iterator.h"
#include "core/layout/LayoutObject.h"
#include "core/layout/ng/ng_physical_box_fragment.h"
#include "platform/wtf/HashMap.h"
namespace blink {
namespace {
// Returns a static empty list.
const NGInlineFragmentIterator::Results* EmptyResults() {
DEFINE_THREAD_SAFE_STATIC_LOCAL(NGInlineFragmentIterator::Results,
empty_reuslts, ());
return &empty_reuslts;
}
// The expected use of the iterator is for a LayoutInline/LayoutText to find
// corresponding fragments and implement existing functions using them without
// relying on InlineBox list that were removed in LayoutNGPaintFragments.
//
// When existing code traverses LayoutObject tree, they call such functions to
// all LayoutInline/LayoutText chlidren, and if each call traverses inline
// fragment tree, it will be O(n^2). To avoid O(n^2) in such case, create a map
// from LayoutObject to fragments and keep it in a cache per the containing box.
//
// TODO(kojii): May need to expand to block children when we enabled fragment
// painting everywhere.
//
// Because the caller is likely traversing the tree, keep only one cache entry.
//
// To sipmlify, the cached map ownership is moved to NGInlineFragmentIterator
// when instantiated, and moved back to the cache when destructed, assuming
// there are no use cases to nest the iterator.
struct CacheEntry {
RefPtr<const NGPhysicalBoxFragment> box;
NGInlineFragmentIterator::LayoutObjectMap map;
};
CacheEntry* GetCacheEntry() {
DEFINE_THREAD_SAFE_STATIC_LOCAL(CacheEntry, cache_entry, ());
return &cache_entry;
}
bool GetCachedMap(const NGPhysicalBoxFragment& box,
NGInlineFragmentIterator::LayoutObjectMap* map) {
CacheEntry* cache = GetCacheEntry();
if (cache->box.get() != &box)
return false;
cache->map.swap(*map);
return true;
}
void AddToCache(const NGPhysicalBoxFragment& box,
NGInlineFragmentIterator::LayoutObjectMap* map) {
CacheEntry* cache = GetCacheEntry();
cache->box = &box;
cache->map.swap(*map);
}
} // namespace
NGInlineFragmentIterator::NGInlineFragmentIterator(
const NGPhysicalBoxFragment& box,
const LayoutObject* filter)
: box_(box) {
DCHECK(filter);
// Check the cache. This iterator owns the map while alive.
if (!GetCachedMap(box, &map_)) {
// Create a map if it's not in the cache.
CollectInlineFragments(box, {}, &map_);
}
const auto it = map_.find(filter);
results_ = it != map_.end() ? &it->value : EmptyResults();
}
NGInlineFragmentIterator::~NGInlineFragmentIterator() {
// Return the ownership of the map to the cache.
AddToCache(box_, &map_);
}
// Create a map from a LayoutObject to a vector of PhysicalFragment and its
// offset to the container box. This is done by collecting inline child
// fragments of the container fragment, while accumulating the offset to the
// container box.
void NGInlineFragmentIterator::CollectInlineFragments(
const NGPhysicalContainerFragment& container,
NGPhysicalOffset offset_to_container_box,
LayoutObjectMap* map) {
for (const auto& child : container.Children()) {
NGPhysicalOffset child_offset = child->Offset() + offset_to_container_box;
// In order to find fragments that correspond to the specified LayoutObject,
// add to the map if the fragment has a LayoutObject.
if (const LayoutObject* child_layout_object = child->GetLayoutObject()) {
const auto it = map->find(child_layout_object);
if (it == map->end()) {
map->insert(child_layout_object, Results{{child.get(), child_offset}});
} else {
it->value.push_back(Result{child.get(), child_offset});
}
}
// Traverse descendants unless the fragment is laid out separately from the
// inline layout algorithm.
if (child->IsContainer() && !child->IsBlockLayoutRoot()) {
CollectInlineFragments(ToNGPhysicalContainerFragment(*child),
child_offset, map);
}
}
}
} // namespace blink
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef NGInlineFragmentIterator_h
#define NGInlineFragmentIterator_h
#include "core/CoreExport.h"
#include "core/layout/ng/geometry/ng_physical_offset.h"
#include "platform/wtf/Allocator.h"
#include "platform/wtf/HashMap.h"
#include "platform/wtf/Vector.h"
namespace blink {
class LayoutObject;
class NGPhysicalBoxFragment;
class NGPhysicalContainerFragment;
class NGPhysicalFragment;
struct NGPhysicalOffset;
// Iterate through inline descendant fragments.
class CORE_EXPORT NGInlineFragmentIterator {
STACK_ALLOCATED();
public:
// Create an iterator that returns inline fragments produced from the
// specified LayoutObject.
NGInlineFragmentIterator(const NGPhysicalBoxFragment&,
const LayoutObject* filter);
~NGInlineFragmentIterator();
// The data struct the iterator returns.
struct Result {
const NGPhysicalFragment* fragment;
NGPhysicalOffset offset_to_container_box;
};
using Results = Vector<Result, 1>;
using LayoutObjectMap = HashMap<const LayoutObject*, Results>;
Results::const_iterator begin() const { return results_->begin(); }
Results::const_iterator end() const { return results_->end(); }
private:
static void CollectInlineFragments(const NGPhysicalContainerFragment&,
NGPhysicalOffset offset_to_container_box,
LayoutObjectMap*);
const Results* results_;
const NGPhysicalBoxFragment& box_;
LayoutObjectMap map_;
};
} // namespace blink
#endif // NGInlineFragmentIterator_h
......@@ -33,8 +33,7 @@ inline bool ShouldCreateBoxFragment(const NGInlineItem& item,
DCHECK(item.Style());
const ComputedStyle& style = *item.Style();
// TODO(kojii): We might need more conditions to create box fragments.
return style.HasBoxDecorationBackground() || style.HasOutline() ||
item_result.needs_box_when_empty;
return style.HasBoxDecorationBackground() || item_result.needs_box_when_empty;
}
NGBfcOffset GetOriginPointForFloats(const NGBfcOffset& container_bfc_offset,
......
......@@ -6,7 +6,6 @@
#include "core/layout/HitTestLocation.h"
#include "core/layout/LayoutAnalyzer.h"
#include "core/layout/ng/inline/ng_inline_fragment_iterator.h"
#include "core/layout/ng/inline/ng_inline_node_data.h"
#include "core/layout/ng/ng_constraint_space.h"
#include "core/layout/ng/ng_fragment_builder.h"
......@@ -288,27 +287,6 @@ LayoutUnit LayoutNGBlockFlow::InlineBlockBaseline(
return LayoutBlockFlow::InlineBlockBaseline(line_direction);
}
bool LayoutNGBlockFlow::LocalVisualRectFor(const LayoutObject* layout_object,
NGPhysicalOffsetRect* visual_rect) {
DCHECK(layout_object &&
(layout_object->IsText() || layout_object->IsLayoutInline()));
DCHECK(visual_rect);
LayoutNGBlockFlow* ng_block_flow = layout_object->EnclosingNGBlockFlow();
if (!ng_block_flow || !ng_block_flow->HasNGInlineNodeData())
return false;
const NGPhysicalBoxFragment* box_fragment = ng_block_flow->CurrentFragment();
// TODO(kojii): CurrentFragment isn't always available after layout clean.
// Investigate why.
if (!box_fragment)
return false;
NGInlineFragmentIterator children(*box_fragment, layout_object);
for (const auto& child : children) {
NGPhysicalOffsetRect child_visual_rect = child.fragment->LocalVisualRect();
visual_rect->Unite(child_visual_rect + child.offset_to_container_box);
}
return true;
}
RefPtr<NGLayoutResult> LayoutNGBlockFlow::CachedLayoutResult(
const NGConstraintSpace& constraint_space,
NGBreakToken* break_token) const {
......
......@@ -61,8 +61,6 @@ class CORE_EXPORT LayoutNGBlockFlow : public LayoutBlockFlow {
NGPaintFragment* PaintFragment() const { return paint_fragment_.get(); }
void SetPaintFragment(RefPtr<const NGPhysicalFragment>);
static bool LocalVisualRectFor(const LayoutObject*, NGPhysicalOffsetRect*);
protected:
bool IsOfType(LayoutObjectType) const override;
......
......@@ -4,9 +4,6 @@
#include "core/layout/ng/ng_physical_box_fragment.h"
#include "core/layout/LayoutBox.h"
#include "core/layout/LayoutObject.h"
namespace blink {
NGPhysicalBoxFragment::NGPhysicalBoxFragment(
......@@ -42,21 +39,7 @@ const NGBaseline* NGPhysicalBoxFragment::Baseline(
}
const NGPhysicalOffsetRect NGPhysicalBoxFragment::LocalVisualRect() const {
const ComputedStyle& style = Style();
if (!style.HasVisualOverflowingEffect())
return {{}, Size()};
LayoutObject* layout_object = GetLayoutObject();
if (layout_object->IsBox()) {
// TODO(kojii): Should move the logic to a common place.
LayoutRect visual_rect({}, Size().ToLayoutSize());
visual_rect.Expand(
ToLayoutBox(layout_object)->ComputeVisualEffectOverflowOutsets());
return NGPhysicalOffsetRect(visual_rect);
}
// TODO(kojii): Implement for inline boxes.
DCHECK(layout_object->IsLayoutInline());
// TODO(kojii): Add its own visual overflow (e.g., box-shadow)
return {{}, Size()};
}
......
......@@ -210,19 +210,6 @@ NGPixelSnappedPhysicalBoxStrut NGPhysicalFragment::BorderWidths() const {
return box_strut.SnapToDevicePixels();
}
NGPhysicalOffsetRect NGPhysicalFragment::LocalVisualRect() const {
switch (Type()) {
case NGPhysicalFragment::kFragmentBox:
return ToNGPhysicalBoxFragment(*this).LocalVisualRect();
case NGPhysicalFragment::kFragmentText:
return ToNGPhysicalTextFragment(*this).LocalVisualRect();
case NGPhysicalFragment::kFragmentLineBox:
return {{}, Size()};
}
NOTREACHED();
return {{}, Size()};
}
void NGPhysicalFragment::PropagateContentsVisualRect(
NGPhysicalOffsetRect* parent_visual_rect) const {
NGPhysicalOffsetRect visual_rect;
......
......@@ -117,9 +117,6 @@ class CORE_EXPORT NGPhysicalFragment
// with LegacyLayout.
LayoutObject* GetLayoutObject() const { return layout_object_; }
// VisualRect of itself, not including contents, in the local coordinate.
NGPhysicalOffsetRect LocalVisualRect() const;
// Unite visual rect to propagate to parent's ContentsVisualRect.
void PropagateContentsVisualRect(NGPhysicalOffsetRect*) const;
......
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