Title: [129186] trunk/Source/WebCore
- Revision
- 129186
- Author
- le...@chromium.org
- Date
- 2012-09-20 18:38:05 -0700 (Thu, 20 Sep 2012)
Log Message
Prevent reading stale data from InlineTextBoxes
https://bugs.webkit.org/show_bug.cgi?id=94750
Reviewed by Abhishek Arya.
Text from dirty InlineTextBoxes should never be read or used. This change
enforces this design goal by forcefully zero-ing out the start and length
of InlineTextBoxes when they're being marked dirty. Ideally, we'd also
add asserts to the accessors for this data, but there are still several
places in editing that cause this. https://bugs.webkit.org/show_bug.cgi?id=97264
tracks these cases.
This change involves making markDirty virtual. Running the line-layout
performance test as well as profiling resizing the html5 spec showed
negligable impact with this change.
No new tests as this doesn't change any proper behavior.
* dom/Position.cpp:
(WebCore::Position::downstream): Adding a FIXME.
* rendering/InlineBox.h:
(WebCore::InlineBox::markDirty): Marking virtual to allow InlineTextBox to
overload and zero out its start and length.
* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::markDirty): Zeroing out the start and length when
we mark the box dirty.
* rendering/InlineTextBox.h:
* rendering/RenderText.cpp:
(WebCore::RenderText::setTextWithOffset): Adding a FIXME.
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (129185 => 129186)
--- trunk/Source/WebCore/ChangeLog 2012-09-21 01:35:17 UTC (rev 129185)
+++ trunk/Source/WebCore/ChangeLog 2012-09-21 01:38:05 UTC (rev 129186)
@@ -1,3 +1,35 @@
+2012-09-20 Levi Weintraub <le...@chromium.org>
+
+ Prevent reading stale data from InlineTextBoxes
+ https://bugs.webkit.org/show_bug.cgi?id=94750
+
+ Reviewed by Abhishek Arya.
+
+ Text from dirty InlineTextBoxes should never be read or used. This change
+ enforces this design goal by forcefully zero-ing out the start and length
+ of InlineTextBoxes when they're being marked dirty. Ideally, we'd also
+ add asserts to the accessors for this data, but there are still several
+ places in editing that cause this. https://bugs.webkit.org/show_bug.cgi?id=97264
+ tracks these cases.
+
+ This change involves making markDirty virtual. Running the line-layout
+ performance test as well as profiling resizing the html5 spec showed
+ negligable impact with this change.
+
+ No new tests as this doesn't change any proper behavior.
+
+ * dom/Position.cpp:
+ (WebCore::Position::downstream): Adding a FIXME.
+ * rendering/InlineBox.h:
+ (WebCore::InlineBox::markDirty): Marking virtual to allow InlineTextBox to
+ overload and zero out its start and length.
+ * rendering/InlineTextBox.cpp:
+ (WebCore::InlineTextBox::markDirty): Zeroing out the start and length when
+ we mark the box dirty.
+ * rendering/InlineTextBox.h:
+ * rendering/RenderText.cpp:
+ (WebCore::RenderText::setTextWithOffset): Adding a FIXME.
+
2012-09-20 Adam Barth <aba...@webkit.org>
Measure how often web pages use Worker and SharedWorker
Modified: trunk/Source/WebCore/dom/Position.cpp (129185 => 129186)
--- trunk/Source/WebCore/dom/Position.cpp 2012-09-21 01:35:17 UTC (rev 129185)
+++ trunk/Source/WebCore/dom/Position.cpp 2012-09-21 01:38:05 UTC (rev 129186)
@@ -718,6 +718,7 @@
// and upstream() will return the left one.
// Also, downstream() will return the last position in the last atomic node in boundary for all of the positions
// in boundary after the last candidate, where endsOfNodeAreVisuallyDistinctPositions(boundary).
+// FIXME: This function should never be called when the line box tree is dirty. See https://bugs.webkit.org/show_bug.cgi?id=97264
Position Position::downstream(EditingBoundaryCrossingRule rule) const
{
Node* startNode = deprecatedNode();
Modified: trunk/Source/WebCore/rendering/InlineBox.h (129185 => 129186)
--- trunk/Source/WebCore/rendering/InlineBox.h 2012-09-21 01:35:17 UTC (rev 129185)
+++ trunk/Source/WebCore/rendering/InlineBox.h 2012-09-21 01:38:05 UTC (rev 129186)
@@ -262,7 +262,7 @@
virtual void clearTruncation() { }
bool isDirty() const { return m_bitfields.dirty(); }
- void markDirty(bool dirty = true) { m_bitfields.setDirty(dirty); }
+ virtual void markDirty(bool dirty = true) { m_bitfields.setDirty(dirty); }
virtual void dirtyLineBoxes();
Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (129185 => 129186)
--- trunk/Source/WebCore/rendering/InlineTextBox.cpp 2012-09-21 01:35:17 UTC (rev 129185)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp 2012-09-21 01:38:05 UTC (rev 129186)
@@ -64,6 +64,15 @@
InlineBox::destroy(arena);
}
+void InlineTextBox::markDirty(bool dirty)
+{
+ if (dirty) {
+ m_len = 0;
+ m_start = 0;
+ }
+ InlineBox::markDirty(dirty);
+}
+
LayoutRect InlineTextBox::logicalOverflowRect() const
{
if (knownToHaveNoOverflow() || !gTextBoxesWithOverflow)
Modified: trunk/Source/WebCore/rendering/InlineTextBox.h (129185 => 129186)
--- trunk/Source/WebCore/rendering/InlineTextBox.h 2012-09-21 01:35:17 UTC (rev 129185)
+++ trunk/Source/WebCore/rendering/InlineTextBox.h 2012-09-21 01:38:05 UTC (rev 129186)
@@ -64,6 +64,7 @@
void setNextTextBox(InlineTextBox* n) { m_nextTextBox = n; }
void setPreviousTextBox(InlineTextBox* p) { m_prevTextBox = p; }
+ // FIXME: These accessors should ASSERT(!isDirty()). See https://bugs.webkit.org/show_bug.cgi?id=97264
unsigned start() const { return m_start; }
unsigned end() const { return m_len ? m_start + m_len - 1 : m_start; }
unsigned len() const { return m_len; }
@@ -71,10 +72,12 @@
void setStart(unsigned start) { m_start = start; }
void setLen(unsigned len) { m_len = len; }
- void offsetRun(int d) { m_start += d; }
+ void offsetRun(int d) { ASSERT(!isDirty()); m_start += d; }
unsigned short truncation() { return m_truncation; }
+ virtual void markDirty(bool dirty = true) OVERRIDE;
+
using InlineBox::hasHyphen;
using InlineBox::setHasHyphen;
using InlineBox::canHaveLeadingExpansion;
Modified: trunk/Source/WebCore/rendering/RenderText.cpp (129185 => 129186)
--- trunk/Source/WebCore/rendering/RenderText.cpp 2012-09-21 01:35:17 UTC (rev 129185)
+++ trunk/Source/WebCore/rendering/RenderText.cpp 2012-09-21 01:38:05 UTC (rev 129186)
@@ -1268,6 +1268,7 @@
// Dirty all text boxes that include characters in between offset and offset+len.
for (InlineTextBox* curr = firstTextBox(); curr; curr = curr->nextTextBox()) {
+ // FIXME: This shouldn't rely on the end of a dirty line box. See https://bugs.webkit.org/show_bug.cgi?id=97264
// Text run is entirely before the affected range.
if (curr->end() < offset)
continue;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo/webkit-changes
