Once the initial URL is opened in a new window, insure that subsequent URLs are opened in the new window instead of the original window. This change hinges on having URL opening code return the browser into which the URL was opened so that subsequent openings may now into which browser to do so.

BUG=39901
TEST=Bring up context menu on a folder with two or more bookmarks and select "Open All in Incognito Window". Insure that all of the URLs are opened in the newly created, top-most window. Verify that no new tabs were created in the original window. Perform this same test using "Open All in New Window".

Review URL: http://codereview.chromium.org/1634004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@44201 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 46 insertions, 42 deletions

diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index f0e332a..9121bac 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -9,9 +9,11 @@
 #include "base/sys_string_conversions.h"
 #include "chrome/browser/bookmarks/bookmark_editor.h"
 #include "chrome/browser/bookmarks/bookmark_model.h"
+#include "chrome/browser/bookmarks/bookmark_utils.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_list.h"
 #import "chrome/browser/browser_theme_provider.h"
+#import "chrome/browser/browser_window.h"
 #import "chrome/browser/cocoa/background_gradient_view.h"
 #import "chrome/browser/cocoa/bookmark_bar_bridge.h"
 #import "chrome/browser/cocoa/bookmark_bar_constants.h"
@@ -180,10 +182,6 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
 
 - (void)watchForExitEvent:(BOOL)watch;
 
-// An ObjC version of bookmark_utils::OpenAllImpl().
-- (void)openBookmarkNodesRecursive:(const BookmarkNode*)node
-                       disposition:(WindowOpenDisposition)disposition;
-
 @end
 
 @implementation BookmarkBarController
@@ -416,7 +414,7 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
 // bookmark folders, not a button context menu.
 - (void)closeAllBookmarkFolders {
   [self watchForExitEvent:NO];
-  [[folderController_ window] close];
+  [folderController_ close];
   folderController_ = nil;
 }
 
@@ -1283,22 +1281,6 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   }
 }
 
-- (void)openBookmarkNodesRecursive:(const BookmarkNode*)node
-                       disposition:(WindowOpenDisposition)disposition {
-  DCHECK(node);
-  for (int i = 0; i < node->GetChildCount(); i++) {
-    const BookmarkNode* child = node->GetChild(i);
-    if (child->is_url()) {
-      [self openURL:child->GetURL() disposition:disposition];
-      // We revert to a basic disposition in case the initial
-      // disposition opened a new window.
-      disposition = NEW_BACKGROUND_TAB;
-    } else {
-      [self openBookmarkNodesRecursive:child disposition:disposition];
-    }
-  }
-}
-
 // Return the BookmarkNode associated with the given NSMenuItem.  Can
 // return NULL which means "do nothing".  One case where it would
 // return NULL is if the bookmark model gets modified while you have a
@@ -1313,10 +1295,20 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   return node;
 }
 
