Should not ASSERT what we know will fail.

The input to the selector position and the rule position is known to possibly overflow 12 and 18 bits respectively since the input comes from unsigned positions into data structures known to keep more elements than that. The selector and rule positions will be wrong regardlessly, unless we make RuleData bigger. Truncating the position to a max value instead of just letting it overflow could be an option. It doesn't make sense for selector position as trying to match any of the other selectors won't make any difference. Truncating the rule position would make the cascading order correct when comparing rules in the representable range with a rule outside the range, while just letting it overflow starting from zero again will yield a correct cascading order between two rules both outside of the representable range. Given that, I conclude to just remove the ASSERTs and not do anything clever about values outside of the ranges. BUG=519315 R=timloh@chromium.org Review URL: https://codereview.chromium.org/1312703002 git-svn-id: svn://svn.chromium.org/blink/trunk@201059 bbb929c8-8fbe-4397-9dbb-9b2b20218538

Should not ASSERT what we know will fail.
The input to the selector position and the rule position is known to possibly overflow 12 and 18 bits respectively since the input comes from unsigned positions into data structures known to keep more elements than that. The selector and rule positions will be wrong regardlessly, unless we make RuleData bigger. Truncating the position to a max value instead of just letting it overflow could be an option. It doesn't make sense for selector position as trying to match any of the other selectors won't make any difference. Truncating the rule position would make the cascading order correct when comparing rules in the representable range with a rule outside the range, while just letting it overflow starting from zero again will yield a correct cascading order between two rules both outside of the representable range. Given that, I conclude to just remove the ASSERTs and not do anything clever about values outside of the ranges. BUG=519315 R=timloh@chromium.org Review URL: https://codereview.chromium.org/1312703002 git-svn-id: svn://svn.chromium.org/blink/trunk@201059 bbb929c8-8fbe-4397-9dbb-9b2b20218538
741c9ed2 · rune@opera.com · e1127317 · 741c9ed2 · 741c9ed2 · 741c9ed2
Commit 741c9ed2 authored Aug 24, 2015 by rune@opera.com
3 changed files
--- a/third_party/WebKit/LayoutTests/fast/css/long-selector-list-assert-expected.txt
+++ b/third_party/WebKit/LayoutTests/fast/css/long-selector-list-assert-expected.txt
+PASS if no crash or assert.
--- a/third_party/WebKit/LayoutTests/fast/css/long-selector-list-assert.html
+++ b/third_party/WebKit/LayoutTests/fast/css/long-selector-list-assert.html
+<!DOCTYPE html>
+<script>
+    if (window.testRunner)
+        testRunner.dumpAsText();
+    var selector = "";
+    for (var i=0; i<4096; i++)
+        selector += "#id" + i + ", ";
+    selector += "#assert ";
+    var stylesheet = "<style>" + selector + "{ color: pink }</style>";
+    document.write(stylesheet);
+</script>
+<p>PASS if no crash or assert.</p>
--- a/third_party/WebKit/Source/core/css/RuleSet.cpp
+++ b/third_party/WebKit/Source/core/css/RuleSet.cpp
@@ -116,8 +116,6 @@ RuleData::RuleData(StyleRule* rule, unsigned selectorIndex, unsigned position, A
    , m_hasDocumentSecurityOrigin(addRuleFlags & RuleHasDocumentSecurityOrigin)
    , m_propertyWhitelistType(determinePropertyWhitelistType(addRuleFlags, selector()))
 {
-    ASSERT(m_position == position);
-    ASSERT(m_selectorIndex == selectorIndex);
    SelectorFilter::collectIdentifierHashes(selector(), m_descendantSelectorIdentifierHashes, maximumIdentifierCount);
 }