Implement "open link as user" context menu entry

chrome/browser/renderer_context_menu/render_view_context_menu.cc

Index: chrome/browser/renderer_context_menu/render_view_context_menu.cc
diff --git a/chrome/browser/renderer_context_menu/render_view_context_menu.cc b/chrome/browser/renderer_context_menu/render_view_context_menu.cc
index 58c38603bc9b9e6ae64344d07ccfbd529ca08126..1c636dfe31499e5fec14012a4cab1ae803763aeb 100644
--- a/chrome/browser/renderer_context_menu/render_view_context_menu.cc
+++ b/chrome/browser/renderer_context_menu/render_view_context_menu.cc
@@ -37,7 +37,11 @@
 #include "chrome/browser/plugins/chrome_plugin_service_filter.h"
 #include "chrome/browser/prefs/incognito_mode_prefs.h"
 #include "chrome/browser/profiles/profile.h"
+#include "chrome/browser/profiles/profile_avatar_icon_util.h"
+#include "chrome/browser/profiles/profile_info_cache.h"
 #include "chrome/browser/profiles/profile_io_data.h"
+#include "chrome/browser/profiles/profile_manager.h"
+#include "chrome/browser/profiles/profile_window.h"
 #include "chrome/browser/renderer_context_menu/context_menu_content_type_factory.h"
 #include "chrome/browser/renderer_context_menu/spelling_menu_observer.h"
 #include "chrome/browser/search/search.h"
@@ -51,6 +55,7 @@
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_commands.h"
 #include "chrome/browser/ui/browser_finder.h"
+#include "chrome/browser/ui/browser_navigator_params.h"
 #include "chrome/browser/ui/chrome_pages.h"
 #include "chrome/browser/ui/tab_contents/core_tab_helper.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
@@ -241,9 +246,10 @@ const struct UmaEnumCommandIdPair {
     {66, -1, IDC_CONTENT_CONTEXT_LOAD_ORIGINAL_IMAGE},
     {67, -1, IDC_CONTENT_CONTEXT_FORCESAVEPASSWORD},
     {68, -1, IDC_ROUTE_MEDIA},
+    {69, -1, IDC_OPEN_LINK_IN_PROFILE_FIRST},
     // Add new items here and use |enum_id| from the next line.
     // Also, add new items to RenderViewContextMenuItem enum in histograms.xml.
-    {69, -1, 0},  // Must be the last. Increment |enum_id| when new IDC
+    {70, -1, 0},  // Must be the last. Increment |enum_id| when new IDC
                   // was added.
 };
 
@@ -267,6 +273,11 @@ int CollapseCommandsForUMA(int id) {
     return IDC_SPELLCHECK_SUGGESTION_0;
   }
 
+  if (id >= IDC_OPEN_LINK_IN_PROFILE_FIRST &&
+      id <= IDC_OPEN_LINK_IN_PROFILE_LAST) {
+    return IDC_OPEN_LINK_IN_PROFILE_FIRST;
+  }
+
   return id;
 }
 
@@ -322,9 +333,8 @@ const GURL& GetDocumentURL(const content::ContextMenuParams& params) {
   return params.frame_url.is_empty() ? params.page_url : params.frame_url;
 }
 
