Typecheck chrome://bookmarks using Closure Compiler

chrome/browser/resources/bookmark_manager/js/bmm/bookmark_list.js

 // Copyright (c) 2012 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.
 
 // TODO(arv): Now that this is driven by a data model, implement a data model
 //            that handles the loading and the events from the bookmark backend.
 
+/**
+ * @typedef {{childIds: Array.<string>}}
+ *
+ * @see chrome/common/extensions/api/bookmarks.json
+ */
+var ReorderInfo;
+
+/**
+ * @typedef {{parentId: string, index: number, oldParentId: string,
+ *            oldIndex: number}}
+ *
+ * @see chrome/common/extensions/api/bookmarks.json
+ */
+var MoveInfo;
+
 cr.define('bmm', function() {
   var List = cr.ui.List;
   var ListItem = cr.ui.ListItem;
   var ArrayDataModel = cr.ui.ArrayDataModel;
   var ContextMenuButton = cr.ui.ContextMenuButton;
 
+  /**
+   * This variable is fake and used only to declare exported bmm.list name.
+   * @suppress {suspiciousCode}

[Dan Beam, 2014/09/23 02:46:55]

why can't we simply change the module return statement to:

  return {
    ...
    list: $('list'),
  };

instead?

[Vitaly Pavlenko, 2014/09/23 22:20:54]

Done, fixed handling in compiler pass. But I don't think it's a good idea: at
the point when return object is created DOM is not processed, so $('list')
returns null.

As long as it's not used before it's assigned externally:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/res...
it's ok, but it could mislead future code readers.

[Dan Beam, 2014/09/24 21:38:58]

see below

[Vitaly Pavlenko, 2014/09/24 22:18:23]

Done.

+   */
   var list;
 
   /**
    * Basic array data model for use with bookmarks.
    * @param {!Array.<!BookmarkTreeNode>} items The bookmark items.
    * @constructor
    * @extends {ArrayDataModel}
    */
   function BookmarksArrayDataModel(items) {
     ArrayDataModel.call(this, items);
@@ skipping 26 matching lines @@
     while ((n = parent.lastChild)) {
       parent.removeChild(n);
     }
     parent.appendChild(newChild);
   }
 
   /**
    * Creates a new bookmark list.
    * @param {Object=} opt_propertyBag Optional properties.
    * @constructor
-   * @extends {HTMLButtonElement}
+   * @extends {cr.ui.List}
    */
   var BookmarkList = cr.ui.define('list');
 
   BookmarkList.prototype = {
     __proto__: List.prototype,
 
     /** @override */
     decorate: function() {
       List.prototype.decorate.call(this);
       this.addEventListener('mousedown', this.handleMouseDown_);
 
       // HACK(arv): http://crbug.com/40902
       window.addEventListener('resize', this.redraw.bind(this));
 
       // We could add the ContextMenuButton in the BookmarkListItem but it slows
       // down redraws a lot so we do this on mouseovers instead.
       this.addEventListener('mouseover', this.handleMouseOver_.bind(this));
 
       bmm.list = this;
     },
 
     createItem: function(bookmarkNode) {
-      return new BookmarkListItem(bookmarkNode);
+      return new BookmarkListItem(/** @type {BookmarkTreeNode} */(
+          bookmarkNode));
     },
 
     /** @private {string} */
     parentId_: '',
 
     /** @private {number} */
     loadCount_: 0,
 
     /**
      * Reloads the list from the bookmarks backend.
@@ skipping 97 matching lines @@
       var event = new Event('urlClicked', {bubbles: true});
       event.url = url;
       event.originalEvent = originalEvent;
       this.dispatchEvent(event);
     },
 
     /**
      * Handles mousedown events so that we can prevent the auto scroll as
      * necessary.
      * @private
-     * @param {!MouseEvent} e The mousedown event object.
+     * @param {!Event} e The mousedown event object.
      */
     handleMouseDown_: function(e) {
+      e = /** @type {!MouseEvent} */(e);
       if (e.button == 1) {
         // WebKit no longer fires click events for middle clicks so we manually
         // listen to mouse up to dispatch a click event.
         this.addEventListener('mouseup', this.handleMiddleMouseUp_);
 
         // When the user does a middle click we need to prevent the auto scroll
         // in case the user is trying to middle click to open a bookmark in a
         // background tab.
         // We do not do this in case the target is an input since middle click
         // is also paste on Linux and we don't want to break that.
         if (e.target.tagName != 'INPUT')
           e.preventDefault();
       }
     },
 
     /**
      * WebKit no longer dispatches click events for middle clicks so we need
      * to emulate it.
      * @private
-     * @param {!MouseEvent} e The mouse up event object.
+     * @param {!Event} e The mouse up event object.
      */
     handleMiddleMouseUp_: function(e) {
+      e = /** @type {!MouseEvent} */(e);
       this.removeEventListener('mouseup', this.handleMiddleMouseUp_);
       if (e.button == 1) {
         var el = e.target;
         while (el.parentNode != this) {
           el = el.parentNode;
         }
         var node = el.bookmarkNode;
         if (node && !bmm.isFolder(node))
           this.dispatchUrlClickedEvent_(node.url, e);
       }
       e.preventDefault();
     },
 
     // Bookmark model update callbacks
     handleBookmarkChanged: function(id, changeInfo) {
       var dataModel = this.dataModel;
       var index = dataModel.findIndexById(id);
       if (index != -1) {
         var bookmarkNode = this.dataModel.item(index);
         bookmarkNode.title = changeInfo.title;
         if ('url' in changeInfo)
           bookmarkNode.url = changeInfo['url'];
 
         dataModel.updateIndex(index);
       }
     },
 
+    /**
+     * @param {string} id
+     * @param {ReorderInfo} reorderInfo
+     */
     handleChildrenReordered: function(id, reorderInfo) {
       if (this.parentId == id) {
         // We create a new data model with updated items in the right order.
         var dataModel = this.dataModel;
         var items = {};
         for (var i = this.dataModel.length - 1; i >= 0; i--) {
           var bookmarkNode = dataModel.item(i);
           items[bookmarkNode.id] = bookmarkNode;
         }
         var newArray = [];
         for (var i = 0; i < reorderInfo.childIds.length; i++) {
           newArray[i] = items[reorderInfo.childIds[i]];
           newArray[i].index = i;
         }
 
         this.dataModel = new BookmarksArrayDataModel(newArray);
       }
     },
 
     handleCreated: function(id, bookmarkNode) {
       if (this.parentId == bookmarkNode.parentId)
         this.dataModel.splice(bookmarkNode.index, 0, bookmarkNode);
     },
 
+    /**
+     * @param {string} id
+     * @param {MoveInfo} moveInfo
+     */
     handleMoved: function(id, moveInfo) {
       if (moveInfo.parentId == this.parentId ||
           moveInfo.oldParentId == this.parentId) {
 
         var dataModel = this.dataModel;
 
         if (moveInfo.oldParentId == moveInfo.parentId) {
           // Reorder within this folder
 
           this.startBatchUpdates();
@@ skipping 50 matching lines @@
         // the size should not change since we already fixed the width.
         window.setTimeout(function() {
           list.style.width = '';
         }, 0);
       }
     }
   };
 
   /**
    * The ID of the bookmark folder we are displaying.
-   * @type {string}
    */
   cr.defineProperty(BookmarkList, 'parentId', cr.PropertyKind.JS,
                     function() {
                       this.reload();
                     });
 
   /**
    * The contextMenu property.
-   * @type {cr.ui.Menu}
    */
   cr.ui.contextMenuHandler.addContextMenuProperty(BookmarkList);
+  /** @type {cr.ui.Menu} */
+  BookmarkList.prototype.contextMenu;
 
   /**
    * Creates a new bookmark list item.
    * @param {!BookmarkTreeNode} bookmarkNode The bookmark node this represents.
    * @constructor
    * @extends {cr.ui.ListItem}
    */
   function BookmarkListItem(bookmarkNode) {
     var el = cr.doc.createElement('div');
     el.bookmarkNode = bookmarkNode;
@@ skipping 131 matching lines @@
             listItem.editing = false;
           }
         }, 50);
       }
 
       var doc = this.ownerDocument;
       if (editing) {
         this.setAttribute('editing', '');
         this.draggable = false;
 
-        labelInput = doc.createElement('input');
+        labelInput = /** @type {HTMLElement} */(doc.createElement('input'));
         labelInput.placeholder =
             loadTimeData.getString('name_input_placeholder');
         replaceAllChildren(labelEl, labelInput);
         labelInput.value = title;
 
         if (!isFolder) {
-          urlInput = doc.createElement('input');
+          urlInput = /** @type {HTMLElement} */(doc.createElement('input'));
           urlInput.type = 'url';
           urlInput.required = true;
           urlInput.placeholder =
               loadTimeData.getString('url_input_placeholder');
 
           // We also need a name for the input for the CSS to work.
           urlInput.name = '-url-input-' + cr.createUid();
-          replaceAllChildren(urlEl, urlInput);
+          replaceAllChildren(assert(urlEl), urlInput);
           urlInput.value = url;
         }
 
-        function stopPropagation(e) {
+        var stopPropagation = function(e) {
           e.stopPropagation();
-        }
+        };

[Dan Beam, 2014/09/23 02:46:55]

why did you make this change?

[Vitaly Pavlenko, 2014/09/23 22:20:54]

##
/usr/local/google/home/vitalyp/src/chrome/browser/resources/bookmark_manager/js/bmm/bookmark_list.js:562:
ERROR - functions can only be declared at top level or immediately within
another function in ES5 strict mode
##         function stopPropagation(e) {
##         ^
## 

 
         var eventsToStop =
             ['mousedown', 'mouseup', 'contextmenu', 'dblclick', 'paste'];
         eventsToStop.forEach(function(type) {
           labelInput.addEventListener(type, stopPropagation);
         });
         labelInput.addEventListener('keydown', handleKeydown);
         labelInput.addEventListener('blur', handleBlur);
         cr.ui.limitInputWidth(labelInput, this, 100, 0.5);
         labelInput.focus();
@@ skipping 55 matching lines @@
         }
       }
     }
   };
 
   return {
     BookmarkList: BookmarkList,
     list: list
   };
 });