Skip to content

GitLab

T
tangled
Commit e18ac62b authored by Darren Shen's avatar Darren Shen Committed by Commit Bot
Browse files
Options

Prevent counter values from over/underflowing. 

Currently we over/underflow if a counter value goes beyond the limits
of a 32 bit integer. The spec [1] says we can ignore the increment
if it would cause an over/underflow. This patch implements this
behaviour.

Firefox follows the spec in this.

[1] https://drafts.csswg.org/css-lists-3/#valdef-counter-reset-custom-ident-integer

Bug: 655473
Change-Id: I6e10b6e4f5f672d723a0ccb69fd309d52d1d1204
Reviewed-on: https://chromium-review.googlesource.com/708105Reviewed-by: default avatarJustin Schuh <jschuh@chromium.org>
Reviewed-by: default avatarEric Willigers <ericwilligers@chromium.org>
Commit-Queue: Darren Shen <shend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509351}
parent 6c57517d
Hide whitespace changes
Inline Side-by-side
Showing with 79 additions and 5 deletions
third_party/WebKit/LayoutTests/fast/css/counters/counter-overflow.html 0 → 100644
View file @ e18ac62b
<!DOCTYPE html>
<script src="../../../resources/testharness.js"></script>
<script src="../../../resources/testharnessreport.js"></script>
<style>
.overflow {
counter-reset: c 0;
}
.overflow h3:before {
counter-increment: c 500000000;
}
.underflow {
counter-reset: c 0;
}
.underflow h3:before {
counter-increment: c -500000000;
}
h3::before {
content: counter(c);
}
</style>
<div id="headings">
</div>
<script>
var headings = document.getElementById('headings');
var items = [];
for (var i = 0; i < 5; i++) {
items.push(document.createElement('h3'));
headings.appendChild(items[i]);
}
function get_list_counters() {
var counters = [];
for (var i = 0; i < items.length; i++) {
counters.push(parseInt(window.internals.counterValue(items[i])));
}
return counters;
}
/* The spec [1] allows implementation-specific limits on the value of counters.
The only requirement is that "If an increment would push the counter’s value
beyond these limits, the increment must be ignored, and the counter’s value
remain unchanged."
We assume a 32 bit int limit.
*/
test(function() {
headings.className = 'overflow';
assert_array_equals(get_list_counters(),
[500000000, 1000000000, 1500000000, 2000000000, 2000000000]);
}, 'large reset does not overflow');
test(function() {
headings.className = 'underflow';
assert_array_equals(get_list_counters(),
[-500000000, -1000000000, -1500000000, -2000000000, -2000000000]);
}, 'small reset does not underflow');
</script>
third_party/WebKit/Source/core/layout/CounterNode.cpp
View file @ e18ac62b
...@@ -22,6 +22,7 @@ ...@@ -22,6 +22,7 @@
#include "core/layout/CounterNode.h" #include "core/layout/CounterNode.h"
#include "core/layout/LayoutCounter.h" #include "core/layout/LayoutCounter.h"
#include "platform/wtf/CheckedNumeric.h"
#ifndef NDEBUG #ifndef NDEBUG
#include <stdio.h> #include <stdio.h>
...@@ -142,11 +143,17 @@ CounterNode* CounterNode::PreviousInPreOrder() const { ...@@ -142,11 +143,17 @@ CounterNode* CounterNode::PreviousInPreOrder() const {
} }
int CounterNode::ComputeCountInParent() const { int CounterNode::ComputeCountInParent() const {
// According to the spec, if an increment would overflow or underflow the
// counter, we are allowed to ignore the increment.
// https://drafts.csswg.org/css-lists-3/#valdef-counter-reset-custom-ident-integer
int increment = ActsAsReset() ? 0 : value_; int increment = ActsAsReset() ? 0 : value_;
if (previous_sibling_) if (previous_sibling_) {
return previous_sibling_->count_in_parent_ + increment; return WTF::CheckAdd(previous_sibling_->count_in_parent_, increment)
.ValueOrDefault(previous_sibling_->count_in_parent_);
}
DCHECK_EQ(parent_->first_child_, this); DCHECK_EQ(parent_->first_child_, this);
return parent_->value_ + increment; return WTF::CheckAdd(parent_->value_, increment)
.ValueOrDefault(parent_->value_);
} }
void CounterNode::AddLayoutObject(LayoutCounter* value) { void CounterNode::AddLayoutObject(LayoutCounter* value) {
......
third_party/WebKit/Source/core/style/CounterDirectives.h
View file @ e18ac62b
...@@ -27,6 +27,7 @@ ...@@ -27,6 +27,7 @@
#include <memory> #include <memory>
#include "platform/wtf/Allocator.h" #include "platform/wtf/Allocator.h"
#include "platform/wtf/CheckedNumeric.h"
#include "platform/wtf/HashMap.h" #include "platform/wtf/HashMap.h"
#include "platform/wtf/MathExtras.h" #include "platform/wtf/MathExtras.h"
#include "platform/wtf/RefPtr.h" #include "platform/wtf/RefPtr.h"
...@@ -83,8 +84,11 @@ class CounterDirectives { ...@@ -83,8 +84,11 @@ class CounterDirectives {
int CombinedValue() const { int CombinedValue() const {
DCHECK(is_reset_set_ || !reset_value_); DCHECK(is_reset_set_ || !reset_value_);
DCHECK(is_increment_set_ || !increment_value_); DCHECK(is_increment_set_ || !increment_value_);
// FIXME: Shouldn't allow overflow here. // According to the spec, if an increment would overflow or underflow the
return reset_value_ + increment_value_; // counter, we are allowed to ignore the increment.
// https://drafts.csswg.org/css-lists-3/#valdef-counter-reset-custom-ident-integer
return WTF::CheckAdd(reset_value_, increment_value_)
.ValueOrDefault(reset_value_);
} }
private: private:
......
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