desktop-pwas: Wrap WebApplicationInfo in a unique_ptr

WebApplicationInfo is not moveable so wwhen calling std::move() on it,
we make a copy. To avoid this, wrap WebApplicationInfo in a unique_ptr.

Bug: 864904
Change-Id: I3568101b8ee209786b18127e515b694729aa2ed2
Reviewed-on: https://chromium-review.googlesource.com/1154733Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Giovanni Ortuño Urquidi <ortuno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#578983}

chrome/browser/web_applications/extensions/bookmark_app_data_retriever.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h"
 
+#include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ -40,7 +41,7 @@ void BookmarkAppDataRetriever::GetWebApplicationInfo(
   if (!entry) {
     base::ThreadTaskRunnerHandle::Get()->PostTask(
         FROM_HERE,
-        base::BindOnce(std::move(get_web_app_info_callback_), base::nullopt));
+        base::BindOnce(std::move(get_web_app_info_callback_), nullptr));
     return;
   }
 
@@ -113,11 +114,11 @@ void BookmarkAppDataRetriever::OnGetWebApplicationInfo(
   content::NavigationEntry* entry =
       web_contents->GetController().GetLastCommittedEntry();
   if (!entry || last_committed_nav_entry_unique_id != entry->GetUniqueID()) {
-    std::move(get_web_app_info_callback_).Run(base::nullopt);
+    std::move(get_web_app_info_callback_).Run(nullptr);
     return;
   }
 
-  base::Optional<WebApplicationInfo> info(web_app_info);
+  auto info = std::make_unique<WebApplicationInfo>(web_app_info);
   if (info->app_url.is_empty())
     info->app_url = web_contents->GetLastCommittedURL();
 
@@ -130,7 +131,7 @@ void BookmarkAppDataRetriever::OnGetWebApplicationInfo(
 }
 
 void BookmarkAppDataRetriever::OnGetWebApplicationInfoFailed() {
-  std::move(get_web_app_info_callback_).Run(base::nullopt);
+  std::move(get_web_app_info_callback_).Run(nullptr);
 }
 
 }  // namespace extensions

chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h

@@ -5,12 +5,12 @@
 #ifndef CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_BOOKMARK_APP_DATA_RETRIEVER_H_
 #define CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_BOOKMARK_APP_DATA_RETRIEVER_H_
 
+#include <memory>
 #include <vector>
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
 #include "base/memory/weak_ptr.h"
-#include "base/optional.h"
 #include "chrome/common/chrome_render_frame.mojom.h"
 #include "chrome/common/web_application_info.h"
 
