New issue
Advanced search Search tips

Issue 786845 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

Mac: With lots of bookmarks, small delay when hitting up arrow each time the Chrome menu is shown

Project Member Reported by tapted@chromium.org, Nov 20 2017

Issue description

Chrome Version       : 64.0.3273.0
OS Version: OS X 10.12.6

What steps will reproduce the problem?
1. Have lots of bookmarks (~1000)
2. Open Chrome app/hotdog menu
3. Press up arrow

What is the expected result?

Menu highlight should immediately move to 'Help'


What happens instead of that?

Moves to help after a small but noticeable delay


There's some info what's happening in  Issue 733921 . See also  Issue 785522 .

The mainMenu bar caches BookmarkMenuBridge, but the app menu does not.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 22 2017

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

commit cf95689a88664eb44716632cc7320d18ed3324ba
Author: Trent Apted <tapted@chromium.org>
Date: Wed Nov 22 00:01:10 2017

Mac: Lazily populate bookmarks menus.

Currently the first time you press a key (any key) in WebContent
on Chrome Mac, Chrome makes a NSMenuItem for every bookmark you have,
and spawns tasks to convert each favicon. These then persist in memory
to service the Bookmarks menu on the menubar.

On my profile with ~1000 bookmarks, Chrome jumps from 95MB to 105MB
resident memory if I open about:blank and press a key. And there is
noticeable lag. Each browser window's App menu also has a full
bookmarks tree, populated once the 'Bookmarks' item is hovered over,
and persisted in memory; taking up about 800kB (approximately doubling
the browser process memory used by a single-tab browser window).

In order to populate these lazily, the bookmarks menu must respond `NO`
to -[NSMenuDelegate menuHasKeyEquivalent:..]. Otherwise the menu is
fully populated on the first key event to see if any bookmark titles
match the event. With this in place, we can leave all bookmarks
submenus empty, but set a delegate that AppKit uses to populate the
menu when the user first shows it.

Bug:  33102 ,  647183 ,  733921 ,  786845 
Change-Id: Idc0a28cf9483f7161dc3fbe9bd6cda717e65de98
Reviewed-on: https://chromium-review.googlesource.com/778644
Commit-Queue: Trent Apted <tapted@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#518459}
[modify] https://crrev.com/cf95689a88664eb44716632cc7320d18ed3324ba/chrome/browser/app_controller_mac.mm
[modify] https://crrev.com/cf95689a88664eb44716632cc7320d18ed3324ba/chrome/browser/app_controller_mac_browsertest.mm
[modify] https://crrev.com/cf95689a88664eb44716632cc7320d18ed3324ba/chrome/browser/ui/cocoa/app_menu/app_menu_controller.mm
[modify] https://crrev.com/cf95689a88664eb44716632cc7320d18ed3324ba/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.h
[modify] https://crrev.com/cf95689a88664eb44716632cc7320d18ed3324ba/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge.mm
[modify] https://crrev.com/cf95689a88664eb44716632cc7320d18ed3324ba/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_bridge_unittest.mm
[modify] https://crrev.com/cf95689a88664eb44716632cc7320d18ed3324ba/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.h
[modify] https://crrev.com/cf95689a88664eb44716632cc7320d18ed3324ba/chrome/browser/ui/cocoa/bookmarks/bookmark_menu_cocoa_controller.mm

Comment 2 by tapted@chromium.org, Nov 23 2017

Status: Fixed (was: Started)

Sign in to add a comment