Move gr-diff-comment-thread factory to diff-host

This is now only used from there, so it no longer needs to be globally
accessible on Gerrit.

Change-Id: I6facd64ece360f5b8ddf7b801621e86bd5f39697

polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.html

@@ -19,6 +19,7 @@ limitations under the License.
 <link rel="import" href="../../shared/gr-js-api-interface/gr-js-api-interface.html">
 <link rel="import" href="../gr-diff-processor/gr-diff-processor.html">
 <link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
+<link rel="import" href="../gr-diff-comment-thread-group/gr-diff-comment-thread-group.html">
 <link rel="import" href="../gr-ranged-comment-layer/gr-ranged-comment-layer.html">
 <link rel="import" href="../gr-syntax-layer/gr-syntax-layer.html">
 

polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group.js

@@ -17,58 +17,6 @@
 (function() {
   'use strict';
 
-  window.Gerrit = window.Gerrit || {};
-
-  // This method will eventually move to gr-diff-host (so that gr-diff and it's
-  // descendants, including gr-diff-comment-thread-group, do not depend on a
-  // specific comment thread implementation, but can instead be used with other
-  // comment widgets). I cannot move it there yet, because it is still called
-  // from this file, and this file cannot depend on gr-diff-host. I decided to
-  // make it a function on the global Gerrit namespace, so that
-  //   1) I can move the call-side in the next change without moving this code,
-  //      and thereby reduce the number of moving parts per commit.
-  //   2) To already now cut the ties to the this object - if it was an element
-  //      method, I would probably want to use isOnParent etc. from `this`, and
-  //      thus be required to change the code when I move it to gr-diff host.
-  //      Making it a free function first requires me to catch any references to
-  //      `this` and instead pass  those in as parameter, which then allows me
-  //      to move it later without any other changes, which makes the diff
-  //      easier to read.
-  /**
-   * @param {Object} thread
-   * @param {number} parentIndex
-   * @param {number} changeNum
-   * @param {string} path
-   * @param {string} projectName
-   */
-  window.Gerrit.createThreadElement = function(
-      thread, parentIndex, changeNum, path, projectName) {
-    const threadEl = document.createElement('gr-diff-comment-thread');
-    threadEl.comments = thread.comments;
-    threadEl.commentSide = thread.commentSide;
-    threadEl.isOnParent = !!thread.isOnParent;
-    threadEl.parentIndex = parentIndex;
-    threadEl.changeNum = changeNum;
-    threadEl.patchNum = thread.patchNum;
-    threadEl.lineNum = thread.lineNum;
-    const rootIdChangedListener = changeEvent => {
-      thread.rootId = changeEvent.detail.value;
-    };
-    threadEl.addEventListener('root-id-changed', rootIdChangedListener);
-    threadEl.path = path;
-    threadEl.projectName = projectName;
-    threadEl.range = thread.range;
-    const threadDiscardListener = e => {
-      const threadEl = /** @type {!Node} */ (e.currentTarget);
-      const parent = Polymer.dom(threadEl).parentNode;
-      threadEl.removeEventListener('root-id-changed', rootIdChangedListener);
-      threadEl.removeEventListener('thread-discard', threadDiscardListener);
-      Polymer.dom(parent).removeChild(threadEl);
-    };
-    threadEl.addEventListener('thread-discard', threadDiscardListener);
-    return threadEl;
-  };
-
   Polymer({
     is: 'gr-diff-comment-thread-group',
 

polygerrit-ui/app/elements/diff/gr-diff-comment-thread-group/gr-diff-comment-thread-group_test.html

@@ -36,38 +36,14 @@ limitations under the License.
 
 <script>
   suite('gr-diff-comment-thread-group tests', () => {
-    let element;
     let sandbox;
 
     setup(() => {
       sandbox = sinon.sandbox.create();
-      element = fixture('basic');
     });
 
     teardown(() => {
       sandbox.restore();
     });
-
-    test('thread-discard handling', () => {
-      const threads = [
-        {comments: [{id: 4711}]},
-        {comments: [{id: 42}]},
-      ];
-      const threadEls = threads.map(thread => Gerrit.createThreadElement(
-          thread, 1, 2, 'some/path', 'Some Project'));
-      assert.equal(threadEls.length, 2);
-      assert.equal(threadEls[0].rootId, 4711);
-      assert.equal(threadEls[1].rootId, 42);
-      for (const threadEl of threadEls) {
-        Polymer.dom(element).appendChild(threadEl);
-      }
-
-      threadEls[0].dispatchEvent(
-          new CustomEvent('thread-discard', {detail: {rootId: 4711}}));
-      const attachedThreads = element.queryAllEffectiveChildren(
-          'gr-diff-comment-thread');
-      assert.equal(attachedThreads.length, 1);
-      assert.equal(attachedThreads[0].rootId, 42);
-    });
   });
 </script>

polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.html

@@ -20,7 +20,6 @@ limitations under the License.
 <link rel="import" href="../../core/gr-reporting/gr-reporting.html">
 <link rel="import" href="../../shared/gr-rest-api-interface/gr-rest-api-interface.html">
 <link rel="import" href="../gr-diff-comment-thread/gr-diff-comment-thread.html">
-<link rel="import" href="../gr-diff-comment-thread-group/gr-diff-comment-thread-group.html">
 <link rel="import" href="../gr-diff/gr-diff.html">
 
 <dom-module id="gr-diff-host">

polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host.js

@@ -599,15 +599,36 @@
     },
 
     _createThreadElement(thread) {
-      const threadEl = Gerrit.createThreadElement(
+      const threadEl = document.createElement('gr-diff-comment-thread');
-          thread, this._parentIndex, this.changeNum, this.path,
-          this.projectName);
       threadEl.className = 'comment-thread';
-      threadEl.addEventListener('thread-discard', e => {
+      threadEl.comments = thread.comments;
+      threadEl.commentSide = thread.commentSide;
+      threadEl.isOnParent = !!thread.isOnParent;
+      threadEl.parentIndex = this._parentIndex;
+      threadEl.changeNum = this.changeNum;
+      threadEl.patchNum = thread.patchNum;
+      threadEl.lineNum = thread.lineNum;
+      const rootIdChangedListener = changeEvent => {
+        thread.rootId = changeEvent.detail.value;
+      };
+      threadEl.addEventListener('root-id-changed', rootIdChangedListener);
+      threadEl.path = this.path;
+      threadEl.projectName = this.projectName;
+      threadEl.range = thread.range;
+      const threadDiscardListener = e => {
+        const threadEl = /** @type {!Node} */ (e.currentTarget);
+
+        const parent = Polymer.dom(threadEl).parentNode;
+        Polymer.dom(parent).removeChild(threadEl);
+
         const i = this._threadEls.findIndex(
             threadEl => threadEl.rootId == e.detail.rootId);
         this._threadEls.splice(i, 1);
-      });
+
+        threadEl.removeEventListener('root-id-changed', rootIdChangedListener);
+        threadEl.removeEventListener('thread-discard', threadDiscardListener);
+      };
+      threadEl.addEventListener('thread-discard', threadDiscardListener);
       return threadEl;
     },
 

polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.html

@@ -55,6 +55,32 @@ limitations under the License.
       sandbox.restore();
     });
 
+    test('thread-discard handling', () => {
+      const threads = [
+        {comments: [{id: 4711}]},
+        {comments: [{id: 42}]},
+      ];
+      element._parentIndex = 1;
+      element.changeNum = '2';
+      element.path = 'some/path';
+      element.projectName = 'Some project';
+      const threadEls = threads.map(
+          thread => element._createThreadElement(thread));
+      assert.equal(threadEls.length, 2);
+      assert.equal(threadEls[0].rootId, 4711);
+      assert.equal(threadEls[1].rootId, 42);
+      for (const threadEl of threadEls) {
+        Polymer.dom(element).appendChild(threadEl);
+      }
+
+      threadEls[0].dispatchEvent(
+          new CustomEvent('thread-discard', {detail: {rootId: 4711}}));
+      const attachedThreads = element.queryAllEffectiveChildren(
+          'gr-diff-comment-thread');
+      assert.equal(attachedThreads.length, 1);
+      assert.equal(attachedThreads[0].rootId, 42);
+    });
+
     test('reload() cancels before network resolves', () => {
       const cancelStub = sandbox.stub(element.$.diff, 'cancel');
 

polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html

@@ -20,6 +20,7 @@ limitations under the License.
 <link rel="import" href="../../../styles/shared-styles.html">
 <link rel="import" href="../../shared/gr-button/gr-button.html">
 <link rel="import" href="../gr-diff-builder/gr-diff-builder.html">
+<link rel="import" href="../gr-diff-comment-thread-group/gr-diff-comment-thread-group.html">
 <link rel="import" href="../gr-diff-highlight/gr-diff-highlight.html">
 <link rel="import" href="../gr-diff-selection/gr-diff-selection.html">
 <link rel="import" href="../gr-syntax-themes/gr-syntax-theme.html">