@@ -25,7 +25,7 @@ namespace extensions {
 class BookmarkAppDataRetriever {
  public:
   using GetWebApplicationInfoCallback =
-      base::OnceCallback<void(base::Optional<WebApplicationInfo>)>;
+      base::OnceCallback<void(std::unique_ptr<WebApplicationInfo>)>;
   using GetIconsCallback =
       base::OnceCallback<void(std::vector<WebApplicationInfo::IconInfo>)>;
 

chrome/browser/web_applications/extensions/bookmark_app_data_retriever_unittest.cc

@@ -4,9 +4,11 @@
 
 #include "chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h"
 
+#include <memory>
 #include <utility>
 #include <vector>
 
+#include "base/optional.h"
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/testing_profile.h"
@@ -85,8 +87,8 @@ class BookmarkAppDataRetrieverTest : public ChromeRenderViewHostTestHarness {
 
   void GetWebApplicationInfoCallback(
       base::OnceClosure quit_closure,
-      base::Optional<WebApplicationInfo> web_app_info) {
-    web_app_info_ = web_app_info;
+      std::unique_ptr<WebApplicationInfo> web_app_info) {
+    web_app_info_ = std::move(web_app_info);
     std::move(quit_closure).Run();
   }
 
@@ -101,14 +103,14 @@ class BookmarkAppDataRetrieverTest : public ChromeRenderViewHostTestHarness {
     return content::WebContentsTester::For(web_contents());
   }
 
-  const base::Optional<WebApplicationInfo>& web_app_info() {
+  const std::unique_ptr<WebApplicationInfo>& web_app_info() {
     return web_app_info_.value();
   }
 
   const std::vector<WebApplicationInfo::IconInfo>& icons() { return icons_; }
 
  private:
-  base::Optional<base::Optional<WebApplicationInfo>> web_app_info_;
+  base::Optional<std::unique_ptr<WebApplicationInfo>> web_app_info_;
   std::vector<WebApplicationInfo::IconInfo> icons_;
 
   DISALLOW_COPY_AND_ASSIGN(BookmarkAppDataRetrieverTest);
@@ -124,7 +126,7 @@ TEST_F(BookmarkAppDataRetrieverTest, GetWebApplicationInfo_NoEntry) {
           base::Unretained(this), run_loop.QuitClosure()));
   run_loop.Run();
 
-  EXPECT_EQ(base::nullopt, web_app_info());
+  EXPECT_EQ(nullptr, web_app_info());
 }
 
 TEST_F(BookmarkAppDataRetrieverTest, GetWebApplicationInfo_AppUrlAbsent) {
@@ -242,7 +244,7 @@ TEST_F(BookmarkAppDataRetrieverTest,
   DeleteContents();
   run_loop.Run();
 
-  EXPECT_EQ(base::nullopt, web_app_info());
+  EXPECT_EQ(nullptr, web_app_info());
 }
 
 TEST_F(BookmarkAppDataRetrieverTest, GetWebApplicationInfo_FrameNavigated) {
@@ -261,7 +263,7 @@ TEST_F(BookmarkAppDataRetrieverTest, GetWebApplicationInfo_FrameNavigated) {
   web_contents_tester()->NavigateAndCommit(GURL(kFooUrl2));
   run_loop.Run();
 
-  EXPECT_EQ(base::nullopt, web_app_info());
+  EXPECT_EQ(nullptr, web_app_info());
 }
 
 TEST_F(BookmarkAppDataRetrieverTest, GetIcons_NoIconsProvided) {

chrome/browser/web_applications/extensions/bookmark_app_installation_task_unittest.cc

@@ -64,7 +64,7 @@ class BookmarkAppInstallationTaskTest : public ChromeRenderViewHostTestHarness {
 // when running callbacks to simulate async behavior in tests as well.
 class TestDataRetriever : public BookmarkAppDataRetriever {
  public:
-  explicit TestDataRetriever(base::Optional<WebApplicationInfo> web_app_info)
+  explicit TestDataRetriever(std::unique_ptr<WebApplicationInfo> web_app_info)
       : web_app_info_(std::move(web_app_info)) {}
 
   ~TestDataRetriever() override = default;
@@ -73,7 +73,8 @@ class TestDataRetriever : public BookmarkAppDataRetriever {
                              GetWebApplicationInfoCallback callback) override {
     DCHECK(web_contents);
     base::ThreadTaskRunnerHandle::Get()->PostTask(
-        FROM_HERE, base::BindOnce(std::move(callback), web_app_info_));
+        FROM_HERE,
+        base::BindOnce(std::move(callback), std::move(web_app_info_)));
   }
 
   void GetIcons(const GURL& app_url,
@@ -85,7 +86,7 @@ class TestDataRetriever : public BookmarkAppDataRetriever {
   }
 
  private:
-  base::Optional<WebApplicationInfo> web_app_info_;
+  std::unique_ptr<WebApplicationInfo> web_app_info_;
 
   DISALLOW_COPY_AND_ASSIGN(TestDataRetriever);
 };
@@ -93,7 +94,7 @@ class TestDataRetriever : public BookmarkAppDataRetriever {
 TEST_F(BookmarkAppInstallationTaskTest, ShortcutFromContents_Delete) {
   auto installer = std::make_unique<BookmarkAppShortcutInstallationTask>();
   installer->SetDataRetrieverForTesting(
-      std::make_unique<TestDataRetriever>(base::nullopt));
+      std::make_unique<TestDataRetriever>(nullptr));
 
   base::RunLoop run_loop;
   installer->InstallFromWebContents(
@@ -109,7 +110,7 @@ TEST_F(BookmarkAppInstallationTaskTest, ShortcutFromContents_Delete) {
 TEST_F(BookmarkAppInstallationTaskTest, ShortcutFromContents_NoWebAppInfo) {
   auto installer = std::make_unique<BookmarkAppShortcutInstallationTask>();
   installer->SetDataRetrieverForTesting(
-      std::make_unique<TestDataRetriever>(base::nullopt));
+      std::make_unique<TestDataRetriever>(nullptr));
 
   base::RunLoop run_loop;
   installer->InstallFromWebContents(
@@ -130,7 +131,7 @@ TEST_F(BookmarkAppInstallationTaskTest, ShortcutFromContents_NoManifest) {
   info.app_url = GURL(kWebAppUrl);
   info.title = base::UTF8ToUTF16(kWebAppTitle);
   installer->SetDataRetrieverForTesting(std::make_unique<TestDataRetriever>(
-      base::make_optional(std::move(info))));
+      std::make_unique<WebApplicationInfo>(std::move(info))));
 
   base::RunLoop run_loop;
   installer->InstallFromWebContents(

chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h"
 
+#include <memory>
 #include <utility>
 #include <vector>
 
@@ -35,7 +36,7 @@ void BookmarkAppShortcutInstallationTask::InstallFromWebContents(
 
 void BookmarkAppShortcutInstallationTask::OnGetWebApplicationInfo(
     ResultCallback result_callback,
-    base::Optional<WebApplicationInfo> web_app_info) {
+    std::unique_ptr<WebApplicationInfo> web_app_info) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   if (!web_app_info) {
     std::move(result_callback).Run(Result::kGetWebApplicationInfoFailed);

chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h

@@ -5,11 +5,11 @@
 #ifndef CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_BOOKMARK_APP_SHORTCUT_INSTALLATION_TASK_H_
 #define CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_BOOKMARK_APP_SHORTCUT_INSTALLATION_TASK_H_
 
+#include <memory>
 #include <vector>
 
 #include "base/callback.h"
 #include "base/memory/weak_ptr.h"
-#include "base/optional.h"
 #include "chrome/browser/web_applications/extensions/bookmark_app_installation_task.h"
 #include "chrome/common/web_application_info.h"
 
@@ -30,8 +30,9 @@ class BookmarkAppShortcutInstallationTask : public BookmarkAppInstallationTask {
                               ResultCallback callback);
 
  private:
-  void OnGetWebApplicationInfo(ResultCallback result_callback,
-                               base::Optional<WebApplicationInfo> web_app_info);
+  void OnGetWebApplicationInfo(
+      ResultCallback result_callback,
+      std::unique_ptr<WebApplicationInfo> web_app_info);
   void OnGetIcons(ResultCallback result_callback,
                   std::vector<WebApplicationInfo::IconInfo> icons);