[Mac][Material Design] Improve bookmark folder menu performance.

[Mac][Material Design] Improve bookmark folder menu performance.

This cl fixes a problem where if you move the mouse over an open
bookmark folder window, if you move the mouse fast enough the hover
state drawing begins to lag the mouse position.

R=tapted@chromium.org
BUG=617803
Committed: https://crrev.com/b4e9a989d00a4753d2d6e1f5229da4ae9a302b5d
Cr-Commit-Position: refs/heads/master@{#401663}

[shrike, 2016-06-16 18:39:13]

PTAL

[tapted, 2016-06-17 05:00:42]

https://codereview.chromium.org/2073753004/diff/1/chrome/browser/ui/cocoa/boo...
File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
(right):

https://codereview.chromium.org/2073753004/diff/1/chrome/browser/ui/cocoa/boo...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm:1386:
NSPoint currentMouseLocation =
Does [[button cell] isMouseReallyInside] do the job?

https://codereview.chromium.org/2073753004/diff/1/chrome/browser/ui/cocoa/boo...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm:1421: [self
setSelectedButtonByIndex:-1];
There's an indenting bug here - this line is indented, but it's not part of the
`if` statement. Could that be generating extra work?

[shrike, 2016-06-21 23:25:21]

PTAL

https://codereview.chromium.org/2073753004/diff/1/chrome/browser/ui/cocoa/boo...
File chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm
(right):

https://codereview.chromium.org/2073753004/diff/1/chrome/browser/ui/cocoa/boo...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm:1386:
NSPoint currentMouseLocation =
On 2016/06/17 05:00:42, tapted wrote:
> Does [[button cell] isMouseReallyInside] do the job?

Yes, thank you.

https://codereview.chromium.org/2073753004/diff/1/chrome/browser/ui/cocoa/boo...
chrome/browser/ui/cocoa/bookmarks/bookmark_bar_folder_controller.mm:1421: [self
setSelectedButtonByIndex:-1];
On 2016/06/17 05:00:42, tapted wrote:
> There's an indenting bug here - this line is indented, but it's not part of
the
> `if` statement. Could that be generating extra work?

It doesn't look like it. It happened in 2011, and fixing it (which I did) does
not make a difference on performance.

[tapted, 2016-06-21 23:49:45]

lgtm

[shrike, 2016-06-22 18:50:49]

The CQ bit was checked by shrike@chromium.org

[commit-bot: I haz the power, 2016-06-22 18:51:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2073753004/20001

[commit-bot: I haz the power, 2016-06-22 19:53:30]

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

[commit-bot: I haz the power, 2016-06-22 19:53:31]

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

[shrike, 2016-06-22 21:01:16]

tapted@ - one of the BookmarkBarFolderController unit tests is now failing
because mouseEnteredButton:event: now checks that the mouse is over the button
(which it isn't, because it's a test).

The test sends a nil event to mouseEnteredButton:event:, while normal callers
send an actual event instance. How do you feel about using a nil event to decide
on skipping the isMouseReallyInside check?

[tapted, 2016-06-23 00:04:41]

On 2016/06/22 21:01:16, shrike wrote:
> tapted@ - one of the BookmarkBarFolderController unit tests is now failing
> because mouseEnteredButton:event: now checks that the mouse is over the button
> (which it isn't, because it's a test).
> 
> The test sends a nil event to mouseEnteredButton:event:, while normal callers
> send an actual event instance. How do you feel about using a nil event to
decide
> on skipping the isMouseReallyInside check?

I think that's reasonable, with a good comment above it.
event_generator_delegate_mac.mm has some stuff for swizzling out things like
+[NSEvent mouseLocation] and +[NSApp currentEvent]  (although nothing has needed
+[NSWindow mouseLocationOutsideEventStream] yet). But only MacViews is using
event_generator_delegate_mac.mm for now.

[shrike, 2016-06-23 17:55:47]

On 2016/06/23 00:04:41, tapted wrote:
> I think that's reasonable, with a good comment above it.
> event_generator_delegate_mac.mm has some stuff for swizzling out things like
> +[NSEvent mouseLocation] and +[NSApp currentEvent]  (although nothing has
needed
> +[NSWindow mouseLocationOutsideEventStream] yet). But only MacViews is using
> event_generator_delegate_mac.mm for now.

OK, sounds good.

[shrike, 2016-06-23 17:56:05]

The CQ bit was checked by shrike@chromium.org

[shrike, 2016-06-23 17:56:05]

The patchset sent to the CQ was uploaded after l-g-t-m from tapted@chromium.org
Link to the patchset: https://codereview.chromium.org/2073753004/#ps40001
(title: "Get unit tests running again.")

[commit-bot: I haz the power, 2016-06-23 17:56:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2073753004/40001

[commit-bot: I haz the power, 2016-06-23 18:47:21]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-23 18:51:40]

Description was changed from

==========
[Mac][Material Design] Improve bookmark folder menu performance.

This cl fixes a problem where if you move the mouse over an open
bookmark folder window, if you move the mouse fast enough the hover
state drawing begins to lag the mouse position.

R=tapted@chromium.org
BUG=617803
==========

to

==========
[Mac][Material Design] Improve bookmark folder menu performance.

This cl fixes a problem where if you move the mouse over an open
bookmark folder window, if you move the mouse fast enough the hover
state drawing begins to lag the mouse position.

R=tapted@chromium.org
BUG=617803

Committed: https://crrev.com/b4e9a989d00a4753d2d6e1f5229da4ae9a302b5d
Cr-Commit-Position: refs/heads/master@{#401663}
==========

[commit-bot: I haz the power, 2016-06-23 18:51:41]

Patchset 3 (id:??) landed as
https://crrev.com/b4e9a989d00a4753d2d6e1f5229da4ae9a302b5d
Cr-Commit-Position: refs/heads/master@{#401663}