Make IntentPickerAppInfo move-only.

BUG=824598
TBR=sky@chromium.org

Change-Id: I09ac08a2042efffb8562b6a0d9764fdaa6231ff1
Reviewed-on: https://chromium-review.googlesource.com/991619
Commit-Queue: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Yusuke Sato <yusukes@chromium.org>
Cr-Commit-Position: refs/heads/master@{#548343}

chrome/browser/chromeos/apps/intent_helper/apps_navigation_throttle.cc

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/chromeos/apps/intent_helper/apps_navigation_throttle.h"
 
+#include <utility>
+
 #include "base/bind.h"
 #include "chrome/browser/chromeos/apps/intent_helper/apps_navigation_types.h"
 #include "chrome/browser/chromeos/arc/arc_util.h"
@@ -180,12 +182,12 @@ AppsNavigationThrottle::WillRedirectRequest() {
 void AppsNavigationThrottle::ShowIntentPickerBubbleForApps(
     const Browser* browser,
     const GURL& url,
-    const std::vector<IntentPickerAppInfo>& apps) {
+    std::vector<IntentPickerAppInfo> apps) {
   if (apps.empty())
     return;
 
   chrome::QueryAndDisplayArcApps(
-      browser, apps,
+      browser, std::move(apps),
       base::BindOnce(&AppsNavigationThrottle::OnIntentPickerClosed, url));
 }
 
@@ -199,7 +201,7 @@ void AppsNavigationThrottle::CancelNavigation() {
 
 void AppsNavigationThrottle::OnDeferredRequestProcessed(
     AppsNavigationAction action,
-    const std::vector<IntentPickerAppInfo>& apps) {
+    std::vector<IntentPickerAppInfo> apps) {
   if (action == AppsNavigationAction::CANCEL) {
     // We found a preferred ARC app to open; cancel the navigation and don't do
     // anything else.
@@ -210,7 +212,7 @@ void AppsNavigationThrottle::OnDeferredRequestProcessed(
   content::NavigationHandle* handle = navigation_handle();
   ShowIntentPickerBubbleForApps(
       chrome::FindBrowserWithWebContents(handle->GetWebContents()),
-      handle->GetURL(), apps);
+      handle->GetURL(), std::move(apps));
 
   // We are about to resume the navigation, which will destroy this object.
   ui_displayed_ = false;

chrome/browser/chromeos/apps/intent_helper/apps_navigation_throttle.h

@@ -72,7 +72,7 @@ class AppsNavigationThrottle : public content::NavigationThrottle {
   static void ShowIntentPickerBubbleForApps(
       const Browser* browser,
       const GURL& url,
-      const std::vector<IntentPickerAppInfo>& apps);
+      std::vector<IntentPickerAppInfo> apps);
   void CancelNavigation();
 
   content::NavigationThrottle::ThrottleCheckResult HandleRequest();
@@ -82,7 +82,7 @@ class AppsNavigationThrottle : public content::NavigationThrottle {
   // request that the navigation be completely cancelled (e.g. if a preferred
   // app has been opened).
   void OnDeferredRequestProcessed(AppsNavigationAction action,
-                                  const std::vector<IntentPickerAppInfo>& apps);
+                                  std::vector<IntentPickerAppInfo> apps);
 
   // A reference to the starting GURL.
   GURL starting_url_;

chrome/browser/chromeos/apps/intent_helper/apps_navigation_types.cc

@@ -6,13 +6,18 @@
 
 namespace chromeos {
 
-IntentPickerAppInfo::IntentPickerAppInfo(AppType app_type,
-                                         const gfx::Image& img,
-                                         const std::string& launch,
-                                         const std::string& name)
-    : type(app_type), icon(img), launch_name(launch), display_name(name) {}
+IntentPickerAppInfo::IntentPickerAppInfo(AppType type,
+                                         const gfx::Image& icon,
+                                         const std::string& launch_name,
+                                         const std::string& display_name)
+    : type(type),
+      icon(icon),
+      launch_name(launch_name),
+      display_name(display_name) {}
 
-IntentPickerAppInfo::IntentPickerAppInfo(const IntentPickerAppInfo& app_info) =
-    default;
+IntentPickerAppInfo::IntentPickerAppInfo(IntentPickerAppInfo&& other) = default;
+
+IntentPickerAppInfo& IntentPickerAppInfo::operator=(
+    IntentPickerAppInfo&& other) = default;
 
 }  // namespace chromeos

chrome/browser/chromeos/apps/intent_helper/apps_navigation_types.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/callback_forward.h"
+#include "base/macros.h"
 #include "ui/gfx/image/image.h"
 
 namespace chromeos {
@@ -49,14 +50,14 @@ enum class AppsNavigationAction {
 
 // Represents the data required to display an app in a picker to the user.
 struct IntentPickerAppInfo {
-  IntentPickerAppInfo(AppType app_type,
-                      const gfx::Image& img,
-                      const std::string& launch,
-                      const std::string& name);
+  IntentPickerAppInfo(AppType type,
+                      const gfx::Image& icon,
+                      const std::string& launch_name,
+                      const std::string& display_name);
 
-  // TODO(crbug.com/824598): make this type move-only to avoid unnecessary
-  // copies.
-  IntentPickerAppInfo(const IntentPickerAppInfo& app_info);
+  IntentPickerAppInfo(IntentPickerAppInfo&& other);
+
+  IntentPickerAppInfo& operator=(IntentPickerAppInfo&& other);
 
   // The type of app that this object represents.
   AppType type;
@@ -70,6 +71,8 @@ struct IntentPickerAppInfo {
 
   // The string shown to the user to identify this app in the intent picker.
   std::string display_name;
+
+  DISALLOW_COPY_AND_ASSIGN(IntentPickerAppInfo);
 };
 
 // Callback to allow app-platform-specific code to asynchronously signal what
@@ -77,12 +80,12 @@ struct IntentPickerAppInfo {
 // which can handle the navigation.
 using AppsNavigationCallback =
     base::OnceCallback<void(AppsNavigationAction action,
-                            const std::vector<IntentPickerAppInfo>& apps)>;
+                            std::vector<IntentPickerAppInfo> apps)>;
 
 // Callback to allow app-platform-specific code to asynchronously provide a list
 // of apps which can handle the navigation.
 using QueryAppsCallback =
-    base::OnceCallback<void(const std::vector<IntentPickerAppInfo>& apps)>;
+    base::OnceCallback<void(std::vector<IntentPickerAppInfo> apps)>;
 
 }  // namespace chromeos
 

chrome/browser/chromeos/arc/intent_helper/arc_external_protocol_dialog.cc

@@ -358,8 +358,8 @@ void OnAppIconsReceived(
   auto show_bubble_cb = base::Bind(ShowIntentPickerBubble());
   WebContents* web_contents =
       tab_util::GetWebContentsByID(render_process_host_id, routing_id);
-  show_bubble_cb.Run(nullptr /* anchor_view */, web_contents, app_info,
-                     !IsChromeAnAppCandidate(handlers),
+  show_bubble_cb.Run(nullptr /* anchor_view */, web_contents,
+                     std::move(app_info), !IsChromeAnAppCandidate(handlers),
                      base::Bind(OnIntentPickerClosed, render_process_host_id,
                                 routing_id, url, base::Passed(&handlers)));
 }

chrome/browser/chromeos/arc/intent_helper/arc_navigation_throttle.cc

@@ -256,7 +256,7 @@ void ArcNavigationThrottle::OnAppIconsReceived(
                           candidate->package_name, candidate->name);
   }
 
-  std::move(callback).Run(app_info);
+  std::move(callback).Run(std::move(app_info));
 }
 
 // static

chrome/browser/ui/browser_commands.cc

@@ -1230,11 +1230,11 @@ bool CanCreateBookmarkApp(const Browser* browser) {
 #endif  // BUILDFLAG(ENABLE_EXTENSIONS)
 
 #if defined(OS_CHROMEOS)
-void QueryAndDisplayArcApps(
-    const Browser* browser,
-    const std::vector<chromeos::IntentPickerAppInfo>& app_info,
-    IntentPickerResponse callback) {
-  browser->window()->ShowIntentPickerBubble(app_info, std::move(callback));
+void QueryAndDisplayArcApps(const Browser* browser,
+                            std::vector<chromeos::IntentPickerAppInfo> app_info,
+                            IntentPickerResponse callback) {
+  browser->window()->ShowIntentPickerBubble(std::move(app_info),
+                                            std::move(callback));
 }
 
 void SetIntentPickerViewVisibility(Browser* browser, bool visible) {

chrome/browser/ui/browser_commands.h

@@ -152,10 +152,9 @@ bool IsDebuggerAttachedToCurrentTab(Browser* browser);
 void CopyURL(Browser* browser);
 void OpenInChrome(Browser* browser);
 #if defined(OS_CHROMEOS)
-void QueryAndDisplayArcApps(
-    const Browser* browser,
-    const std::vector<chromeos::IntentPickerAppInfo>& app_info,
-    IntentPickerResponse callback);
+void QueryAndDisplayArcApps(const Browser* browser,
+                            std::vector<chromeos::IntentPickerAppInfo> app_info,
+                            IntentPickerResponse callback);
 void SetIntentPickerViewVisibility(Browser* browser, bool visible);
 #endif  // defined(OS_CHROMEOS)
 

chrome/browser/ui/browser_dialogs.h

@@ -332,7 +332,7 @@ using IntentPickerResponse =
 using BubbleShowPtr =
     views::Widget* (*)(views::View*,
                        content::WebContents*,
-                       const std::vector<chromeos::IntentPickerAppInfo>&,
+                       std::vector<chromeos::IntentPickerAppInfo>,
                        bool disable_display_in_chrome,
                        IntentPickerResponse);
 

chrome/browser/ui/browser_window.h

@@ -234,7 +234,7 @@ class BrowserWindow : public ui::BaseWindow {
   // display and |callback| gives access so we can redirect the user (if needed)
   // and store UMA metrics.
   virtual void ShowIntentPickerBubble(
-      const std::vector<chromeos::IntentPickerAppInfo>& app_info,
+      std::vector<chromeos::IntentPickerAppInfo> app_info,
       IntentPickerResponse callback) = 0;
   virtual void SetIntentPickerViewVisibility(bool visible) = 0;
 #endif  // defined(OS_CHROMEOS)

chrome/browser/ui/views/frame/browser_view.cc

@@ -1155,9 +1155,9 @@ void BrowserView::ShowUpdateChromeDialog() {
 
 #if defined(OS_CHROMEOS)
 void BrowserView::ShowIntentPickerBubble(
-    const std::vector<IntentPickerBubbleView::AppInfo>& app_info,
+    std::vector<IntentPickerBubbleView::AppInfo> app_info,
     IntentPickerResponse callback) {
-  toolbar_->ShowIntentPickerBubble(app_info, std::move(callback));
+  toolbar_->ShowIntentPickerBubble(std::move(app_info), std::move(callback));
 }
 
 void BrowserView::SetIntentPickerViewVisibility(bool visible) {

chrome/browser/ui/views/frame/browser_view.h

@@ -331,7 +331,7 @@ class BrowserView : public BrowserWindow,
   void ShowUpdateChromeDialog() override;
 #if defined(OS_CHROMEOS)
   void ShowIntentPickerBubble(
-      const std::vector<IntentPickerBubbleView::AppInfo>& app_info,
+      std::vector<IntentPickerBubbleView::AppInfo> app_info,
       IntentPickerResponse callback) override;
   void SetIntentPickerViewVisibility(bool visible) override;
 #endif  //  defined(OS_CHROMEOS)

chrome/browser/ui/views/intent_picker_bubble_view.cc

@@ -69,7 +69,7 @@ views::Separator* CreateHorizontalSeparator() {
 class IntentPickerLabelButton : public views::LabelButton {
  public:
   IntentPickerLabelButton(views::ButtonListener* listener,
-                          gfx::Image* icon,
+                          const gfx::Image* icon,
                           const std::string& launch_name,
                           const std::string& display_name)
       : LabelButton(listener,
@@ -112,7 +112,7 @@ IntentPickerBubbleView* IntentPickerBubbleView::intent_picker_bubble_ = nullptr;
 views::Widget* IntentPickerBubbleView::ShowBubble(
     views::View* anchor_view,
     content::WebContents* web_contents,
-    const std::vector<AppInfo>& app_info,
+    std::vector<AppInfo> app_info,
     bool disable_stay_in_chrome,
     IntentPickerResponse intent_picker_cb) {
   if (intent_picker_bubble_) {
@@ -129,9 +129,9 @@ views::Widget* IntentPickerBubbleView::ShowBubble(
     return nullptr;
   }
   BrowserView* browser_view = BrowserView::GetBrowserViewForBrowser(browser);
-  intent_picker_bubble_ =
-      new IntentPickerBubbleView(app_info, std::move(intent_picker_cb),
-                                 web_contents, disable_stay_in_chrome);
+  intent_picker_bubble_ = new IntentPickerBubbleView(
+      std::move(app_info), std::move(intent_picker_cb), web_contents,
+      disable_stay_in_chrome);
   intent_picker_bubble_->set_margins(gfx::Insets());
 
   if (anchor_view) {
@@ -163,13 +163,13 @@ views::Widget* IntentPickerBubbleView::ShowBubble(
 
 // static
 std::unique_ptr<IntentPickerBubbleView>
-IntentPickerBubbleView::CreateBubbleView(const std::vector<AppInfo>& app_info,
+IntentPickerBubbleView::CreateBubbleView(std::vector<AppInfo> app_info,
                                          bool disable_stay_in_chrome,
                                          IntentPickerResponse intent_picker_cb,
                                          content::WebContents* web_contents) {
-  std::unique_ptr<IntentPickerBubbleView> bubble(
-      new IntentPickerBubbleView(app_info, std::move(intent_picker_cb),
-                                 web_contents, disable_stay_in_chrome));
+  std::unique_ptr<IntentPickerBubbleView> bubble(new IntentPickerBubbleView(
+      std::move(app_info), std::move(intent_picker_cb), web_contents,
+      disable_stay_in_chrome));
   bubble->Init();
   return bubble;
 }
@@ -224,7 +224,7 @@ void IntentPickerBubbleView::Init() {
 
   size_t i = 0;
   size_t to_erase = app_info_.size();
-  for (AppInfo app_info : app_info_) {
+  for (const auto& app_info : app_info_) {
     if (arc::ArcIntentHelperBridge::IsIntentHelperPackage(
             app_info.launch_name)) {
       to_erase = i;
@@ -302,7 +302,7 @@ base::string16 IntentPickerBubbleView::GetDialogButtonLabel(
 }
 
 IntentPickerBubbleView::IntentPickerBubbleView(
-    const std::vector<AppInfo>& app_info,
+    std::vector<AppInfo> app_info,
     IntentPickerResponse intent_picker_cb,
     content::WebContents* web_contents,
     bool disable_stay_in_chrome)
@@ -312,7 +312,7 @@ IntentPickerBubbleView::IntentPickerBubbleView(
       intent_picker_cb_(std::move(intent_picker_cb)),
       selected_app_tag_(0),
       scroll_view_(nullptr),
-      app_info_(app_info),
+      app_info_(std::move(app_info)),
       remember_selection_checkbox_(nullptr),
       disable_stay_in_chrome_(disable_stay_in_chrome) {
   chrome::RecordDialogCreation(chrome::DialogIdentifier::INTENT_PICKER);

chrome/browser/ui/views/intent_picker_bubble_view.h

@@ -62,11 +62,11 @@ class IntentPickerBubbleView : public LocationBarBubbleDelegateView,
   ~IntentPickerBubbleView() override;
   static views::Widget* ShowBubble(views::View* anchor_view,
                                    content::WebContents* web_contents,
-                                   const std::vector<AppInfo>& app_info,
+                                   std::vector<AppInfo> app_info,
                                    bool disable_stay_in_chrome,
                                    IntentPickerResponse intent_picker_cb);
   static std::unique_ptr<IntentPickerBubbleView> CreateBubbleView(
-      const std::vector<AppInfo>& app_info,
+      std::vector<AppInfo> app_info,
       bool disable_stay_in_chrome,
       IntentPickerResponse intent_picker_cb,
       content::WebContents* web_contents);
@@ -100,7 +100,7 @@ class IntentPickerBubbleView : public LocationBarBubbleDelegateView,
   FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, ChromeNotInCandidates);
   FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, StayInChromeTest);
   FRIEND_TEST_ALL_PREFIXES(IntentPickerBubbleViewTest, WebContentsTiedToBubble);
-  IntentPickerBubbleView(const std::vector<AppInfo>& app_info,
+  IntentPickerBubbleView(std::vector<AppInfo> app_info,
                          IntentPickerResponse intent_picker_cb,
                          content::WebContents* web_contents,
                          bool disable_display_in_chrome);

chrome/browser/ui/views/intent_picker_bubble_view_unittest.cc

@@ -63,8 +63,17 @@ class IntentPickerBubbleViewTest : public BrowserWithTestWindowTest {
         OpenURLParams(url, Referrer(), WindowOpenDisposition::CURRENT_TAB,
                       ui::PAGE_TRANSITION_TYPED, false));
 
+    std::vector<AppInfo> app_info;
+
+    // AppInfo is move only. Manually create a new app_info array to pass into
+    // the bubble constructor.
+    for (const auto& app : app_info_) {
+      app_info.emplace_back(app.type, app.icon, app.launch_name,
+                            app.display_name);
+    }
+
     bubble_ = IntentPickerBubbleView::CreateBubbleView(
-        app_info_, disable_stay_in_chrome,
+        std::move(app_info), disable_stay_in_chrome,
         base::Bind(&IntentPickerBubbleViewTest::OnBubbleClosed,
                    base::Unretained(this)),
         web_contents);
@@ -118,7 +127,7 @@ TEST_F(IntentPickerBubbleViewTest, LabelsPtrVectorSize) {
   CreateBubbleView(true, false);
   size_t size = app_info_.size();
   size_t chrome_package_repetitions = 0;
-  for (AppInfo app_info : app_info_) {
+  for (const AppInfo& app_info : app_info_) {
     if (arc::ArcIntentHelperBridge::IsIntentHelperPackage(app_info.launch_name))
       ++chrome_package_repetitions;
   }

chrome/browser/ui/views/toolbar/toolbar_view.cc

@@ -293,7 +293,7 @@ bool ToolbarView::IsAppMenuFocused() {
 
 #if defined(OS_CHROMEOS)
 void ToolbarView::ShowIntentPickerBubble(
-    const std::vector<IntentPickerBubbleView::AppInfo>& app_info,
+    std::vector<IntentPickerBubbleView::AppInfo> app_info,
     IntentPickerResponse callback) {
   IntentPickerView* intent_picker_view = location_bar()->intent_picker_view();
   if (intent_picker_view) {
@@ -303,7 +303,7 @@ void ToolbarView::ShowIntentPickerBubble(
     }
 
     views::Widget* bubble_widget = IntentPickerBubbleView::ShowBubble(
-        intent_picker_view, GetWebContents(), app_info,
+        intent_picker_view, GetWebContents(), std::move(app_info),
         false /* disable_stay_in_chrome */, std::move(callback));
     if (bubble_widget && intent_picker_view)
       intent_picker_view->OnBubbleWidgetCreated(bubble_widget);

chrome/browser/ui/views/toolbar/toolbar_view.h

@@ -83,7 +83,7 @@ class ToolbarView : public views::AccessiblePaneView,
 
 #if defined(OS_CHROMEOS)
   void ShowIntentPickerBubble(
-      const std::vector<IntentPickerBubbleView::AppInfo>& app_info,
+      std::vector<IntentPickerBubbleView::AppInfo> app_info,
       IntentPickerResponse callback);
 #endif  // defined(OS_CHROMEOS)
 

chrome/test/base/test_browser_window.h

@@ -99,7 +99,7 @@ class TestBrowserWindow : public BrowserWindow {
   void ShowBookmarkBubble(const GURL& url, bool already_bookmarked) override {}
 #if defined(OS_CHROMEOS)
   void ShowIntentPickerBubble(
-      const std::vector<chromeos::IntentPickerAppInfo>& app_info,
+      std::vector<chromeos::IntentPickerAppInfo> app_info,
       IntentPickerResponse callback) override {}
   void SetIntentPickerViewVisibility(bool visible) override {}
 #endif  // defined(OS_CHROMEOS)