-content::Referrer CreateSaveAsReferrer(
-    const GURL& url,
-    const content::ContextMenuParams& params) {
+content::Referrer CreateReferrer(const GURL& url,
+                                 const content::ContextMenuParams& params) {
   const GURL& referring_url = GetDocumentURL(params);
   return content::Referrer::SanitizeForRequest(
       url,
@@ -363,6 +373,30 @@ void WriteURLToClipboard(const GURL& url, const std::string& languages) {
 
 bool g_custom_id_ranges_initialized = false;
 
+void AddIconToLastMenuItem(gfx::Image icon, ui::SimpleMenuModel* menu) {
+  int width = icon.Width();
+  int height = icon.Height();
+  gfx::CalculateFaviconTargetSize(&width, &height);
+  menu->SetIcon(menu->GetItemCount() - 1,
+                profiles::GetSizedAvatarIcon(icon, true, width, height));
+}
+
+void OnProfileCreated(chrome::HostDesktopType desktop_type,
+                      const GURL& link_url,
+                      const content::Referrer& referrer,
+                      Profile* profile,
+                      Profile::CreateStatus status) {
+  if (status == Profile::CREATE_STATUS_INITIALIZED) {
+    Browser* browser = chrome::FindLastActiveWithProfile(profile, desktop_type);
+    chrome::NavigateParams nav_params(browser, link_url,
+                                      ui::PAGE_TRANSITION_LINK);
+    nav_params.disposition = NEW_FOREGROUND_TAB;
+    nav_params.referrer = referrer;
+    nav_params.window_action = chrome::NavigateParams::SHOW_WINDOW;
+    chrome::Navigate(&nav_params);
+  }
+}
+
 }  // namespace
 
 // static
@@ -404,6 +438,7 @@ RenderViewContextMenu::RenderViewContextMenu(
                        this,
                        &menu_model_,
                        base::Bind(MenuItemMatchesParams, params_)),
+      profile_link_submenu_model_(this),
       protocol_handler_submenu_model_(this),
       protocol_handler_registry_(
           ProtocolHandlerRegistryFactory::GetForBrowserContext(GetProfile())),
@@ -803,6 +838,64 @@ void RenderViewContextMenu::AppendLinkItems() {
 
     menu_model_.AddItemWithStringId(IDC_CONTENT_CONTEXT_OPENLINKOFFTHERECORD,
                                     IDS_CONTENT_CONTEXT_OPENLINKOFFTHERECORD);
+
+    // g_browser_process->profile_manager() is null during unit tests.
+    if (g_browser_process->profile_manager() &&
+        GetProfile()->GetProfileType() == Profile::REGULAR_PROFILE) {
+      ProfileManager* profile_manager = g_browser_process->profile_manager();
+      const ProfileInfoCache& profile_info_cache =
+          profile_manager->GetProfileInfoCache();
+
+      // Find all regular profiles other than the current one which have at
+      // least one open window.
+      std::vector<size_t> target_profiles;
+      std::vector<size_t> all_target_profiles;
+      chrome::HostDesktopType desktop_type =
+          chrome::GetHostDesktopTypeForNativeView(
+              source_web_contents_->GetNativeView());
+
+      const size_t profile_count = profile_info_cache.GetNumberOfProfiles();
+      for (size_t profile_index = 0; profile_index < profile_count;
+           ++profile_index) {
+        base::FilePath profile_path =
+            profile_info_cache.GetPathOfProfileAtIndex(profile_index);
+        Profile* profile = profile_manager->GetProfileByPath(profile_path);
+        if ((profile != GetProfile()) &&
+            !profile_info_cache.IsOmittedProfileAtIndex(profile_index)) {
+          all_target_profiles.push_back(profile_index);
+          if (chrome::FindLastActiveWithProfile(profile, desktop_type))
+            target_profiles.push_back(profile_index);
+        }
+      }
+
+      if (target_profiles.size() == 1 ||
+          (target_profiles.size() == 2 && all_target_profiles.size() == 2)) {

[Peter Kasting, 2015/10/13 18:59:00]

This doesn't seem to match the slides.  Unless I'm confused, this implements:

  * If only one profile active, hide menu items.
  * If two profiles active, show top-level menu item.
  * If three profiles active and there are only three total profiles, show
top-level menu item.
  * Otherwise, show submenu.

I don't know why the third bullet is there.

[jochen (gone - plz use gerrit), 2015/10/13 19:07:30]

that's step 2 b i) in the slides

[Peter Kasting, 2015/10/13 19:16:38]

No, it's not 2bi.  2bi is the second bullet I wrote:

  if (target_profiles.size() == 1) ...

The problem is the size 2 case, which deals with having _3_ profiles.  That's
supposed to go in a submenu.

Unless the slides are incredibly confusing and "2 profiles active" and "only 2
profiles" are counting different things (active versus total profiles), in which
case (a) the slides need to be reworded, and (b) this code is wrong anyway
because you don't show both other profiles in the (target_profiles.size() == 1
&& all_target_profiles.size() == 2) case.

I think the rules on this are too complicated.  You should do the algorithm you
originally set out to do:
* Only show profiles from active windows.
* If one such profile, use a top-level slot.  Otherwise use a context menu.

This is simpler to implement, simpler to reason about as a user, and won't spam
you with tons of profile links you don't care about.  I don't find Alex' user
switcher argument compelling unless we want to show all other profiles all the
time.  The inconsistency of sometimes showing them all and sometimes not is a
much bigger disadvantage than the consistency with the switcher in some cases is
an advantage.  If you want to show all other profiles every time there's more
than one total profile currently open, I will support that.

+        for (size_t profile_index : target_profiles) {

[Peter Kasting, 2015/10/13 18:59:00]

In extreme edge cases, this might add more profiles than we have command IDs
reserved.

[jochen (gone - plz use gerrit), 2015/10/13 19:50:22]

added a bail-out

+          menu_model_.AddItem(
+              IDC_OPEN_LINK_IN_PROFILE_FIRST + profile_index,
+              l10n_util::GetStringFUTF16(
+                  IDS_CONTENT_CONTEXT_OPENLINKINPROFILE,
+                  profile_info_cache.GetNameOfProfileAtIndex(profile_index)));
+          AddIconToLastMenuItem(
+              profile_info_cache.GetAvatarIconOfProfileAtIndex(profile_index),
+              &menu_model_);
+        }
+      } else if (!target_profiles.empty()) {
+        for (size_t profile_index : all_target_profiles) {

[Peter Kasting, 2015/10/13 18:59:00]

I don't think we should do what this implements.

This basically says that at some point we should show targets for all profiles,
even ones that aren't open.  I don't think we should ever do that.  If we should
do that, we should do it all the time.  I really don't think we should switch to
this behavior once we reach some threshold number of open profiles.

[jochen (gone - plz use gerrit), 2015/10/13 19:07:30]

Alex' argument is that this matches what the user switcher menu will show. If
you only have one additional profile, the desire to not show too many entries
trumps being consistent with the user switcher menu.

+          profile_link_submenu_model_.AddItem(
+              IDC_OPEN_LINK_IN_PROFILE_FIRST + profile_index,
+              profile_info_cache.GetNameOfProfileAtIndex(profile_index));
+          AddIconToLastMenuItem(
+              profile_info_cache.GetAvatarIconOfProfileAtIndex(profile_index),
+              &profile_link_submenu_model_);
+        }
+        menu_model_.AddSubMenuWithStringId(
+            IDC_CONTENT_CONTEXT_OPENLINKINPROFILE,
+            IDS_CONTENT_CONTEXT_OPENLINKINPROFILES,
+            &profile_link_submenu_model_);
+      }
+    }
+    menu_model_.AddSeparator(ui::NORMAL_SEPARATOR);
     menu_model_.AddItemWithStringId(IDC_CONTENT_CONTEXT_SAVELINKAS,
                                     IDS_CONTENT_CONTEXT_SAVELINKAS);
   }
@@ -1131,6 +1224,11 @@ bool RenderViewContextMenu::IsCommandIdEnabled(int id) const {
     return true;
   }
 
+  if (id >= IDC_OPEN_LINK_IN_PROFILE_FIRST &&
+      id <= IDC_OPEN_LINK_IN_PROFILE_LAST) {
+    return params_.link_url.is_valid();
+  }
+
   IncognitoModePrefs::Availability incognito_avail =
       IncognitoModePrefs::GetAvailability(prefs);
   switch (id) {
@@ -1203,6 +1301,7 @@ bool RenderViewContextMenu::IsCommandIdEnabled(int id) const {
 
     case IDC_CONTENT_CONTEXT_OPENLINKNEWTAB:
     case IDC_CONTENT_CONTEXT_OPENLINKNEWWINDOW:
+    case IDC_CONTENT_CONTEXT_OPENLINKINPROFILE:
       return params_.link_url.is_valid();
 
     case IDC_CONTENT_CONTEXT_COPYLINKLOCATION:
@@ -1477,6 +1576,26 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) {
     return;
   }
 
+  if (id >= IDC_OPEN_LINK_IN_PROFILE_FIRST &&
+      id <= IDC_OPEN_LINK_IN_PROFILE_LAST) {
+    ProfileManager* profile_manager = g_browser_process->profile_manager();
+    const ProfileInfoCache& profile_info_cache =
+        profile_manager->GetProfileInfoCache();
+
+    base::FilePath profile_path = profile_info_cache.GetPathOfProfileAtIndex(
+        id - IDC_OPEN_LINK_IN_PROFILE_FIRST);
+    chrome::HostDesktopType desktop_type =
+        chrome::GetHostDesktopTypeForNativeView(
+            source_web_contents_->GetNativeView());
+
+    profiles::SwitchToProfile(
+        profile_path, desktop_type, false /* always_create */,

[Peter Kasting, 2015/10/13 18:59:00]

Nit: Omit the param comment here; it actually misled me into thinking you were
doing the opposite of what you're doing, and there aren't multiple booleans here
so there's no need to distinguish them.

[jochen (gone - plz use gerrit), 2015/10/13 19:50:22]

done

+        base::Bind(OnProfileCreated, desktop_type, params_.link_url,
+                   CreateReferrer(params_.link_url, params_)),
+        ProfileMetrics::CONTEXT_MENU);
+    return;
+  }
+
   switch (id) {
     case IDC_CONTENT_CONTEXT_OPENLINKNEWTAB: {
       Browser* browser =
@@ -1503,7 +1622,7 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) {
     case IDC_CONTENT_CONTEXT_SAVELINKAS: {
       RecordDownloadSource(DOWNLOAD_INITIATED_BY_CONTEXT_MENU);
       const GURL& url = params_.link_url;
-      content::Referrer referrer = CreateSaveAsReferrer(url, params_);
+      content::Referrer referrer = CreateReferrer(url, params_);
       DownloadManager* dlm =
           BrowserContext::GetDownloadManager(browser_context_);
       scoped_ptr<DownloadUrlParameters> dl_params(
@@ -1528,7 +1647,7 @@ void RenderViewContextMenu::ExecuteCommand(int id, int event_flags) {
       } else {
         RecordDownloadSource(DOWNLOAD_INITIATED_BY_CONTEXT_MENU);
         const GURL& url = params_.src_url;
-        content::Referrer referrer = CreateSaveAsReferrer(url, params_);
+        content::Referrer referrer = CreateReferrer(url, params_);
 
         std::string headers;
         DataReductionProxyChromeSettings* settings =