Improve CssClassSet add / remove

tools/dom/src/dart2js_CssClassSet.dart

Index: tools/dom/src/dart2js_CssClassSet.dart
diff --git a/tools/dom/src/dart2js_CssClassSet.dart b/tools/dom/src/dart2js_CssClassSet.dart
index cf5fa33660748dc256431f16a4a2704dcf39ccc8..b5fa0d68688bc44974f167138fc1720878ef8ac2 100644
--- a/tools/dom/src/dart2js_CssClassSet.dart
+++ b/tools/dom/src/dart2js_CssClassSet.dart
@@ -142,16 +142,15 @@ class _ElementCssClassSet extends CssClassSetImpl {
 
   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;
   }
@@ -225,7 +224,17 @@ class _ElementCssClassSet extends CssClassSetImpl {
       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) {