Better cancel bookmark edit dialogs when needed.

BUG=39333

TEST=Create 2 windows.
Add a new folder on the bookmark bar.
Add Page... on it.
In 2nd window delete --> dialog should ben canceled.
Add a new folder on the bookmark bar.
Add Folder... on it.
In 2nd window delete it --> dialog should ben canceled.

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

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

5 files changed, 25 insertions, 23 deletions

diff --git a/chrome/browser/cocoa/bookmark_editor_base_controller.h b/chrome/browser/cocoa/bookmark_editor_base_controller.h
index bb447b5..5fbdf5e 100644
--- a/chrome/browser/cocoa/bookmark_editor_base_controller.h
+++ b/chrome/browser/cocoa/bookmark_editor_base_controller.h
@@ -104,7 +104,7 @@ class BookmarkModel;
 // Notifications called when the BookmarkModel changes out from under me.
 - (void)nodeRemoved:(const BookmarkNode*)node
          fromParent:(const BookmarkNode*)parent;
-- (void)modelChanged;
+- (void)modelChangedPreserveSelection:(BOOL)preserve;
 
 // Accessors
 - (BookmarkModel*)bookmarkModel;
diff --git a/chrome/browser/cocoa/bookmark_editor_base_controller.mm b/chrome/browser/cocoa/bookmark_editor_base_controller.mm
index 77b1235..8ecbc47 100644
--- a/chrome/browser/cocoa/bookmark_editor_base_controller.mm
+++ b/chrome/browser/cocoa/bookmark_editor_base_controller.mm
@@ -28,7 +28,9 @@
 - (void)buildFolderTree;
 
 // Notifies the controller that the bookmark model has changed.
-- (void)modelChanged;
+// |selection| specifies if the current selection should be
+// maintained (usually YES).
+- (void)modelChangedPreserveSelection:(BOOL)preserve;
 
 // Notifies the controller that a node has been removed.
 - (void)nodeRemoved:(const BookmarkNode*)node
