From 9e6dee2aa4a800e861c8359d3bfdc3c3c10c20f5 Mon Sep 17 00:00:00 2001
From: lgarron <lgarron@chromium.org>
Date: Mon, 17 Nov 2014 17:03:39 -0800
Subject: Cache the bookmarks menu on OSX between profile switches.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, when you switched to a profile, we invalidated the existing
bookmarks menu and updated it lazily. Because of the way OSX works, this causes
the menu to be recalculated from scratch the first time you press *any* keyboard
shortcut afer switching to the profile. On a profile with several hundred
bookmarks, this can take several seconds - more than enough to impact casual
browsing.
In particular, it would appear to the user that various innocent shortcuts (e.g.
opening a new tab, copying text, highlighting the location bar) are extremely
slow some of the time. Even more annoyingly, if you switched to a profile and
then immediately alt-tabbed away from it (for example, to get to a third
profile), the lockup would take place when you switched *away* from the profile
– even if you didn't perform any actions on the profile!

This change caches the bookmarks menu and restores it when a window for the
profile gains focus again. That way, the bookmarks menu is updated only when an
action directly invalidates it (i.e. adding or changing bookmarks themselves).
The accompanying test (BookmarksMenuIsRestoredAfterProfileSwitch) verifies that
the menu is restored correctly after switching profile windows and back.

Thanks to Robert Sesek for extensive help while writing and debugging this fix,
as well as its accompanying test. :-)

BUG=429371
TEST=Follow the steps in comment #27 on bug 429371: https://crbug.com/429371#c27

Review URL: https://codereview.chromium.org/733673003

Cr-Commit-Position: refs/heads/master@{#304531}
---
 chrome/browser/app_controller_mac.mm | 36 ++++++++++++++++++++++++++++--------
 1 file changed, 28 insertions(+), 8 deletions(-)

(limited to 'chrome/browser/app_controller_mac.mm')

diff --git a/chrome/browser/app_controller_mac.mm b/chrome/browser/app_controller_mac.mm
index 2c17fa2..f234859 100644
--- a/chrome/browser/app_controller_mac.mm
+++ b/chrome/browser/app_controller_mac.mm
@@ -14,6 +14,7 @@
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
+#include "base/stl_util.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
@@ -480,6 +481,9 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
   [self unregisterEventHandlers];
 
   appShimMenuController_.reset();
+
+  STLDeleteContainerPairSecondPointers(profileBookmarkMenuBridgeMap_.begin(),
+                                       profileBookmarkMenuBridgeMap_.end());
 }
 
 - (void)didEndMainMessageLoop {
@@ -887,6 +891,14 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
   // to the old profile.
   if (profilePath == lastProfile->GetPath())
     lastProfile_ = g_browser_process->profile_manager()->GetLastUsedProfile();
+
+  Profile* profile =
+      g_browser_process->profile_manager()->GetProfile(profilePath);
+  auto it = profileBookmarkMenuBridgeMap_.find(profile);
+  if (it != profileBookmarkMenuBridgeMap_.end()) {
+    delete it->second;
+    profileBookmarkMenuBridgeMap_.erase(it);
+  }
 }
 
 // Returns true if there is a modal window (either window- or application-
@@ -1520,7 +1532,7 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
 }
 
 - (BookmarkMenuBridge*)bookmarkMenuBridge {
-  return bookmarkMenuBridge_.get();
+  return bookmarkMenuBridge_;
 }
 
 - (void)addObserverForWorkAreaChange:(ui::WorkAreaWatcherObserver*)observer {
@@ -1540,18 +1552,26 @@ class AppControllerProfileObserver : public ProfileInfoCacheObserver {
   if (lastProfile_ == profile)
     return;
 
-  // Before tearing down the menu controller bridges, return the Cocoa menus to
-  // their initial state.
-  if (bookmarkMenuBridge_.get())
-    bookmarkMenuBridge_->ResetMenu();
-  if (historyMenuBridge_.get())
+  // Before tearing down the menu controller bridges, return the history menu to
+  // its initial state.
+  if (historyMenuBridge_)
     historyMenuBridge_->ResetMenu();
 
   // Rebuild the menus with the new profile.
   lastProfile_ = profile;
 
-  bookmarkMenuBridge_.reset(new BookmarkMenuBridge(lastProfile_,
-      [[[NSApp mainMenu] itemWithTag:IDC_BOOKMARKS_MENU] submenu]));
+  auto it = profileBookmarkMenuBridgeMap_.find(profile);
+  if (it == profileBookmarkMenuBridgeMap_.end()) {
+    base::scoped_nsobject<NSMenu> submenu(
+        [[[[NSApp mainMenu] itemWithTag:IDC_BOOKMARKS_MENU] submenu] copy]);
+    bookmarkMenuBridge_ = new BookmarkMenuBridge(lastProfile_, submenu);
+    profileBookmarkMenuBridgeMap_[profile] = bookmarkMenuBridge_;
+  } else {
+    bookmarkMenuBridge_ = it->second;
+  }
+
+  [[[NSApp mainMenu] itemWithTag:IDC_BOOKMARKS_MENU] setSubmenu:
+      bookmarkMenuBridge_->BookmarkMenu()];
   // No need to |BuildMenu| here.  It is done lazily upon menu access.
 
   historyMenuBridge_.reset(new HistoryMenuBridge(lastProfile_));
-- 
cgit v1.1