Making bookmark folder context menu display the number of bookmarks that will be opened by Open All

chrome/browser/ui/bookmarks/bookmark_utils_desktop.cc

Index: chrome/browser/ui/bookmarks/bookmark_utils_desktop.cc
diff --git a/chrome/browser/ui/bookmarks/bookmark_utils_desktop.cc b/chrome/browser/ui/bookmarks/bookmark_utils_desktop.cc
index e80f31904d26a0f5088d37c087fe9d0875d19f63..a86888519a107f86ee9da76daee8ea32fdcbbf45 100644
--- a/chrome/browser/ui/bookmarks/bookmark_utils_desktop.cc
+++ b/chrome/browser/ui/bookmarks/bookmark_utils_desktop.cc
@@ -227,6 +227,38 @@ void OpenAll(gfx::NativeWindow parent,
   OpenAll(parent, navigator, nodes, initial_disposition, browser_context);
 }
 
+int OpenCount(gfx::NativeWindow parent,
+              const std::vector<const bookmarks::BookmarkNode*>& nodes,
+              bool offTheRecord,
+              content::BrowserContext* browser_context) {
+  int count = 0;
+  if (!ShouldOpenAll(parent, nodes))
+    return count;
+  // Counts all |nodes| of type URL and any children of |nodes| that are of type
+  // URL.
+  OpenURLIterator iterator(nodes);
+  while (iterator.has_next()) {

[Peter Kasting, 2017/04/13 04:52:09]

This feels very copy-and-pasted from OpenAll().  If you implement my suggestion
above, then write a lambda or functor that checks if a URL is safe to open in
incognito mode, it becomes easy to use methods like base::EraseIf() or
std::count_if() to transform the vector, check how many safe-to-open URLs there
are, etc.

[Paezagon, 2017/04/14 01:00:29]

I have added a second get vector function that returns URLs to use in incognito
mode. Seemed to make sense to do as that code is used in a couple places.

+    const GURL* url = iterator.NextURL();
+    // When |initial_disposition| is OFF_THE_RECORD, a node which can't be
+    // opened in incognito window, it is detected using |browser_context|, is
+    // not opened.

[Peter Kasting, 2017/04/13 04:52:09]

This comment doesn't make much sense.  I realize you're copying it from above,
but please rewrite, or eliminate if the comment adds nothing to the code.

[Paezagon, 2017/04/14 01:00:29]

Done.

+    if (offTheRecord && !IsURLAllowedInIncognito(*url, browser_context))
+      continue;
+
+    count++;
+  }
+  return count;
+}
+
+int OpenCount(gfx::NativeWindow parent,
+              const BookmarkNode* node,
+              bool offTheRecord,
+              content::BrowserContext* browser_context) {
+  std::vector<const BookmarkNode*> nodes;
+  nodes.push_back(node);
+  return OpenCount(parent, nodes, offTheRecord, browser_context);

[Peter Kasting, 2017/04/13 04:52:09]

Nit: Simpler:

  return OpenCount(parent, {node}, offTheRecord, browser_context);

[Paezagon, 2017/04/14 01:00:29]

When I make the change, the compiler complains that it is recursive. 

c:\src\chromium\src\chrome\browser\ui\bookmarks\bookmark_utils_desktop.cc(258) :
warning C4717: 'chrome::OpenCount': recursive on all control paths, function
will cause runtime stack overflow

[Peter Kasting, 2017/04/14 08:17:14]

Might need to do something like:

  return OpenCount(parent, std::vector<const bookmarks::BookmarkNode*>{node},
...);

?  I'm not sure if that even compiles.

If not this should:

  std::vector<const BookmarkNode*> nodes = {node};
  return OpenCount(parent, nodes, offTheRecord, browser_context);

[Paezagon, 2017/04/14 18:47:24]

Done.

+}
+
 bool ConfirmDeleteBookmarkNode(const BookmarkNode* node,
                                gfx::NativeWindow window) {
   DCHECK(node && node->is_folder() && !node->empty());