summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--chrome/browser/cocoa/bookmark_bar_controller.h3
-rw-r--r--chrome/browser/cocoa/bookmark_bar_controller.mm24
-rw-r--r--chrome/browser/cocoa/bookmark_bar_controller_unittest.mm26
-rw-r--r--chrome/browser/cocoa/bookmark_model_observer_for_cocoa.h10
4 files changed, 59 insertions, 4 deletions
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.h b/chrome/browser/cocoa/bookmark_bar_controller.h
index 29952b8..6b9e428 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.h
+++ b/chrome/browser/cocoa/bookmark_bar_controller.h
@@ -334,7 +334,8 @@ willAnimateFromState:(bookmarks::VisualState)oldState
- (BOOL)isEventAnExitEvent:(NSEvent*)event;
- (id)folderTarget;
- (int)displayedButtonCount;
-
+- (NSMenu*)buttonContextMenu;
+- (void)setButtonContextMenu:(id)menu;
@end
// The (internal) |NSPasteboard| type string for bookmark button drags, used for
diff --git a/chrome/browser/cocoa/bookmark_bar_controller.mm b/chrome/browser/cocoa/bookmark_bar_controller.mm
index ca8110c..bbb43d0 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller.mm
@@ -549,6 +549,16 @@ const NSTimeInterval kBookmarkBarAnimationDuration = 0.12;
return displayedButtonCount_;
}
+- (NSMenu*)buttonContextMenu {
+ return buttonContextMenu_;
+}
+
+// Intentionally ignores ownership issues; used for testing and we try
+// to minimize touching the object passed in (likely a mock).
+- (void)setButtonContextMenu:(id)menu {
+ buttonContextMenu_ = menu;
+}
+
// Keep the "no items" label centered in response to a frame size change.
- (void)centerNoItemsLabel {
// Note that this computation is done in the parent's coordinate system,
@@ -1727,8 +1737,19 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
[self configureOffTheSideButtonContentsAndVisibility];
}
+// To avoid problems with sync, changes that may impact the current
+// bookmark (e.g. deletion) make sure context menus are closed. This
+// prevents deleting a node which no longer exists.
+- (void)cancelMenuTracking {
+ [buttonContextMenu_ cancelTracking];
+ [buttonFolderContextMenu_ cancelTracking];
+}
+
- (void)nodeAdded:(BookmarkModel*)model
parent:(const BookmarkNode*)newParent index:(int)newIndex {
+ // If a context menu is open, close it.
+ [self cancelMenuTracking];
+
const BookmarkNode* newNode = newParent->GetChild(newIndex);
id<BookmarkButtonControllerProtocol> newController =
[self controllerForNode:newParent];
@@ -1740,6 +1761,9 @@ static BOOL ValueInRangeInclusive(CGFloat low, CGFloat value, CGFloat high) {
- (void)nodeRemoved:(BookmarkModel*)model
parent:(const BookmarkNode*)oldParent index:(int)index {
+ // If a context menu is open, close it.
+ [self cancelMenuTracking];
+
// Locate the parent node. The parent may not be showing, in which case
// we do nothing.
id<BookmarkButtonControllerProtocol> parentController =
diff --git a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
index a80b7e5..bb4e8b4 100644
--- a/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
+++ b/chrome/browser/cocoa/bookmark_bar_controller_unittest.mm
@@ -1329,6 +1329,32 @@ TEST_F(BookmarkBarControllerTest, NodeDeletedWhileMenuIsOpen) {
[bar_ openAllBookmarksIncognitoWindow:item];
}
+TEST_F(BookmarkBarControllerTest, NodeDeletedWhileContextMenuIsOpen) {
+ BookmarkModel* model = helper_.profile()->GetBookmarkModel();
+ [bar_ loaded:model];
+
+ const BookmarkNode* parent = model->GetBookmarkBarNode();
+ const BookmarkNode* folder = model->AddGroup(parent,
+ parent->GetChildCount(),
+ L"group");
+ const BookmarkNode* framma = model->AddURL(folder, folder->GetChildCount(),
+ L"f1",
+ GURL("http://framma-lamma.com"));
+
+ // Mock in a menu
+ id origMenu = [bar_ buttonContextMenu];
+ id fakeMenu = [OCMockObject partialMockForObject:origMenu];
+ [[fakeMenu expect] cancelTracking];
+ [bar_ setButtonContextMenu:fakeMenu];
+
+ // Force a delete which should cancelTracking on the menu.
+ model->Remove(framma->GetParent(), framma->GetParent()->IndexOfChild(framma));
+
+ // Restore, then confirm cancelTracking was called.
+ [bar_ setButtonContextMenu:origMenu];
+ [fakeMenu verify];
+}
+
TEST_F(BookmarkBarControllerTest, CloseFolderOnAnimate) {
BookmarkModel* model = helper_.profile()->GetBookmarkModel();
const BookmarkNode* parent = model->GetBookmarkBarNode();
diff --git a/chrome/browser/cocoa/bookmark_model_observer_for_cocoa.h b/chrome/browser/cocoa/bookmark_model_observer_for_cocoa.h
index 398b2b8..b5ba908 100644
--- a/chrome/browser/cocoa/bookmark_model_observer_for_cocoa.h
+++ b/chrome/browser/cocoa/bookmark_model_observer_for_cocoa.h
@@ -33,6 +33,10 @@ class BookmarkModelObserverForCocoa : public BookmarkModelObserver {
// IBOutlet. The arg passed is nil.
// Many notifications happen independently of node
// (e.g. BeingDeleted), so |node| can be nil.
+ //
+ // |object| is NOT retained, since the expected use case is for
+ // ||object| to own the BookmarkModelObserverForCocoa and we don't
+ // want a retain cycle.
BookmarkModelObserverForCocoa(const BookmarkNode* node,
BookmarkModel* model,
NSObject* object,
@@ -40,7 +44,7 @@ class BookmarkModelObserverForCocoa : public BookmarkModelObserver {
DCHECK(model);
node_ = node;
model_ = model;
- object_.reset([object retain]);
+ object_ = object;
selector_ = selector;
model_->AddObserver(this);
}
@@ -69,7 +73,7 @@ class BookmarkModelObserverForCocoa : public BookmarkModelObserver {
}
virtual void BookmarkNodeChanged(BookmarkModel* model,
const BookmarkNode* node) {
- if (node_ == node)
+ if ((node_ == node) || (!node_))
Notify();
}
virtual void BookmarkImportBeginning(BookmarkModel* model) {
@@ -98,7 +102,7 @@ class BookmarkModelObserverForCocoa : public BookmarkModelObserver {
private:
const BookmarkNode* node_; // Weak; owned by a BookmarkModel.
BookmarkModel* model_; // Weak; it is owned by a Profile.
- scoped_nsobject<NSObject> object_;
+ NSObject* object_; // Weak, like a delegate.
SEL selector_;
void Notify() {