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 8af019df57b9f36a23c1a2a2b10f6461053833fb..49ebb0ae32886d319d812a2a3dc6141438e68aba 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/spellchecker_submenu_observer.h"
 #include "chrome/browser/renderer_context_menu/spelling_menu_observer.h"
@@ -225,9 +229,10 @@ const struct UmaEnumCommandIdPair {
     {65, IDC_WRITING_DIRECTION_RTL},
     {66, IDC_CONTENT_CONTEXT_SHOW_ORIGINAL_IMAGE},
     {67, IDC_CONTENT_CONTEXT_FORCESAVEPASSWORD},
+    {68, 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.
-    {68, 0},  // Must be the last. Increment |enum_id| when new IDC was added.
+    {69, 0},  // Must be the last. Increment |enum_id| when new IDC was added.
 };
 
 // Collapses large ranges of ids before looking for UMA enum.
@@ -250,6 +255,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;
 }
 
@@ -371,6 +381,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())),
@@ -732,6 +743,66 @@ void RenderViewContextMenu::AppendLinkItems() {
       AppendProtocolHandlerSubMenu();
     }
 
+    // g_browser_process->profile_manager() is NULL during unit tests.

[Peter Kasting, 2015/06/15 23:45:15]

Is null, or may be null?  (i.e. Is it ever non-null?)

[jochen (gone - plz use gerrit), 2015/06/16 08:00:14]

it's always null during the context menu unit tests.

[Peter Kasting, 2015/06/16 08:20:24]

OK.  (Prefer "null" to "NULL", though.)

[jochen (gone - plz use gerrit), 2015/06/16 09:01:50]

done

