[Mac] Remove no-op method in bookmark bar controller

[Mac] Remove no-op method in bookmark bar controller

BUG=648560
Review-Url: https://codereview.chromium.org/2793363002
Cr-Commit-Position: refs/heads/master@{#461861}
Committed: https://chromium.googlesource.com/chromium/src/+/760e7128bf4d8885d28b25b6386d31421fb35954

[lgrey, 2017-04-04 15:37:22]

The CQ bit was checked by lgrey@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 15:37:57]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 17:06:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-04 17:06:29]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[lgrey, 2017-04-04 17:48:10]

The CQ bit was checked by lgrey@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 17:48:40]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 18:07:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-04 18:07:43]

Dry run: This issue passed the CQ dry run.

[lgrey, 2017-04-04 18:08:17]

lgrey@chromium.org changed reviewers:
+ ellyjones@google.com

[lgrey, 2017-04-04 18:08:18]

PTAL :)

[Elly Fong-Jones, 2017-04-04 19:07:32]

ellyjones@chromium.org changed reviewers:
+ ellyjones@chromium.org

[Elly Fong-Jones, 2017-04-04 19:07:33]

lgtm

It looks like this was originally introduced 7 years ago (!) as the fix for the
linked bug, in https://codereview.chromium.org/1165001/patch/3001/4001

https://codereview.chromium.org/2793363002/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_bridge_unittest.mm (right):

https://codereview.chromium.org/2793363002/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_bridge_unittest.mm:57:
[callbacks_ addObject:[NSNumber numberWithInt:1]];
are these intended to mean something? should they be an enum?

https://codereview.chromium.org/2793363002/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_bridge_unittest.mm:124:
EXPECT_TRUE([controller.get()->callbacks_ count] == 8);
EXPECT_EQ() surely?

[lgrey, 2017-04-04 19:42:21]

https://codereview.chromium.org/2793363002/diff/20001/chrome/browser/ui/cocoa...
File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_bridge_unittest.mm (right):

https://codereview.chromium.org/2793363002/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_bridge_unittest.mm:57:
[callbacks_ addObject:[NSNumber numberWithInt:1]];
On 2017/04/04 19:07:33, Elly Fong-Jones wrote:
> are these intended to mean something? should they be an enum?

Per offline discussion, switched to storing/comparing selectors directly instead
of coding them as NSNumbers.

https://codereview.chromium.org/2793363002/diff/20001/chrome/browser/ui/cocoa...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_bridge_unittest.mm:124:
EXPECT_TRUE([controller.get()->callbacks_ count] == 8);
On 2017/04/04 19:07:33, Elly Fong-Jones wrote:
> EXPECT_EQ() surely?

Done.

[lgrey, 2017-04-04 19:42:25]

The CQ bit was checked by lgrey@chromium.org

[lgrey, 2017-04-04 19:42:25]

The patchset sent to the CQ was uploaded after l-g-t-m from
ellyjones@chromium.org
Link to the patchset: https://codereview.chromium.org/2793363002/#ps40001
(title: "Make test clearer")

[lgrey, 2017-04-04 19:42:26]

The CQ bit was unchecked by lgrey@chromium.org

[lgrey, 2017-04-04 19:42:37]

The CQ bit was checked by lgrey@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-04 19:42:52]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 20:07:39]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-04 20:07:39]

Dry run: This issue passed the CQ dry run.

[lgrey, 2017-04-04 21:59:42]

The CQ bit was checked by lgrey@chromium.org

[commit-bot: I haz the power, 2017-04-04 22:00:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-04 22:08:55]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491343182563170,
"parent_rev": "ebbfebff49abb37df9789e93cb1feb29dd399941", "commit_rev":
"760e7128bf4d8885d28b25b6386d31421fb35954"}

[commit-bot: I haz the power, 2017-04-04 22:09:46]

Description was changed from

==========
[Mac] Remove no-op method in bookmark bar controller

BUG=648560
==========

to

==========
[Mac] Remove no-op method in bookmark bar controller

BUG=648560

Review-Url: https://codereview.chromium.org/2793363002
Cr-Commit-Position: refs/heads/master@{#461861}
Committed:
https://chromium.googlesource.com/chromium/src/+/760e7128bf4d8885d28b25b6386d...
==========

[commit-bot: I haz the power, 2017-04-04 22:09:47]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/760e7128bf4d8885d28b25b6386d...