Make spellcheck_test easier to use

third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js

Index: third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js
diff --git a/third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js b/third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js
index c98232bd9d33690a6f8a6834644254994e325c0f..01a6d7181fd9e6a165148311f79c6941fa787eff 100644
--- a/third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js
+++ b/third_party/WebKit/LayoutTests/editing/spelling/spellcheck_test.js
@@ -43,197 +43,300 @@
 (function() {
 const Sample = window.Sample;
 
-/** @type {string} */
-const kSpelling = 'spelling';
-/** @type {string} */
-const kGrammar = 'grammar';
+// TODO(editing-dev): Once we can import JavaScript file from scripts, we should
+// import "imported/wpt/html/resources/common.js", since |HTML5_VOID_ELEMENTS|
+// is defined in there.
+/**
+ * @const @type {!Set<string>}
+ * only void (without end tag) HTML5 elements
+ */
+const HTML5_VOID_ELEMENTS = new Set([
+  'area', 'base', 'br', 'col', 'command', 'embed', 'hr', 'img', 'input',
+  'keygen', 'link', 'meta', 'param', 'source','track', 'wbr' ]);
 
-class Marker {
-  /**
-   * @public
-   * @param {number} location
-   * @param {number} length
-   * @param {string=} opt_type
-   * @param {string=} opt_description
-   */
-  constructor(location, length, opt_type, opt_description) {
-    /** @type {number} */
-    this.location_ = location;
-    /** @type {number} */
-    this.length_ = length;
-    /** @type {string} */
-    this.type_ = opt_type || 'spelling';
-    /** @type {boolean} */
-    this.ignoreDescription_ = opt_description === undefined;
-    /** @type {string} */
-    this.description_ = opt_description || '';
-  }
+// TODO(editing-dev): Reduce code duplication with assert_selection's Serializer
+// once we can import and export Javascript modules.
 
-  /** @return {number} */
-  get location() { return this.location_; }
+/**
+ * @param {!Node} node
+ * @return {boolean}
+ */
+function isCharacterData(node) {
+  return node.nodeType === Node.TEXT_NODE ||
+      node.nodeType === Node.COMMENT_NODE;
+}
 
-  /** @return {number} */
-  get length() { return this.length_; }
+/**
+ * @param {!Node} node
+ * @return {boolean}
+ */
+function isElement(node) {
+  return node.nodeType === Node.ELEMENT_NODE;
+}
 
-  /** @return {string} */
-  get type() { return this.type_; }
+/**
+ * @param {!Node} node
+ * @return {boolean}
+ */
+function isHTMLInputElement(node) {
+  if (!isElement(node))

[yosin_UTC9, 2016/10/25 09:41:11]

|node.nodeName === 'INPUT'| is enough.

[Xiaocheng, 2016/10/25 13:19:17]

Done.

+    return false;
+  const window = node.ownerDocument.defaultView;
+  return node instanceof window.HTMLInputElement;
+}
 
-  /** @return {boolean} */
-  get ignoreDescription() { return this.ignoreDescription_; }
+/**
+ * @param {!Node} node
+ * @return {boolean}
+ */
+function isHTMLTextAreaElement(node) {
+  if (!isElement(node))

[yosin_UTC9, 2016/10/25 09:41:11]

|node.nodeName === 'TEXTAREA'| is enough.

[Xiaocheng, 2016/10/25 13:19:17]

Done.

+    return false;
+  const window = node.ownerDocument.defaultView;
+  return node instanceof window.HTMLTextAreaElement;
+}
 
-  /** @return {string} */
-  get description() { return this.description_; }
+/**
+ * @param {?Range} range
+ * @param {!Node} node
+ * @param {number} offset
+ */
+function isAtRangeEnd(range, node, offset) {
+  return range && node === range.endContainer && offset === range.endOffset;
+}
 
+class MarkerSerializer {
   /**
    * @public
+   * @param {!Array<string>} markerTypes
+   * @param {string} markerMasks
    */
-  assertValid() {
-    // TODO(xiaochengh): Add proper assert descriptions when needed.
-    assert_true(Number.isInteger(this.location_));
-    assert_greater_than_equal(this.location_, 0);
-    assert_true(Number.isInteger(this.length_));
-    assert_greater_than(this.length_, 0);
-    assert_true(this.type_ === kSpelling || this.type_ === kGrammar);
-    assert_true(typeof this.description_ === 'string');
+  constructor(markerTypes, markerMasks) {
+    assert_equals(markerMasks.length + 1, 1 << markerTypes.length,
+                 'Maker types and masks do not match each other.');
+    /** @type {!Array<string>} */
+    this.strings_ = [];
+    /** @type {!Array<string>} */
+    this.markerTypes_ = markerTypes;
+    /** @type {string} */
+    this.markerMasks_ = markerMasks;
+    /** @type {!Object} */
+    this.activeMarkerRanges_ = {};
+    markerTypes.forEach(type => this.activeMarkerRanges_[type] = null);
   }
 
   /**
-   * @public
-   * @param {!Marker} expected
+   * @private
+   * @return {number}
+   */
+  get activeMarkerTypes() {

[yosin_UTC9, 2016/10/25 09:41:11]

This should be a method instead of a getter, since an implementation isn't
simple.

[Xiaocheng, 2016/10/25 13:19:17]

Done.

+    return this.markerTypes_.reduce(
+        (result, type, index) =>
+            this.activeMarkerRanges_[type] ? (result | (1 << index)) : result,
+        0);
+  }
+
+  /**
+   * @private
+   * @param {string} string
    */
-  assertMatch(expected) {
-    try {
-      assert_equals(this.location, expected.location);
-      assert_equals(this.length, expected.length);
-      assert_equals(this.type, expected.type);
-      if (expected.ignoreDescription)
+  emit(string) { this.strings_.push(string); }
+
+  /**
+   * @private
+   * @param {!Node} node
+   * @param {number} offset
+   */
+  advancedTo(node, offset) {
+    this.markerTypes_.forEach(type => {
+      if (isAtRangeEnd(this.activeMarkerRanges_[type], node, offset))
+        this.activeMarkerRanges_[type] = null;
+      if (this.activeMarkerRanges_[type])
         return;
-      assert_equals(this.description, expected.description);
-    } catch (error) {
-      throw new Error(`Expected ${expected} but got ${this}.`);
+      const markerCount = window.internals.markerCountForNode(node, type);
+      for (let i = 0; i < markerCount; ++i) {
+        const marker = window.internals.markerRangeForNode(node, type, i);
+        assert_equals(marker.startContainer, node,

[yosin_UTC9, 2016/10/25 09:41:10]

We don't need to verity marker. It should work as expected.

[Xiaocheng, 2016/10/25 13:19:17]

I prefer keeping them in case anything goes wrong. I've changed the failure
message to emphasize that it's an internal failure instead of due to a wrong
test case.

+                      'Marker range does not start in the annotated node.');
+        assert_equals(marker.endContainer, node,
+                      'Marker range does not end in the annotated node.');
+        if (marker.startOffset == offset) {

[yosin_UTC9, 2016/10/25 09:41:11]

nit: s/==/===/

[Xiaocheng, 2016/10/25 13:19:16]

Done.

+          assert_greater_than(marker.endOffset, offset,
+                              'Marker range is collapsed.');
+          this.activeMarkerRanges_[type] = marker;
+          break;
+        }
+      }
+    });
+  }
+
+  /**
+   * @private
+   * @param {!CharacterData} node
+   */
+  handleCharacterData(node) {
+    /** @type {string} */
+    const text = node.nodeValue;
+    /** @type {number} */
+    const length = text.length;
+    for (let offset = 0; offset < length; ++offset) {
+      this.advancedTo(node, offset);
+      /** @type {number} */
+      const activeTypes = this.activeMarkerTypes;
+      /** @type {string} */
+      const charToEmit = activeTypes ? this.markerMasks_[activeTypes - 1] : text[offset];
+      this.emit(charToEmit);
     }
+    this.advancedTo(node, length);
   }
 
-  /** @override */
-  toString() {
-    return `${this.type_} marker at ` +
-        `[${this.location_}, ${this.location_ + this.length_}]` +
-        (this.description_ ? ` with description "${this.description_}"` : ``);
+  /**
+   * @private
+   * @param {!HTMLElement} element
+   */
+  handleInnerEditorOf(element) {
+    /** @type {!ShadowRoot} */
+    const shadowRoot = window.internals.shadowRoot(element);
+    /** @type {!HTMLDivElement} */
+    const innerEditor = shadowRoot.firstChild;
+    assert_equals(innerEditor.tagName, 'DIV');
+    innerEditor.childNodes.forEach((child, index) => {
+      assert_true(isCharacterData(child));
+      this.advancedTo(innerEditor, index);
+      this.handleCharacterData(child);
+    });
+    this.advancedTo(innerEditor, innerEditor.childNodes.length);
   }
-}
 
-/**
- * @param {number} location
- * @param {number} length
- * @param {string=} opt_description
- * @return {!Marker}
- */
-function spellingMarker(location, length, opt_description) {
-  return new Marker(location, length, kSpelling, opt_description);
-}
+  /**
+   * @private
+   * @param {!HTMLTextAreaElement} element
+   */
+  handleTextAreaNode(element) {
+    this.handleInnerEditorOf(element);
+  }
 
-/**
- * @param {number} location
- * @param {number} length
- * @param {string=} opt_description
- * @return {!Marker}
- */
-function grammarMarker(location, length, opt_description) {
-  return new Marker(location, length, kGrammar, opt_description);
-}
+  /**
+   * @private
+   * @param {!HTMLInputElement} element
+   */
+  handleInputNode(element) {
+    this.emit(' value="');
+    this.handleInnerEditorOf(element);
+    this.emit('"');
+  }
 
-/**
- * @param {!Marker} marker1
- * @param {!Marker} marker2
- * @return {number}
- */
-function markerComparison(marker1, marker2) {
-  return marker1.location - marker2.location;
-}
+  /**
+   * @private
+   * @param {!HTMLElement} element
+   */
+  handleElementNode(element) {
+    /** @type {string} */
+    const tagName = element.tagName.toLowerCase();
+    this.emit(`<${tagName}`);
+    Array.from(element.attributes)
+        .sort((attr1, attr2) => attr1.name.localeCompare(attr2.name))
+        .forEach(attr => {
+          if (attr.value === '')
+            return this.emit(` ${attr.name}`);
+          const value = attr.value.replace(/&/g, '&amp;')
+                            .replace(/\u0022/g, '&quot;')
+                            .replace(/\u0027/g, '&apos;');
+          this.emit(` ${attr.name}="${value}"`);
+        });
+    if (isHTMLInputElement(element) && element.value)
+      this.handleInputNode(element);
+    this.emit('>');
+
+    if (isHTMLTextAreaElement(element) && element.value)
+      this.handleTextAreaNode(element);
+
+    if (element.childNodes.length === 0 &&
+        HTML5_VOID_ELEMENTS.has(tagName)) {
+      return;
+    }
+    this.serializeChildren(element);
+    this.emit(`</${tagName}>`);
+  }
 
-/**
- * @param {!Array<!Marker>} expectedMarkers
- */
-function checkExpectedMarkers(expectedMarkers) {
-  if (expectedMarkers.length === 0)
-    return;
-  expectedMarkers.forEach(marker => marker.assertValid());
-  expectedMarkers.sort(markerComparison);
-  expectedMarkers.reduce((lastMarker, currentMarker) => {
-    assert_less_than(
-        lastMarker.location + lastMarker.length, currentMarker.location,
-        'Marker ranges should be disjoint.');
-    return currentMarker;
-  });
-}
+  /**
+   * @public
+   * @param {!HTMLDocument} document
+   */
+  serialize(document) {
+    if (document.body)
+        this.serializeChildren(document.body);
+    else
+        this.serializeInternal(document.documentElement);
+    return this.strings_.join('');
+  }
 
-/**
- * @param {!Node} node
- * @param {string} type
- * @param {!Array<!Marker>} markers
- */
-function extractMarkersOfType(node, type, markers) {
-  /** @type {!HTMLBodyElement} */
-  const body = node.ownerDocument.body;
-  /** @type {number} */
-  const markerCount = window.internals.markerCountForNode(node, type);
-  for (let i = 0; i < markerCount; ++i) {
-    /** @type {!Range} */
-    const markerRange = window.internals.markerRangeForNode(node, type, i);
-    /** @type {string} */
-    const description = window.internals.markerDescriptionForNode(node, type, i);
-    /** @type {number} */
-    const location = window.internals.locationFromRange(body, markerRange);
-    /** @type {number} */
-    const length = window.internals.lengthFromRange(body, markerRange);
+  /**
+   * @private
+   * @param {!HTMLElement} element
+   */
+  serializeChildren(element) {
+    /** @type {!Array<!Node>} */
+    const childNodes = Array.from(element.childNodes);

[yosin_UTC9, 2016/10/25 09:41:11]

Since Node#childNodes works with for-of, we don't need to use Array.from()

[Xiaocheng, 2016/10/25 13:19:17]

Switched to a for loop.

+    if (childNodes.length === 0) {

[yosin_UTC9, 2016/10/25 09:41:11]

if (element.childNodes.length === 0)

[Xiaocheng, 2016/10/25 13:19:16]

Switched to a for loop and removed this check.

Perhaps you should do the same for assert_selection...

+      this.advancedTo(element, 0);
+      return;
+    }
 
-    markers.push(new Marker(location, length, type, description));
+    /** @type {number} */
+    let childIndex = 0;
+    for (const child of childNodes) {
+      this.advancedTo(element, childIndex);
+      this.serializeInternal(child, childIndex);
+      ++childIndex;
+    }
+    this.advancedTo(element, childIndex);
   }
-}
 
-/**
- * @param {!Node} node
- * @param {!Array<!Marker>} markers
- */
-function extractAllMarkersRecursivelyTo(node, markers) {
-  extractMarkersOfType(node, kSpelling, markers);
-  extractMarkersOfType(node, kGrammar, markers);
-  node.childNodes.forEach(
-      child => extractAllMarkersRecursivelyTo(child, markers));
+  /**
+   * @private
+   * @param {!Node} node
+   */
+  serializeInternal(node) {
+    if (isElement(node))
+      return this.handleElementNode(node);
+    if (isCharacterData(node))
+      return this.handleCharacterData(node);
+    throw new Error(`Unexpected node ${node}`);
+  }
 }
 
-/**
- * @param {!Document} doc
- * @return {!Array<!Marker>}
- */
-function extractAllMarkers(doc) {
-  /** @type {!Array<!Marker>} */
-  const markers = [];
-  extractAllMarkersRecursivelyTo(doc.body, markers);
-  markers.sort(markerComparison);
-  return markers;
-}
+/** @type {string} */
+const kSpelling = 'spelling';
+/** @type {string} */
+const kGrammar = 'grammar';
+/** @type {string} */
+const kSpellingMarker = '_';
+/** @type {string} */
+const kGrammarMarker = '~';
+/** @type {string} */
+const kBothMarkers = '#';
 
 /**
  * @param {!Test} testObject
- * @param {!Sample} sample,
- * @param {!Array<!Marker>} expectedMarkers
+ * @param {!Sample} sample
+ * @param {string} expectedText
  * @param {number} remainingRetry
  * @param {number} retryInterval
  */
 function verifyMarkers(
-    testObject, sample, expectedMarkers, remainingRetry, retryInterval) {
+    testObject, sample, expectedText, remainingRetry, retryInterval) {
   assert_not_equals(
       window.internals, undefined,
       'window.internals is required for running automated spellcheck tests.');
 
-  /** @type {!Array<!Marker>} */
-  const actualMarkers = extractAllMarkers(sample.document);
+  /** @type {!MarkerSerializer} */
+  const serializer = new MarkerSerializer(
+      [kSpelling, kGrammar],
+      [kSpellingMarker, kGrammarMarker, kBothMarkers].join(''));
+
   try {
-    assert_equals(actualMarkers.length, expectedMarkers.length,
-                  'Number of markers mismatch.');
-    actualMarkers.forEach(
-        (marker, index) => marker.assertMatch(expectedMarkers[index]));
+    assert_equals(serializer.serialize(sample.document), expectedText);
     testObject.done();
     sample.remove();
   } catch (error) {
@@ -249,7 +352,7 @@ function verifyMarkers(
     // know the completion of spellchecking instead of passively waiting for
     // markers to appear or disappear.
     testObject.step_timeout(
-        () => verifyMarkers(testObject, sample, expectedMarkers,
+        () => verifyMarkers(testObject, sample, expectedText,
                             remainingRetry - 1, retryInterval),
         retryInterval);
   }
@@ -271,47 +374,39 @@ const testQueue = [];
 /**
  * @param {string} inputText
  * @param {function(!Document)|string} tester
- * @param {!Marker|!Array<!Marker>} expectedMarkers
+ * @param {string} expectedText
  * @param {string=} opt_title
  */
-function invokeSpellcheckTest(inputText, tester, expectedMarkers, opt_title) {
+function invokeSpellcheckTest(inputText, tester, expectedText, opt_title) {
   spellcheckTestRunning = true;
 
-  /** @type {!Test} */
-  const testObject = async_test(opt_title, {isSpellcheckTest: true});
-
-  if (!(expectedMarkers instanceof Array))
-    expectedMarkers = [expectedMarkers]
-  testObject.step(() => checkExpectedMarkers(expectedMarkers));
-
-  if (window.testRunner)
-    window.testRunner.setMockSpellCheckerEnabled(true);
+  async_test(testObject => {
+    // TODO(xiaochengh): Merge the following part with |assert_selection|.
+    /** @type {!Sample} */
+    const sample = new Sample(inputText);
+    if (typeof(tester) === 'function') {
+      tester.call(window, sample.document);
+    } else if (typeof(tester) === 'string') {
+      const strings = tester.split(/ (.+)/);
+      sample.document.execCommand(strings[0], false, strings[1]);
+    } else {
+      assert_unreached(`Invalid tester: ${tester}`);
+    }
 
-  // TODO(xiaochengh): Merge the following part with |assert_selection|.
-  /** @type {!Sample} */
-  const sample = new Sample(inputText);
-  if (typeof(tester) === 'function') {
-    tester.call(window, sample.document);
-  } else if (typeof(tester) === 'string') {
-    const strings = tester.split(/ (.+)/);
-    sample.document.execCommand(strings[0], false, strings[1]);
-  } else {
-    testObject.step(() => assert_unreached(`Invalid tester: ${tester}`));
-  }
+    /** @type {number} */
+    const kMaxRetry = 10;
+    /** @type {number} */
+    const kRetryInterval = 50;
 
-  /** @type {number} */
-  const kMaxRetry = 10;
-  /** @type {number} */
-  const kRetryInterval = 50;
-
-  // TODO(xiaochengh): We should make SpellCheckRequester::didCheck trigger
-  // something in JavaScript (e.g., a |Promise|), so that we can actively know
-  // the completion of spellchecking instead of passively waiting for markers to
-  // appear or disappear.
-  testObject.step_timeout(
-      () => verifyMarkers(testObject, sample, expectedMarkers,
-                          kMaxRetry, kRetryInterval),
-      kRetryInterval);
+    // TODO(xiaochengh): We should make SpellCheckRequester::didCheck trigger
+    // something in JavaScript (e.g., a |Promise|), so that we can actively know
+    // the completion of spellchecking instead of passively waiting for markers to
+    // appear or disappear.
+    testObject.step_timeout(
+        () => verifyMarkers(testObject, sample, expectedText,
+                            kMaxRetry, kRetryInterval),
+        kRetryInterval);
+  }, opt_title, {isSpellcheckTest: true});
 }
 
 add_result_callback(testObj => {
@@ -323,29 +418,31 @@ add_result_callback(testObj => {
     if (args === undefined)
       return;
     invokeSpellcheckTest(args.inputText, args.tester,
-                         args.expectedMarkers, args.opt_title);
+                         args.expectedText, args.opt_title);
 });
 
+// TODO(xiaochengh): Add support for checking marker descriptions.
+
 /**
  * @param {string} inputText
  * @param {function(!Document)|string} tester
- * @param {!Marker|!Array<!Marker>} expectedMarkers
+ * @param {string} expectedText
  * @param {string=} opt_title
  */
-function spellcheckTest(inputText, tester, expectedMarkers, opt_title) {
+function spellcheckTest(inputText, tester, expectedText, opt_title) {
+  if (window.testRunner)

[yosin_UTC9, 2016/10/25 09:41:11]

We should abort if window.internals is unavailable by assert_undefined().

[Xiaocheng, 2016/10/25 13:19:17]

There's a reason not to abort here.

|internals| is needed only for inspecting the markers. Unless we need to
manipulate clipboard, |tester| can be executed without |internals|. By not
aborting here, we are allowed to load the test file in a browser and see the
result.

+    window.testRunner.setMockSpellCheckerEnabled(true);
+
   if (spellcheckTestRunning) {
     testQueue.push({
         inputText: inputText, tester: tester,
-        expectedMarkers: expectedMarkers, opt_title: opt_title});
+        expectedText: expectedText, opt_title: opt_title});
     return;
   }
 
-  invokeSpellcheckTest(inputText, tester, expectedMarkers, opt_title);
+  invokeSpellcheckTest(inputText, tester, expectedText, opt_title);
 }
 
 // Export symbols
-window.Marker = Marker;
-window.spellingMarker = spellingMarker;
-window.grammarMarker = grammarMarker;
 window.spellcheck_test = spellcheckTest;
 })();