MD Bookmarks: Prevent navigating to invalid folders

chrome/browser/resources/md_bookmarks/actions.js

 // Copyright 2017 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.
 
 /**
  * @fileoverview Module for functions which produce action objects. These are
  * listed in one place to document available actions and their parameters.
  */
 
 cr.define('bookmarks.actions', function() {
@@ skipping 79 matching lines @@
    */
   function refreshNodes(nodeMap) {
     return {
       name: 'refresh-nodes',
       nodes: nodeMap,
     };
   };
 
   /**
    * @param {string} id
+   * @param {NodeList} nodes Current node state. Can be ommitted in tests.

[calamity, 2017/04/11 04:13:37]

Doesn't omitting this always result in a no-op? When would that be useful?

[tsergeant, 2017/05/02 03:20:24]

If nodes is falsey, the check for validity will be skipped entirely, and a
select-folder action will always be returned. This is useful in tests that don't
care about the validity check.

    * @return {!Action}
    */
-  function selectFolder(id) {
-    assert(id != '0', 'Cannot select root folder');
+  function selectFolder(id, nodes) {
+    if (nodes && (id == ROOT_NODE_ID || !nodes[id] || nodes[id].url)) {
+      return {
+        name: 'no-op',
+      };

[calamity, 2017/04/11 04:13:37]

Make no-op a private action and early return in reduceAction.

[tsergeant, 2017/05/02 03:20:24]

I've replace 'no-op' with returning null, which does exactly that.

+    }
     return {
       name: 'select-folder',
       id: id,
     };
   }
 
   /**
    * @param {string} id
    * @param {boolean} open
    * @return {!Action}
@@ skipping 92 matching lines @@
     moveBookmark: moveBookmark,
     refreshNodes: refreshNodes,
     removeBookmark: removeBookmark,
     reorderChildren: reorderChildren,
     selectFolder: selectFolder,
     selectItem: selectItem,
     setSearchResults: setSearchResults,
     setSearchTerm: setSearchTerm,
   };
 });