Improve CssClassSet add / remove

tools/dom/src/dart2js_CssClassSet.dart

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 part of html;
 
 /**
  * A set (union) of the CSS classes that are present in a set of elements.
  * Implemented separately from _ElementCssClassSet for performance.
  */
@@ skipping 124 matching lines @@
   void retainWhere(bool test(String name)) {
     _removeWhere(_element, test, false);
   }
 
   static bool _contains(Element _element, Object value) {
     return value is String && _classListContains(_classListOf(_element), value);
   }
 
   static bool _add(Element _element, String value) {
     DomTokenList list = _classListOf(_element);
-    // Compute returned result independently of action upon the set. One day we
-    // will be able to optimize it way if unused.
-    bool added = !_classListContains(list, value);
+    // Compute returned result independently of action upon the set.
+    bool added = !_classListContainsBeforeAddRemove(list, value);
     _classListAdd(list, value);
     return added;
   }
 
   static bool _remove(Element _element, String value) {
     DomTokenList list = _classListOf(_element);
-    bool removed = _classListContains(list, value);
+    bool removed = _classListContainsBeforeAddRemove(list, value);
     _classListRemove(list, value);
     return removed;
   }
 
   static bool _toggle(Element _element, String value, bool shouldAdd) {
     // There is no value that can be passed as the second argument of
     // DomTokenList.toggle that behaves the same as passing one argument.
     // `null` is seen as false, meaning 'remove'.
     return shouldAdd == null
         ? _toggleDefault(_element, value)
@@ skipping 53 matching lines @@
   // the DomTokenList methods.
 
   static DomTokenList _classListOf(Element e) =>
       JS('returns:DomTokenList;creates:DomTokenList;effects:none;depends:all;',
          '#.classList', e);
 
   static int _classListLength(DomTokenList list) =>
       JS('returns:JSUInt31;effects:none;depends:all;', '#.length', list);
 
   static bool _classListContains(DomTokenList list, String value) =>
-      JS('returns:bool;effects:none;depends:all;',
+      JS('returns:bool;effects:none;depends:all',
+          '#.contains(#)', list, value);
+
+  static bool _classListContainsBeforeAddRemove(

[Siggi Cherem (dart-lang), 2015/04/21 05:07:25]

optional minor nit: ...AddRemove => ...AddOrRemove?

[sra1, 2015/04/21 19:38:08]

Done.

+      DomTokenList list, String value) =>
+      // 'throws:never' is a lie, since 'contains' will throw on an illegal

[Siggi Cherem (dart-lang), 2015/04/21 05:07:25]

silly question just because I'm not used to these annotations yet: I assume
null(1) == never?

[sra1, 2015/04/21 19:38:08]

null(1) means throws iff first # is null.
It doesn't really matter because _classListOf returns non-null.

[Siggi Cherem (dart-lang), 2015/04/21 22:32:50]

since this is anyways lying, any reason not to just make it 'throws:never'?

+      // token.  However, we always call this function immediately prior to
+      // add/remove/toggle with the same token.  Often the result of 'contains'
+      // is unused and the lie makes it possible for the 'contains' instruction
+      // to be removed.
+      JS('returns:bool;effects:none;depends:all;throws:null(1)',
           '#.contains(#)', list, value);
 
   static void _classListAdd(DomTokenList list, String value) {
     // list.add(value);
     JS('', '#.add(#)', list, value);
   }
 
   static void _classListRemove(DomTokenList list, String value) {
     // list.remove(value);
     JS('', '#.remove(#)', list, value);
   }
 
   static bool _classListToggle1(DomTokenList list, String value) {
     return JS('bool', '#.toggle(#)', list, value);
   }
 
   static bool _classListToggle2(
       DomTokenList list, String value, bool shouldAdd) {
     return JS('bool', '#.toggle(#, #)', list, value, shouldAdd);
   }
 }