+- (void)openAll:(const BookmarkNode*)node
+    disposition:(WindowOpenDisposition)disposition {
+  [self closeAllBookmarkFolders];
+  bookmark_utils::OpenAll(browser_->window()->GetNativeHandle(),
+                          browser_->profile(),
+                          browser_,
+                          node,
+                          disposition);
+}
+
 - (IBAction)openAllBookmarks:(id)sender {
   const BookmarkNode* node = [self nodeFromMenuItem:sender];
   if (node) {
-    [self openBookmarkNodesRecursive:node disposition:NEW_FOREGROUND_TAB];
+    [self openAll:node disposition:NEW_FOREGROUND_TAB];
     UserMetrics::RecordAction(UserMetricsAction("OpenAllBookmarks"),
                               browser_->profile());
   }
@@ -1325,7 +1317,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
 - (IBAction)openAllBookmarksNewWindow:(id)sender {
   const BookmarkNode* node = [self nodeFromMenuItem:sender];
   if (node) {
-    [self openBookmarkNodesRecursive:node disposition:NEW_WINDOW];
+    [self openAll:node disposition:NEW_WINDOW];
     UserMetrics::RecordAction(UserMetricsAction("OpenAllBookmarksNewWindow"),
                               browser_->profile());
   }
@@ -1334,7 +1326,7 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
 - (IBAction)openAllBookmarksIncognitoWindow:(id)sender {
   const BookmarkNode* node = [self nodeFromMenuItem:sender];
   if (node) {
-    [self openBookmarkNodesRecursive:node disposition:OFF_THE_RECORD];
+    [self openAll:node disposition:OFF_THE_RECORD];
     UserMetrics::RecordAction(
         UserMetricsAction("OpenAllBookmarksIncognitoWindow"),
         browser_->profile());
diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
index 44de1e9..7facb96 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
@@ -520,6 +520,8 @@ TEST_F(BookmarkBarControllerTest, OpenBookmark) {
   EXPECT_EQ(bar_.get()->dispositions_[0], CURRENT_TAB);
 }
 
+// TODO(mrossetti):Add OpenBookmarkInNewWindow test.
+
 // Confirm opening of bookmarks works from the menus (different
 // dispositions than clicking on the button).
 TEST_F(BookmarkBarControllerTest, OpenBookmarkFromMenus) {
@@ -748,6 +750,10 @@ TEST_F(BookmarkBarControllerTest, OpenAllBookmarks) {
   EXPECT_EQ(bar_.get()->dispositions_.size(), 2U);
 }
 
+// TODO(mrossetti):Add OpenAllBookmarkInNewWindow test.
+
+// TODO(mrossetti):Add OpenAllBookmarkInIncognitoWindow test.
+
 // TODO(jrg): write a test to confirm that nodeFavIconLoaded calls
 // checkForBookmarkButtonGrowth:.
 
diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller.h b/chrome/browser/cocoa/bookmark_bar_folder_controller.h
index 64ae5f6..1086249 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_folder_controller.h
@@ -86,8 +86,13 @@
 
   // Amount to scroll by on each timer fire.  Can be + or -.
   CGFloat verticalScrollDelta_;
+
+  // Set as a semaphore indicating that the window is in the process of
+  // closing. Used primarily to prevent undesired mouse enter events.
+  BOOL windowClosing_;
 }
 
+// Designated initializer.
 - (id)initWithParentButton:(BookmarkButton*)button
       parentController:(NSObject<BookmarkButtonControllerProtocol>*)controller;
 
diff --git a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
index b99ba51..bb904e2 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_folder_controller.mm
@@ -779,19 +779,16 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   [self autorelease];
 }
 
+- (void)openAll:(const BookmarkNode*)node
+    disposition:(WindowOpenDisposition)disposition {
+  [parentController_ openAll:node disposition:disposition];
+}
+
 - (IBAction)openBookmark:(id)sender {
   // Parent controller closes it all...
   [parentController_ openBookmark:sender];
 }
 
-// Flow up the chain until someone (the top level controller) knows
-// what to do.
-- (void)openBookmarkNodesRecursive:(const BookmarkNode*)node
-                       disposition:(WindowOpenDisposition)disposition {
-  [parentController_ openBookmarkNodesRecursive:node
-                                    disposition:disposition];
-}
-
 // Add a new folder controller as triggered by the given folder button.
 - (void)addNewFolderControllerWithParentButton:(BookmarkButton*)parentButton {
   if (folderController_)
@@ -808,4 +805,9 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
   return buttons_.get();
 }
 
+- (void)close {
+  [folderController_ close];
+  [super close];
+}
+
 @end  // BookmarkBarFolderController
diff --git a/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm b/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm
index 6a46198..96189f5 100644
--- a/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_folder_view_unittest.mm
@@ -103,10 +103,6 @@
 - (void)childFolderWillClose:(id<BookmarkButtonControllerProtocol>)child {
 }
 
-- (void)openBookmarkNodesRecursive:(const BookmarkNode*)node
-                       disposition:(WindowOpenDisposition)disposition {
-}
-
 - (void)addNewFolderControllerWithParentButton:(BookmarkButton*)parentButton {
 }
 
@@ -122,6 +118,10 @@
   return nil;
 }
 
+- (void)openAll:(const BookmarkNode*)node
+    disposition:(WindowOpenDisposition)disposition {
+}
+
 @end
 
 namespace {
diff --git a/chrome/browser/cocoa/bookmark_button.h b/chrome/browser/cocoa/bookmark_button.h
index a5176ac..8cc366db 100644
--- a/chrome/browser/cocoa/bookmark_button.h
+++ b/chrome/browser/cocoa/bookmark_button.h
@@ -108,10 +108,6 @@ class ThemeProvider;
 // Return a controller's folder controller for a subfolder, or nil.
 - (BookmarkBarFolderController*)folderController;
 
-// Recursively open all bookmarks from this folder using the given disposition.
-- (void)openBookmarkNodesRecursive:(const BookmarkNode*)node
-                       disposition:(WindowOpenDisposition)disposition;
-
 // Add a new folder controller as triggered by the given folder button.
 // If there is a current folder controller, close it.
 - (void)addNewFolderControllerWithParentButton:(BookmarkButton*)parentButton;
@@ -122,6 +118,10 @@ class ThemeProvider;
 // Provide a contextual menu for a bookmark node.  May return nil.
 - (NSMenu*)contextMenuForNode:(const BookmarkNode*)node;
 
+// Open all of the nodes for the given node with disposition.
+- (void)openAll:(const BookmarkNode*)node
+    disposition:(WindowOpenDisposition)disposition;
+
 @end  // @protocol BookmarkButtonControllerProtocol
 
 
diff --git a/chrome/browser/cocoa/bookmark_folder_target.mm b/chrome/browser/cocoa/bookmark_folder_target.mm
index 54a868e..ae3e328 100644
--- a/chrome/browser/cocoa/bookmark_folder_target.mm
+++ b/chrome/browser/cocoa/bookmark_folder_target.mm
@@ -52,8 +52,7 @@
   WindowOpenDisposition disposition =
       event_utils::WindowOpenDispositionFromNSEvent([NSApp currentEvent]);
   if (disposition == NEW_BACKGROUND_TAB) {
-    [controller_ openBookmarkNodesRecursive:[sender bookmarkNode]
-                                disposition:disposition];
+    [controller_ openAll:[sender bookmarkNode] disposition:disposition];
     return;
   }