Return ActiveSheetsChanged when rulesets change in common prefix.

third_party/WebKit/Source/core/css/ActiveStyleSheets.cpp

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/css/ActiveStyleSheets.h"
 
 #include "core/css/CSSStyleSheet.h"
 #include "core/css/RuleSet.h"
 #include "core/css/resolver/ScopedStyleResolver.h"
 #include "core/dom/ContainerNode.h"
@@ skipping 20 matching lines @@
     if (newStyleSheets[index].second == oldStyleSheets[index].second)
       continue;
 
     if (newStyleSheets[index].second)
       changedRuleSets.add(newStyleSheets[index].second);
     if (oldStyleSheets[index].second)
       changedRuleSets.add(oldStyleSheets[index].second);
   }
 
   if (index == oldStyleSheetCount) {
-    if (index == newStyleSheetCount) {
-      return changedRuleSets.isEmpty() ? NoActiveSheetsChanged
-                                       : ActiveSheetsChanged;
-    }
-
-    // Sheets added at the end.
+    bool ruleSetsChanged = !changedRuleSets.isEmpty();

[sashab, 2017/01/25 02:45:00]

Hmm, is this the same logic as before? This is way better. :) Just make this:

if (!changedRuleSets.isEmpty())
    return ActiveSheetsChanged;

and remove it from below.

[meade_UTC10, 2017/01/25 05:14:40]

I don't think that'd have the same behaviour, Sasha :)

But I do think it'd be clearer to just use this condition directly in the if
statement, since you only use the variable ruleSetsChanged once.

[rune, 2017/01/25 08:57:34]

The isEmpty check must be done before the for-loop. See the other reply.

[rune, 2017/01/25 08:57:34]

Those are two different tests. The test here is the key of this fix.

Given that upper case letters are style sheets and lower case are their rulesets
below.

If we have stylesheets appended to the end, we can return ActiveSheetsAppended
and just add C and D to the ScopedStyleResolver like in this case:

old:

AB
ab

new

ABCD
abcd

But, if you have:

old:

AB
ab

new:

ABCD
aecd

You cannot simply add C and D because we would not have the ScopedStyleResolver
pick up the b -> e change. So instead we need to re-add all sheets to
ScopedStyleResolver, which is what ActiveSheetsChanged does.

We cannot do an early return here since we need the trailing sheets' rulesets
added to the changedRuleSets.

The changedRuleSets.isEmpty() below is to check if the trailing sheets added any
rulesets.

     for (; index < newStyleSheetCount; index++) {
       if (newStyleSheets[index].second)
         changedRuleSets.add(newStyleSheets[index].second);
     }
-    return changedRuleSets.isEmpty() ? NoActiveSheetsChanged
-                                     : ActiveSheetsAppended;
+    if (ruleSetsChanged)
+      return ActiveSheetsChanged;
+    if (changedRuleSets.isEmpty())
+      return NoActiveSheetsChanged;
+    return ActiveSheetsAppended;
   }
 
   if (index == newStyleSheetCount) {
     // Sheets removed from the end.
     for (; index < oldStyleSheetCount; index++) {
       if (oldStyleSheets[index].second)
         changedRuleSets.add(oldStyleSheets[index].second);
     }
     return changedRuleSets.isEmpty() ? NoActiveSheetsChanged
                                      : ActiveSheetsChanged;
@@ skipping 37 matching lines @@
     if (sheet1.second)
       changedRuleSets.add(sheet1.second);
     if (sheet2.second)
       changedRuleSets.add(sheet2.second);
   }
   return changedRuleSets.isEmpty() ? NoActiveSheetsChanged
                                    : ActiveSheetsChanged;
 }
 
 }  // namespace blink