@@ -87,7 +89,7 @@ class BookmarkEditorBaseControllerBridge : public BookmarkModelObserver {
   { }
 
   virtual void Loaded(BookmarkModel* model) {
-    [controller_ modelChanged];
+    [controller_ modelChangedPreserveSelection:YES];
   }
 
   virtual void BookmarkNodeMoved(BookmarkModel* model,
@@ -96,14 +98,14 @@ class BookmarkEditorBaseControllerBridge : public BookmarkModelObserver {
                                  const BookmarkNode* new_parent,
                                  int new_index) {
     if (!importing_ && new_parent->GetChild(new_index)->is_folder())
-      [controller_ modelChanged];
+      [controller_ modelChangedPreserveSelection:YES];
   }
 
   virtual void BookmarkNodeAdded(BookmarkModel* model,
                                  const BookmarkNode* parent,
                                  int index) {
     if (!importing_ && parent->GetChild(index)->is_folder())
-      [controller_ modelChanged];
+      [controller_ modelChangedPreserveSelection:YES];
   }
 
   virtual void BookmarkNodeRemoved(BookmarkModel* model,
@@ -112,19 +114,19 @@ class BookmarkEditorBaseControllerBridge : public BookmarkModelObserver {
                                    const BookmarkNode* node) {
     [controller_ nodeRemoved:node fromParent:parent];
     if (node->is_folder())
-      [controller_ modelChanged];
+      [controller_ modelChangedPreserveSelection:NO];
   }
 
   virtual void BookmarkNodeChanged(BookmarkModel* model,
                                    const BookmarkNode* node) {
     if (!importing_ && node->is_folder())
-      [controller_ modelChanged];
+      [controller_ modelChangedPreserveSelection:YES];
   }
 
   virtual void BookmarkNodeChildrenReordered(BookmarkModel* model,
                                              const BookmarkNode* node) {
     if (!importing_)
-      [controller_ modelChanged];
+      [controller_ modelChangedPreserveSelection:YES];
   }
 
   virtual void BookmarkNodeFavIconLoaded(BookmarkModel* model,
@@ -140,7 +142,7 @@ class BookmarkEditorBaseControllerBridge : public BookmarkModelObserver {
   // themselves if they were waiting for the update to finish.
   virtual void BookmarkImportEnding(BookmarkModel* model) {
     importing_ = false;
-    [controller_ modelChanged];
+    [controller_ modelChangedPreserveSelection:YES];
   }
 
  private:
@@ -426,10 +428,12 @@ class BookmarkEditorBaseControllerBridge : public BookmarkModelObserver {
   [self setFolderTreeArray:baseArray];
 }
 
-- (void)modelChanged {
+- (void)modelChangedPreserveSelection:(BOOL)preserve {
   const BookmarkNode* selectedNode = [self selectedNode];
   [self buildFolderTree];
-  if (selectedNode && configuration_ == BookmarkEditor::SHOW_TREE)
+  if (preserve &&
+      selectedNode &&
+      configuration_ == BookmarkEditor::SHOW_TREE)
     [self selectNodeInBrowser:selectedNode];
 }
 
diff --git a/chrome/browser/cocoa/bookmark_editor_controller.mm b/chrome/browser/cocoa/bookmark_editor_controller.mm
index 364feec..3cfe881 100644
--- a/chrome/browser/cocoa/bookmark_editor_controller.mm
+++ b/chrome/browser/cocoa/bookmark_editor_controller.mm
@@ -63,11 +63,9 @@
 - (void)nodeRemoved:(const BookmarkNode*)node
          fromParent:(const BookmarkNode*)parent
 {
-  if (node_ == node || node_->HasAncestor(node)) {
-    // The node I'm editing is being deleted. Just close.
-    node_ = NULL;
-    [self cancel:self];
-  }
+  // Be conservative; it is needed (e.g. "Add Page...")
+  node_ = NULL;
+  [self cancel:self];
 }
 
 #pragma mark Bookmark Editing
diff --git a/chrome/browser/cocoa/bookmark_model_observer_for_cocoa.h b/chrome/browser/cocoa/bookmark_model_observer_for_cocoa.h
index 01acb69..398b2b8 100644
--- a/chrome/browser/cocoa/bookmark_model_observer_for_cocoa.h
+++ b/chrome/browser/cocoa/bookmark_model_observer_for_cocoa.h
@@ -31,11 +31,13 @@ class BookmarkModelObserverForCocoa : public BookmarkModelObserver {
   // When |node| in |model| changes, send |selector| to |object|.
   // Assumes |selector| is a selector that takes one arg, like an
   // IBOutlet.  The arg passed is nil.
+  // Many notifications happen independently of node
+  // (e.g. BeingDeleted), so |node| can be nil.
   BookmarkModelObserverForCocoa(const BookmarkNode* node,
                                 BookmarkModel* model,
                                 NSObject* object,
                                 SEL selector) {
-    DCHECK(node && model);
+    DCHECK(model);
     node_ = node;
     model_ = model;
     object_.reset([object retain]);
diff --git a/chrome/browser/cocoa/bookmark_name_folder_controller.mm b/chrome/browser/cocoa/bookmark_name_folder_controller.mm
index ef6cdc1..108d87a 100644
--- a/chrome/browser/cocoa/bookmark_name_folder_controller.mm
+++ b/chrome/browser/cocoa/bookmark_name_folder_controller.mm
@@ -71,12 +71,10 @@
 
 - (void)runAsModalSheet {
   // Ping me when things change out from under us.
-  if (node_)
-    observer_.reset(new BookmarkModelObserverForCocoa(
-                      node_, profile_->GetBookmarkModel(),
-                      self,
-                      @selector(cancel:)));
-
+  observer_.reset(new BookmarkModelObserverForCocoa(
+                    node_, profile_->GetBookmarkModel(),
+                    self,
+                    @selector(cancel:)));
   [NSApp beginSheet:[self window]
      modalForWindow:parentWindow_
       modalDelegate:self