[text-autospace] Change the code for testing
`InlineTextAutoSpace::Apply()` had an optional vector for testing code to inspect its computation. This patch changes it to a callback. This reduces test-only code from the production, and makes it easier to change the core logic without changing tests. This patch has no behavior changes. Bug: 40275399 Change-Id: I5371e936fce4e954c245d839315c49efb897fbef Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6577875 Auto-Submit: Koji Ishii <kojii@chromium.org> Reviewed-by: Kent Tamura <tkent@chromium.org> Commit-Queue: Koji Ishii <kojii@chromium.org> Cr-Commit-Position: refs/heads/main@{#1464556}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
ff7255ab68
commit
c5c8506d5f
third_party/blink/renderer
core
platform
fonts
shaping
@@ -101,6 +101,13 @@ class SpacingApplier {
|
|||||||
DCHECK(shape_result);
|
DCHECK(shape_result);
|
||||||
shape_result->ApplyTextAutoSpacing(offsets_with_spacing_);
|
shape_result->ApplyTextAutoSpacing(offsets_with_spacing_);
|
||||||
item->SetUnsafeToReuseShapeResult();
|
item->SetUnsafeToReuseShapeResult();
|
||||||
|
if (callback_for_testing_) [[unlikely]] {
|
||||||
|
callback_for_testing_->DidApply(offsets_with_spacing_);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
void SetCallbackForTesting(InlineTextAutoSpace::Callback* callback) {
|
||||||
|
callback_for_testing_ = callback;
|
||||||
}
|
}
|
||||||
|
|
||||||
private:
|
private:
|
||||||
@@ -108,6 +115,7 @@ class SpacingApplier {
|
|||||||
// Stores the spacing (1/8 ic) and auto-space points's previous positions, for
|
// Stores the spacing (1/8 ic) and auto-space points's previous positions, for
|
||||||
// the previous item.
|
// the previous item.
|
||||||
Vector<OffsetWithSpacing, 16> offsets_with_spacing_;
|
Vector<OffsetWithSpacing, 16> offsets_with_spacing_;
|
||||||
|
InlineTextAutoSpace::Callback* callback_for_testing_ = nullptr;
|
||||||
};
|
};
|
||||||
|
|
||||||
} // namespace
|
} // namespace
|
||||||
@@ -151,8 +159,7 @@ void InlineTextAutoSpace::Initialize(const InlineItemsData& data) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
void InlineTextAutoSpace::Apply(InlineItemsData& data,
|
void InlineTextAutoSpace::Apply(InlineItemsData& data) {
|
||||||
Vector<wtf_size_t>* offsets_out) {
|
|
||||||
const String& text = data.text_content;
|
const String& text = data.text_content;
|
||||||
DCHECK(!text.Is8Bit());
|
DCHECK(!text.Is8Bit());
|
||||||
DCHECK_EQ(text.length(), ranges_.back().end);
|
DCHECK_EQ(text.length(), ranges_.back().end);
|
||||||
@@ -166,6 +173,7 @@ void InlineTextAutoSpace::Apply(InlineItemsData& data,
|
|||||||
// whether to add spacing into the bound of two items.
|
// whether to add spacing into the bound of two items.
|
||||||
TextDirection last_direction = TextDirection::kLtr;
|
TextDirection last_direction = TextDirection::kLtr;
|
||||||
SpacingApplier applier;
|
SpacingApplier applier;
|
||||||
|
applier.SetCallbackForTesting(callback_for_testing_);
|
||||||
for (const Member<InlineItem>& item_ptr : data.items) {
|
for (const Member<InlineItem>& item_ptr : data.items) {
|
||||||
const InlineItem& item = *item_ptr;
|
const InlineItem& item = *item_ptr;
|
||||||
if (item.Type() != InlineItem::kText) {
|
if (item.Type() != InlineItem::kText) {
|
||||||
@@ -275,11 +283,7 @@ void InlineTextAutoSpace::Apply(InlineItemsData& data,
|
|||||||
}
|
}
|
||||||
} while (offset < item.EndOffset());
|
} while (offset < item.EndOffset());
|
||||||
|
|
||||||
if (!offsets_out) {
|
applier.SetSpacing(offsets, &item, *style);
|
||||||
applier.SetSpacing(offsets, &item, *style);
|
|
||||||
} else {
|
|
||||||
offsets_out->AppendVector(offsets);
|
|
||||||
}
|
|
||||||
offsets.Shrink(0);
|
offsets.Shrink(0);
|
||||||
}
|
}
|
||||||
// Apply the pending spacing for the last item if needed.
|
// Apply the pending spacing for the last item if needed.
|
||||||
|
@@ -23,6 +23,12 @@ class CORE_EXPORT InlineTextAutoSpace : public TextAutoSpace {
|
|||||||
STACK_ALLOCATED();
|
STACK_ALLOCATED();
|
||||||
|
|
||||||
public:
|
public:
|
||||||
|
// A class for testing to inspect what `InlineTextAutoSpace` did.
|
||||||
|
class CORE_EXPORT Callback {
|
||||||
|
public:
|
||||||
|
virtual void DidApply(base::span<const OffsetWithSpacing>) = 0;
|
||||||
|
};
|
||||||
|
|
||||||
explicit InlineTextAutoSpace(const InlineItemsData& data);
|
explicit InlineTextAutoSpace(const InlineItemsData& data);
|
||||||
|
|
||||||
// True if this may apply auto-spacing. If this is false, it's safe to skip
|
// True if this may apply auto-spacing. If this is false, it's safe to skip
|
||||||
@@ -33,21 +39,22 @@ class CORE_EXPORT InlineTextAutoSpace : public TextAutoSpace {
|
|||||||
// https://drafts.csswg.org/css-text-4/#propdef-text-autospace
|
// https://drafts.csswg.org/css-text-4/#propdef-text-autospace
|
||||||
//
|
//
|
||||||
// The `data` must be the same instance as the one given to the constructor.
|
// The `data` must be the same instance as the one given to the constructor.
|
||||||
//
|
void Apply(InlineItemsData& data);
|
||||||
// If `offsets_out` is not null, the offsets of auto-space points are added to
|
void ApplyIfNeeded(InlineItemsData& data) {
|
||||||
// it without applying auto-spacing. This is for tseting-purpose.
|
|
||||||
void Apply(InlineItemsData& data, Vector<wtf_size_t>* offsets_out = nullptr);
|
|
||||||
void ApplyIfNeeded(InlineItemsData& data,
|
|
||||||
Vector<wtf_size_t>* offsets_out = nullptr) {
|
|
||||||
if (MayApply()) [[unlikely]] {
|
if (MayApply()) [[unlikely]] {
|
||||||
Apply(data, offsets_out);
|
Apply(data);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void SetCallbackForTesting(Callback* callback) {
|
||||||
|
callback_for_testing_ = callback;
|
||||||
|
}
|
||||||
|
|
||||||
private:
|
private:
|
||||||
void Initialize(const InlineItemsData& data);
|
void Initialize(const InlineItemsData& data);
|
||||||
|
|
||||||
InlineItemSegments::RunSegmenterRanges ranges_;
|
InlineItemSegments::RunSegmenterRanges ranges_;
|
||||||
|
Callback* callback_for_testing_ = nullptr;
|
||||||
};
|
};
|
||||||
|
|
||||||
inline InlineTextAutoSpace::InlineTextAutoSpace(const InlineItemsData& data) {
|
inline InlineTextAutoSpace::InlineTextAutoSpace(const InlineItemsData& data) {
|
||||||
|
@@ -21,6 +21,16 @@ using testing::ElementsAreArray;
|
|||||||
class InlineTextAutoSpaceTest : public RenderingTest,
|
class InlineTextAutoSpaceTest : public RenderingTest,
|
||||||
ScopedCSSTextAutoSpaceForTest {
|
ScopedCSSTextAutoSpaceForTest {
|
||||||
public:
|
public:
|
||||||
|
struct AutoSpaceCallback : public InlineTextAutoSpace::Callback {
|
||||||
|
void DidApply(base::span<const OffsetWithSpacing> applied_offsets) final {
|
||||||
|
for (const OffsetWithSpacing& offset : applied_offsets) {
|
||||||
|
offsets.push_back(offset.offset);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
Vector<wtf_size_t> offsets;
|
||||||
|
};
|
||||||
|
|
||||||
explicit InlineTextAutoSpaceTest() : ScopedCSSTextAutoSpaceForTest(true) {}
|
explicit InlineTextAutoSpaceTest() : ScopedCSSTextAutoSpaceForTest(true) {}
|
||||||
|
|
||||||
LayoutBlockFlow* PreparePageLayoutBlock(String html,
|
LayoutBlockFlow* PreparePageLayoutBlock(String html,
|
||||||
@@ -44,10 +54,11 @@ class InlineTextAutoSpaceTest : public RenderingTest,
|
|||||||
const LayoutBlockFlow* container =
|
const LayoutBlockFlow* container =
|
||||||
PreparePageLayoutBlock(html, container_css);
|
PreparePageLayoutBlock(html, container_css);
|
||||||
InlineNodeData* node_data = container->GetInlineNodeData();
|
InlineNodeData* node_data = container->GetInlineNodeData();
|
||||||
Vector<wtf_size_t> offsets;
|
|
||||||
InlineTextAutoSpace auto_space(*node_data);
|
InlineTextAutoSpace auto_space(*node_data);
|
||||||
auto_space.ApplyIfNeeded(*node_data, &offsets);
|
AutoSpaceCallback callback;
|
||||||
return offsets;
|
auto_space.SetCallbackForTesting(&callback);
|
||||||
|
auto_space.ApplyIfNeeded(*node_data);
|
||||||
|
return callback.offsets;
|
||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@@ -101,7 +101,7 @@ struct ShapeResultCharacterData {
|
|||||||
};
|
};
|
||||||
|
|
||||||
// A space should be appended after `offset` with the width of `spacing`.
|
// A space should be appended after `offset` with the width of `spacing`.
|
||||||
struct OffsetWithSpacing {
|
struct PLATFORM_EXPORT OffsetWithSpacing {
|
||||||
wtf_size_t offset;
|
wtf_size_t offset;
|
||||||
float spacing;
|
float spacing;
|
||||||
};
|
};
|
||||||
|
Reference in New Issue
Block a user