Commit 8815502e authored by Etienne Bergeron's avatar Etienne Bergeron Committed by Commit Bot

Add sanity checks to text properties bounds in RenderText

This CL is adding sanity check to ensure no components in chrome are
using incorrectly the RenderText API.

We recently observed a crash (crbug/1124118) that was caused by
setting a property outside of the text bounds.

The RenderText API assume correct uses of these properties. With this
CL they will be reported as a crash reports.

Bug: 1142020
Change-Id: I58d19bbbfeca97ad8425706eb85a825105db9f21
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2518895
Commit-Queue: Etienne Bergeron <etienneb@chromium.org>
Reviewed-by: default avatarMichael Wasserman <msw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823866}
parent 5ac05eb2
......@@ -11,6 +11,8 @@
#include "base/check_op.h"
#include "base/command_line.h"
#include "base/debug/alias.h"
#include "base/debug/dump_without_crashing.h"
#include "base/i18n/break_iterator.h"
#include "base/i18n/char_iterator.h"
#include "base/notreached.h"
......@@ -261,6 +263,22 @@ int GetLineSegmentContainingXCoord(const internal::Line& line,
return line.segments.size();
}
// This code is a crash debugging helper for http://crbug.com/1142020. Remove
// this code when the bug is fixed.
void EnsureRangeWithinTextBounds(const Range& range,
const base::string16& text) {
static bool already_uploaded = false;
if (!already_uploaded && !range.is_empty() && range.end() > text.size()) {
already_uploaded = true;
NOTREACHED() << "Applying a text property outside of text bounds.";
std::string text_utf8 = base::UTF16ToUTF8(text);
DEBUG_ALIAS_FOR_CSTR(text_copy_, text_utf8.c_str(), 32);
Range range_copy = range;
base::debug::Alias(&range_copy);
base::debug::DumpWithoutCrashing();
}
}
} // namespace
namespace internal {
......@@ -815,6 +833,7 @@ void RenderText::SetColor(SkColor value) {
}
void RenderText::ApplyColor(SkColor value, const Range& range) {
EnsureRangeWithinTextBounds(range, text_);
colors_.ApplyValue(value, range);
OnLayoutTextAttributeChanged(false);
}
......@@ -825,12 +844,14 @@ void RenderText::SetBaselineStyle(BaselineStyle value) {
}
void RenderText::ApplyBaselineStyle(BaselineStyle value, const Range& range) {
EnsureRangeWithinTextBounds(range, text_);
baselines_.ApplyValue(value, range);
OnLayoutTextAttributeChanged(false);
}
void RenderText::ApplyFontSizeOverride(int font_size_override,
const Range& range) {
EnsureRangeWithinTextBounds(range, text_);
font_size_overrides_.ApplyValue(font_size_override, range);
OnLayoutTextAttributeChanged(false);
}
......@@ -845,6 +866,7 @@ void RenderText::SetStyle(TextStyle style, bool value) {
}
void RenderText::ApplyStyle(TextStyle style, bool value, const Range& range) {
EnsureRangeWithinTextBounds(range, text_);
styles_[style].ApplyValue(value, range);
cached_bounds_and_offset_valid_ = false;
......@@ -861,6 +883,7 @@ void RenderText::SetWeight(Font::Weight weight) {
}
void RenderText::ApplyWeight(Font::Weight weight, const Range& range) {
EnsureRangeWithinTextBounds(range, text_);
weights_.ApplyValue(weight, range);
cached_bounds_and_offset_valid_ = false;
......
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