+    if (g_browser_process->profile_manager() &&
+        GetProfile()->GetProfileType() == Profile::REGULAR_PROFILE) {
+      ProfileManager* profile_manager = g_browser_process->profile_manager();
+      ProfileInfoCache& profile_info_cache =

[Peter Kasting, 2015/06/15 23:45:15]

I believe that if you mark ProfileInfoCache::GetAvatarIconOfProfileAtIndex() as
a const function, this can become a const ref.  (I think that function is, in
fact, const, so this should be possible.)

[jochen (gone - plz use gerrit), 2015/06/16 08:00:13]

done

+          profile_manager->GetProfileInfoCache();
+
+      std::vector<Profile*> target_profiles;

[Peter Kasting, 2015/06/15 23:45:15]

Nit: Maybe above this block say something like "Find all regular profiles other
than the current one which have at least one open window."

[jochen (gone - plz use gerrit), 2015/06/16 08:00:13]

done

+      std::vector<Profile*> all_profiles = profile_manager->GetLoadedProfiles();
+      chrome::HostDesktopType desktop_type =
+          chrome::FindBrowserWithWebContents(source_web_contents_)

[Peter Kasting, 2015/06/15 23:45:15]

See null-check question I have about this in ExecuteCommand() below.

[jochen (gone - plz use gerrit), 2015/06/16 08:00:13]

done

+              ->host_desktop_type();
+      for (std::vector<Profile*>::iterator it = all_profiles.begin();
+           it != all_profiles.end(); ++it) {

[Peter Kasting, 2015/06/15 23:45:15]

Nit: Use range-based for

[jochen (gone - plz use gerrit), 2015/06/16 08:00:14]

done

+        if ((*it)->GetProfileType() != Profile::REGULAR_PROFILE)

[Peter Kasting, 2015/06/15 23:45:15]

Nit: Shorter:

        if ((profile->GetProfileType() == Profile::REGULAR_PROFILE) &&
            (profile != GetProfile() &&
            chrome::FindLastActiveWithProfile(profile, desktop_type))
          target_profiles.push_back(profile);

[jochen (gone - plz use gerrit), 2015/06/16 08:00:13]

done

+          continue;
+        if (*it == GetProfile())
+          continue;
+
+        // A loaded profile doesn't necessary have an open window associated
+        // with it.
+        if (!chrome::FindLastActiveWithProfile(*it, desktop_type))
+          continue;
+        target_profiles.push_back(*it);
+      }
+
+      if (target_profiles.size() == 1) {
+        Profile* profile = target_profiles[0];
+        size_t profile_index =
+            profile_info_cache.GetIndexOfProfileWithPath(profile->GetPath());
+        menu_model_.AddItem(
+            IDC_OPEN_LINK_IN_PROFILE_FIRST + profile_index,
+            l10n_util::GetStringFUTF16(
+                IDS_CONTENT_CONTEXT_OPENLINKINPROFILE,
+                profile_info_cache.GetNameOfProfileAtIndex(profile_index)));
+      } else if (target_profiles.size() > 1) {

[Peter Kasting, 2015/06/15 23:45:15]

Can this conditional ever fail?

[jochen (gone - plz use gerrit), 2015/06/16 08:00:13]

there could be no target_profile (since target_profile does not include the
current profile)

[Peter Kasting, 2015/06/16 08:20:24]

OK; how about "} else if (!target_profiles.empty()) {" then.

[jochen (gone - plz use gerrit), 2015/06/16 09:01:50]

done

+        for (std::vector<Profile*>::iterator it = target_profiles.begin();
+             it != target_profiles.end(); ++it) {

[Peter Kasting, 2015/06/15 23:45:15]

Nit: Use range-based for

[jochen (gone - plz use gerrit), 2015/06/16 08:00:14]

done

+          Profile* profile = *it;
+          size_t profile_index =
+              profile_info_cache.GetIndexOfProfileWithPath(profile->GetPath());
+          profile_link_submenu_model_.AddItem(
+              IDC_OPEN_LINK_IN_PROFILE_FIRST + profile_index,
+              profile_info_cache.GetNameOfProfileAtIndex(profile_index));
+          gfx::Image icon =
+              profile_info_cache.GetAvatarIconOfProfileAtIndex(profile_index);
+          int width = icon.Width();
+          int height = icon.Height();
+          gfx::CalculateFaviconTargetSize(&width, &height);
+          profile_link_submenu_model_.SetIcon(
+              profile_link_submenu_model_.GetItemCount() - 1,
+              profiles::GetSizedAvatarIcon(icon, true, width, height));
+        }
+        menu_model_.AddSubMenuWithStringId(
+            IDC_CONTENT_CONTEXT_OPENLINKINPROFILE,
+            IDS_CONTENT_CONTEXT_OPENLINKINPROFILES,
+            &profile_link_submenu_model_);
+      }
+    }
+
     menu_model_.AddItemWithStringId(IDC_CONTENT_CONTEXT_OPENLINKOFFTHERECORD,
                                     IDS_CONTENT_CONTEXT_OPENLINKOFFTHERECORD);
     menu_model_.AddItemWithStringId(IDC_CONTENT_CONTEXT_SAVELINKAS,
@@ -1094,6 +1165,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) {
@@ -1160,6 +1236,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:
@@ -1417,6 +1494,44 @@ 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();
+    ProfileInfoCache& profile_info_cache =
+        profile_manager->GetProfileInfoCache();
+
+    base::FilePath profile_path = profile_info_cache.GetPathOfProfileAtIndex(
+        id - IDC_OPEN_LINK_IN_PROFILE_FIRST);
+    Profile* profile = profile_manager->GetProfileByPath(profile_path);
+    DCHECK(profile);
+
+    chrome::HostDesktopType desktop_type =
+        chrome::FindBrowserWithWebContents(source_web_contents_)

[Peter Kasting, 2015/06/15 23:45:15]

The code below for IDC_CONTENT_CONTEXT_OPENLINKNEWTAB null-checks the result of
this call.  Does this code need to do that?  It seems like these need to be the
same, though I'm not precisely sure how this could return null in either case?

[jochen (gone - plz use gerrit), 2015/06/16 08:00:13]

fair point. I'm getting the desktop_top from the native view then

+            ->host_desktop_type();
+
+    Browser* browser = chrome::FindLastActiveWithProfile(profile, desktop_type);
+    chrome::NavigateParams nav_params(browser, params_.link_url,
+                                      ui::PAGE_TRANSITION_LINK);
+    nav_params.disposition = NEW_FOREGROUND_TAB;
+    nav_params.referrer = content::Referrer::SanitizeForRequest(
+        params_.link_url,
+        content::Referrer(GetDocumentURL(params_).GetAsReferrer(),
+                          params_.referrer_policy));
+    nav_params.window_action = chrome::NavigateParams::SHOW_WINDOW;
+    if (!browser) {
+      profiles::FindOrCreateNewWindowForProfile(
+          profile, chrome::startup::IS_NOT_PROCESS_STARTUP,
+          chrome::startup::IS_NOT_FIRST_RUN, desktop_type,
+          false /* always_create */);
+      nav_params.browser =
+          chrome::FindLastActiveWithProfile(profile, desktop_type);
+      DCHECK(nav_params.browser);
+      nav_params.disposition = CURRENT_TAB;
+    }
+    chrome::Navigate(&nav_params);
+    return;
+  }
+
   switch (id) {
     case IDC_CONTENT_CONTEXT_OPENLINKNEWTAB: {
       Browser* browser =