sdy@ - can you take a look?

 Issue 787053  has been merged into this issue.

Able to reproduce the issue on reported version 64.0.3273.0 and the same is not seen on latest stable 62.0.3202.94 using Mac 10.12.6.

Bisect info:
----------------------
Good build: 64.0.3264.0
Bad build: 64.0.3265.0 

Note: This issue is specific to Mac. 
The chrome behaviour on invoked builds while running per revision bisect script is showing different behaviour which match neither good or bad behaviour. Hence, providing changelog from Omahaproxy
https://chromium.googlesource.com/chromium/src/+log/64.0.3264.0..64.0.3265.0?pretty=fuller&n=10000

As we are unable to find the suspect, request someone from Dev team to take a look at this issue and assign to the right owner. 

As it is a recent regression, adding label RBS. Please feel free to remove the same if not appropriate.
Thanks!!

Okay, based on comment 4, I was able to bisect a narrow regression range:

It was broken between build 515637 and build 515663: 

https://chromium.googlesource.com/chromium/src/+log/99836a8d8cd1fbc20492f696b04695c487843a77..831558e75e852a9225cbb109da5baf52503eb0f5

But really hard to tell, which CL could have cause this :-(

Since merged  issue 787053  has an effect on the Menubar (CMD-W has the same location as the new share button), could this maybe the cause: https://chromium-review.googlesource.com/762217 ?

--> +lgrey@

lgrey@: Please take a look at this and confirm if the suspected change in C#6 could be related to this.

Re #6 I'm not following. Command-W is in the same place, the Share menu replaces "Email  Page Location" in the next section.

In any case, the menu location shouldn't affect the speed. AFAICT it's the same if you quickly click the new tab button.

lgrey@, thank you for your feedback. Do you have an idea, which CL in https://chromium.googlesource.com/chromium/src/+log/99836a8d8cd1fbc20492f696b04695c487843a77..831558e75e852a9225cbb109da5baf52503eb0f5?pretty=fuller&n=10000 could have cause this issue? All tab related changes in this range seem to be iOS or Android related :-(

Thanks.

Not sure. I'm getting the sense that the major change here is that the tab count isn't getting updated immediately in Canary, so the existent tabs don't resize? 

Without digging in too much, https://chromium-review.googlesource.com/c/chromium/src/+/734201 was the last change to the directly relevant code and is just outside that range, so it might be something to look at.

Thanks lgrey@, I rechecked the range and I can confirm that it was broken between build 515637 and build 515663.

Attached a screencast.

May be I am in an experiment flag?!

Deleted: press_and_hold_CMD_T_CMD_W_515637_vs_515663.mov 12.9 MB

press_and_hold_CMD_T_CMD_W_515637_vs_515663.mov 12.9 MB Download

I feel like this shouldn't be a release blocker. The tab strip animations break now when you hold cmd+t/cmd+w, and this is just a slightly different breakage. We should fix the animations either way. shrike@, how do you feel about it?

I also can't get it to happen :/. So, yes, maybe it's related to an experiment? Could you share your hashes from chrome://version?

Hello sdy@,

Here are the requested variations from my Canary Build:

c134752e-3d9db57d
16e0dd70-3f4a17df
da89714-4ad60575
64da5c1e-ca7d8d80
1e528f0f-15305a2
b130ecb8-1410f10
6025934e-3f4a17df
7c1bc906-86bf56d9
d52c4ff7-d52c4ff7
3eb101d6-3f4a17df
47e5d3db-3d47f4f4
b1edbc38-cf4f6ead
1c752ce9-30f1ce65
34d450b1-f23d1dea
79616653-3f4a17df
19c1fdaf-ca7d8d80
f9884634-f5e2d583
3042ad4b-ca7d8d80
9e201a2b-38d7ffb9
44c96c7b-92bf7ff7
57f575bb-3f4a17df
d0ecf1da-f23d1dea
b72f69e9-3f4a17df
f347910c-f23d1dea
4b61504a-dd106449
77bbdddc-f23d1dea
5485fc4d-ca7d8d80
9773d3bd-cc03d394
8e3b2dc5-93702590
9e5c75f1-36ea934f
f79cb77b-3d47f4f4
47edb3-566ebc9b
274c53f-3f4a17df
4ea303a6-3f4a17df
d92562a9-78c337f0
90bcbadc-7737cf5f
2b33233e-f23d1dea
25fc488a-4d2fac87
6973a1cf-3f4a17df
1aecb842-f23d1dea
ad6d27cc-c6d02f41
757a5d98-28165b59
23496387-232b3cab
b2f0086-3f907e47
ef25c1eb-3f4a17df
2d871858-f23d1dea
344833e9-1525b35b
3f273a97-e3ad1896
4bc337ce-66edfe5b
494d8760-5f8667a
3ac60855-486e2a9c
f296190c-a2200d3b
4442aae2-a5822863
ed1d377-e1cc0f14
75f0f0a0-a5822863
e2b18481-a5822863
e7e71889-4ad60575
34baa302-f23d1dea
f5fff3a2-3f4a17df
94e68624-f23d1dea
e9ce63c1-36ab09a2
9cade933-f23d1dea
493ac2c5-ca7d8d80
da4aaa01-3f4a17df

> I feel like this shouldn't be a release blocker. The tab strip animations break now when you hold cmd+t/cmd+w, and this is just a slightly different breakage. We should fix the animations either way. shrike@, how do you feel about it?

It would be nice to understand what changed and make a decision from there about RBS.

mehmet@ - if you disable the Share menu feature in chrome://flags, does the problem go away?

Hello shrike@,

Yes, the problem goes away by disabling chrome://flags/#mac-system-share-menu.

Some sort of knock-on effect from share menu item's menuNeedsUpdate:?

But I don't really get how that would work. AFAICT the new tab still draws and ~same number of of frames. The big thing I noticed was the delayed update on the other tabs which should be in tab-land entirely.

OK, so it looks like |menuNeedsUpdate:| is called every time a hotkey is pressed (eesh!)

We can adapt this approach https://cs.chromium.org/chromium/src/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.mm?type=cs&q=menuHasKeyEquivalent:&sq=package:chromium&l=88 but I think we'd have to prime the menu the first time (see the linked comment.)

The user can't disable the mail share extension, so we could skip priming the menu and just return YES from menuHasKeyEquivalent:..., though that seems a little brittle.

We should probably also audit all uses of |menuNeedsUpdate:| since this is the second time something like this has come up in two weeks (that bookmark code is from last week.)

> Some sort of knock-on effect from share menu item's menuNeedsUpdate:?

I was thinking the Share menu could be causing more updates somewhere. It sounds like menuNeedsUpdate: is the culprit? For a next step I was going to ask mehmet@ to take an Activity Monitor Sample of the browser process while creating new tabs and see if anything stood out. Do you think we still want that or do you think menuNeedsUpdate: is the problem?

I'm not super sensitive to the difference. Since disabling share menu seems to fix it, and I think it's probably important to fix up the |menuNeedsUpdate:| situation either way, I'd like to ship that change and ask mehmet@ to verify.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/431f11316de4ca546042526635700e171443a28c

commit 431f11316de4ca546042526635700e171443a28c
Author: Leonard Grey <lgrey@chromium.org>
Date: Fri Dec 01 14:43:51 2017

[Mac] Don't rebuild share menu every time a hotkey is pressed

Currently, the share menu controller's |menuNeedsUpdate:| is called
every time a hotkey is pressed.

Implementing |menuHasKeyEquivalent:forEvent:target:action:| disables
that path in the menu code, so this change implements that. Similarly
to the bookmark menu, we return NO unconditionally and let the hotkey
be handled by the menu item. To ensure the menu item exists to handle
it, we populate the menu on the first call to |menuHasKeyEquivalent:|
unless it's already been populated.

Bug:  787049 
Change-Id: I321ed87dfc64f89f31c5738712472d33a923a55a
Reviewed-on: https://chromium-review.googlesource.com/801854
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Leonard Grey <lgrey@chromium.org>
Cr-Commit-Position: refs/heads/master@{#520933}
[modify] https://crrev.com/431f11316de4ca546042526635700e171443a28c/chrome/browser/ui/cocoa/share_menu_controller.mm
[modify] https://crrev.com/431f11316de4ca546042526635700e171443a28c/chrome/browser/ui/cocoa/share_menu_controller_browsertest.mm

Tested the issue on Mac 10.12.6 using Chrome version M65 - 65.0.3284.0 as per the issue mentioned in original comment. Observed that issue is working as intended (Upon opening multiple tabs, animation displayed is now similar with Chrome Stable). Hence adding TE-Verified label. Attached the screencast for reference.

Thank you!

Deleted: 787049.mov 2.2 MB

787049.mov 2.2 MB Download

pnangunoori@ the share menu was disabled by default around the same time. Do you think you could take a look with chrome://flags/#mac-system-share-menu enabled?

With chrome://flags/#mac-system-share-menu enabled CMD-T and CMD-W are working as expected again. No more animation lags while creating NewTabs and no more lags while closing these NewTabs. Thanks, lgrey@.

Tested with Version 65.0.3284.0 (Official Build) canary (64-bit) on macOS 10.12.6.

[Auto-generated comment by a script] We noticed that this issue is targeted for M-64; it appears the fix may have landed after branch point, meaning a merge might be required. Please confirm if a merge is required here - if so add Merge-Request-64 label, otherwise remove Merge-TBD label. Thanks.

No merge necessary, this flag is off by default in 64