Skip to content

GitLab

T
tangled
Commit f96d7cfc authored by Alexey Baskakov's avatar Alexey Baskakov Committed by Commit Bot
Browse files
Options

WebApp: Move BookmarkAppDataRetriever to components/ 

Rename it to WebAppDataRetriever.

Modernize: Use BindOnce.

Bug: 875698
Change-Id: I4bf74a2966642f606f0ed602bc5bc7d72466e1da
Reviewed-on: https://chromium-review.googlesource.com/c/1256404Reviewed-by: default avatarGiovanni Ortuño Urquidi <ortuno@chromium.org>
Reviewed-by: default avatarDominick Ng <dominickn@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596525}
parent dcf0d8cf
Hide whitespace changes
Inline Side-by-side
Showing with 80 additions and 86 deletions
chrome/browser/web_applications/components/BUILD.gn
View file @ f96d7cfc
......@@ -7,6 +7,8 @@ source_set("components") {
"install_result_code.h",
"pending_app_manager.cc",
"pending_app_manager.h",
"web_app_data_retriever.cc",
"web_app_data_retriever.h",
"web_app_helpers.cc",
"web_app_helpers.h",
"web_app_icon_generator.cc",
......@@ -71,6 +73,7 @@ source_set("unit_tests") {
sources = [
"pending_app_manager_unittest.cc",
"web_app_data_retriever_unittest.cc",
"web_app_helpers_unittest.cc",
"web_app_icon_downloader_unittest.cc",
"web_app_icon_generator_unittest.cc",
......
chrome/browser/web_applications/extensions/bookmark_app_data_retriever.cc → chrome/browser/web_applications/components/web_app_data_retriever.cc
View file @ f96d7cfc
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h"
#include "chrome/browser/web_applications/components/web_app_data_retriever.h"
#include <memory>
#include <string>
......@@ -22,13 +22,13 @@
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
#include "third_party/skia/include/core/SkColor.h"
namespace extensions {
namespace web_app {
BookmarkAppDataRetriever::BookmarkAppDataRetriever() = default;
WebAppDataRetriever::WebAppDataRetriever() = default;
BookmarkAppDataRetriever::~BookmarkAppDataRetriever() = default;
WebAppDataRetriever::~WebAppDataRetriever() = default;
void BookmarkAppDataRetriever::GetWebApplicationInfo(
void WebAppDataRetriever::GetWebApplicationInfo(
content::WebContents* web_contents,
GetWebApplicationInfoCallback callback) {
// Concurrent calls are not allowed.
......@@ -52,20 +52,20 @@ void BookmarkAppDataRetriever::GetWebApplicationInfo(
// the WebContents or the RenderFrameHost are destroyed and the connection
// to ChromeRenderFrame is lost.
chrome_render_frame.set_connection_error_handler(
base::BindOnce(&BookmarkAppDataRetriever::OnGetWebApplicationInfoFailed,
base::BindOnce(&WebAppDataRetriever::OnGetWebApplicationInfoFailed,
weak_ptr_factory_.GetWeakPtr()));
// Bind the InterfacePtr into the callback so that it's kept alive
// until there's either a connection error or a response.
auto* web_app_info_proxy = chrome_render_frame.get();
web_app_info_proxy->GetWebApplicationInfo(base::Bind(
&BookmarkAppDataRetriever::OnGetWebApplicationInfo,
weak_ptr_factory_.GetWeakPtr(), base::Passed(&chrome_render_frame),
web_app_info_proxy->GetWebApplicationInfo(base::BindOnce(
&WebAppDataRetriever::OnGetWebApplicationInfo,
weak_ptr_factory_.GetWeakPtr(), std::move(chrome_render_frame),
web_contents, entry->GetUniqueID()));
}
void BookmarkAppDataRetriever::GetIcons(const GURL& app_url,
const std::vector<GURL>& icon_urls,
GetIconsCallback callback) {
void WebAppDataRetriever::GetIcons(const GURL& app_url,
const std::vector<GURL>& icon_urls,
GetIconsCallback callback) {
// TODO(crbug.com/864904): Download icons using |icon_urls|.
// Generate missing icons.
......@@ -105,7 +105,7 @@ void BookmarkAppDataRetriever::GetIcons(const GURL& app_url,
FROM_HERE, base::BindOnce(std::move(callback), std::move(icons)));
}
void BookmarkAppDataRetriever::OnGetWebApplicationInfo(
void WebAppDataRetriever::OnGetWebApplicationInfo(
chrome::mojom::ChromeRenderFrameAssociatedPtr chrome_render_frame,
content::WebContents* web_contents,
int last_committed_nav_entry_unique_id,
......@@ -129,8 +129,8 @@ void BookmarkAppDataRetriever::OnGetWebApplicationInfo(
std::move(get_web_app_info_callback_).Run(std::move(info));
}
void BookmarkAppDataRetriever::OnGetWebApplicationInfoFailed() {
void WebAppDataRetriever::OnGetWebApplicationInfoFailed() {
std::move(get_web_app_info_callback_).Run(nullptr);
}
} // namespace extensions
} // namespace web_app
chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h → chrome/browser/web_applications/components/web_app_data_retriever.h
View file @ f96d7cfc
......@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_BOOKMARK_APP_DATA_RETRIEVER_H_
#define CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_BOOKMARK_APP_DATA_RETRIEVER_H_
#ifndef CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_DATA_RETRIEVER_H_
#define CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_DATA_RETRIEVER_H_
#include <memory>
#include <vector>
......@@ -18,19 +18,19 @@ namespace content {
class WebContents;
}
namespace extensions {
namespace web_app {
// Class used by BookmarkAppInstallationTask to retrieve the necessary
// information to install an app. Should only be called from the UI thread.
class BookmarkAppDataRetriever {
class WebAppDataRetriever {
public:
using GetWebApplicationInfoCallback =
base::OnceCallback<void(std::unique_ptr<WebApplicationInfo>)>;
using GetIconsCallback =
base::OnceCallback<void(std::vector<WebApplicationInfo::IconInfo>)>;
BookmarkAppDataRetriever();
virtual ~BookmarkAppDataRetriever();
WebAppDataRetriever();
virtual ~WebAppDataRetriever();
// Runs |callback| with the result of retrieving the WebApplicationInfo from
// |web_contents|.
......@@ -55,11 +55,11 @@ class BookmarkAppDataRetriever {
// Saved callback from GetWebApplicationInfo().
GetWebApplicationInfoCallback get_web_app_info_callback_;
base::WeakPtrFactory<BookmarkAppDataRetriever> weak_ptr_factory_{this};
base::WeakPtrFactory<WebAppDataRetriever> weak_ptr_factory_{this};
DISALLOW_COPY_AND_ASSIGN(BookmarkAppDataRetriever);
DISALLOW_COPY_AND_ASSIGN(WebAppDataRetriever);
};
} // namespace extensions
} // namespace web_app
#endif // CHROME_BROWSER_WEB_APPLICATIONS_EXTENSIONS_BOOKMARK_APP_DATA_RETRIEVER_H_
#endif // CHROME_BROWSER_WEB_APPLICATIONS_COMPONENTS_WEB_APP_DATA_RETRIEVER_H_
chrome/browser/web_applications/extensions/bookmark_app_data_retriever_unittest.cc → chrome/browser/web_applications/components/web_app_data_retriever_unittest.cc
View file @ f96d7cfc
......@@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h"
#include "chrome/browser/web_applications/components/web_app_data_retriever.h"
#include <memory>
#include <utility>
......@@ -21,7 +21,7 @@
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
namespace extensions {
namespace web_app {
namespace {
......@@ -68,10 +68,10 @@ class FakeChromeRenderFrame
mojo::AssociatedBinding<chrome::mojom::ChromeRenderFrame> binding_{this};
};
class BookmarkAppDataRetrieverTest : public ChromeRenderViewHostTestHarness {
class WebAppDataRetrieverTest : public ChromeRenderViewHostTestHarness {
public:
BookmarkAppDataRetrieverTest() = default;
~BookmarkAppDataRetrieverTest() override = default;
WebAppDataRetrieverTest() = default;
~WebAppDataRetrieverTest() override = default;
void SetFakeChromeRenderFrame(
FakeChromeRenderFrame* fake_chrome_render_frame) {
......@@ -112,23 +112,22 @@ class BookmarkAppDataRetrieverTest : public ChromeRenderViewHostTestHarness {
base::Optional<std::unique_ptr<WebApplicationInfo>> web_app_info_;
std::vector<WebApplicationInfo::IconInfo> icons_;
DISALLOW_COPY_AND_ASSIGN(BookmarkAppDataRetrieverTest);
DISALLOW_COPY_AND_ASSIGN(WebAppDataRetrieverTest);
};
TEST_F(BookmarkAppDataRetrieverTest, GetWebApplicationInfo_NoEntry) {
TEST_F(WebAppDataRetrieverTest, GetWebApplicationInfo_NoEntry) {
base::RunLoop run_loop;
BookmarkAppDataRetriever retriever;
WebAppDataRetriever retriever;
retriever.GetWebApplicationInfo(
web_contents(),
base::BindOnce(
&BookmarkAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
base::BindOnce(&WebAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
run_loop.Run();
EXPECT_EQ(nullptr, web_app_info());
}
TEST_F(BookmarkAppDataRetrieverTest, GetWebApplicationInfo_AppUrlAbsent) {
TEST_F(WebAppDataRetrieverTest, GetWebApplicationInfo_AppUrlAbsent) {
web_contents_tester()->NavigateAndCommit(GURL(kFooUrl));
WebApplicationInfo original_web_app_info;
......@@ -138,12 +137,11 @@ TEST_F(BookmarkAppDataRetrieverTest, GetWebApplicationInfo_AppUrlAbsent) {
SetFakeChromeRenderFrame(&fake_chrome_render_frame);
base::RunLoop run_loop;
BookmarkAppDataRetriever retriever;
WebAppDataRetriever retriever;
retriever.GetWebApplicationInfo(
web_contents(),
base::BindOnce(
&BookmarkAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
base::BindOnce(&WebAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
run_loop.Run();
// If the WebApplicationInfo has no URL, we fallback to the last committed
......@@ -151,7 +149,7 @@ TEST_F(BookmarkAppDataRetrieverTest, GetWebApplicationInfo_AppUrlAbsent) {
EXPECT_EQ(GURL(kFooUrl), web_app_info()->app_url);
}
TEST_F(BookmarkAppDataRetrieverTest, GetWebApplicationInfo_AppUrlPresent) {
TEST_F(WebAppDataRetrieverTest, GetWebApplicationInfo_AppUrlPresent) {
web_contents_tester()->NavigateAndCommit(GURL(kFooUrl));
WebApplicationInfo original_web_app_info;
......@@ -161,19 +159,17 @@ TEST_F(BookmarkAppDataRetrieverTest, GetWebApplicationInfo_AppUrlPresent) {
SetFakeChromeRenderFrame(&fake_chrome_render_frame);
base::RunLoop run_loop;
BookmarkAppDataRetriever retriever;
WebAppDataRetriever retriever;
retriever.GetWebApplicationInfo(
web_contents(),
base::BindOnce(
&BookmarkAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
base::BindOnce(&WebAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
run_loop.Run();
EXPECT_EQ(original_web_app_info.app_url, web_app_info()->app_url);
}
TEST_F(BookmarkAppDataRetrieverTest,
GetWebApplicationInfo_TitleAbsentFromRenderer) {
TEST_F(WebAppDataRetrieverTest, GetWebApplicationInfo_TitleAbsentFromRenderer) {
web_contents_tester()->NavigateAndCommit(GURL(kFooUrl));
const auto web_contents_title = base::UTF8ToUTF16(kFooTitle);
......@@ -186,12 +182,11 @@ TEST_F(BookmarkAppDataRetrieverTest,
SetFakeChromeRenderFrame(&fake_chrome_render_frame);
base::RunLoop run_loop;
BookmarkAppDataRetriever retriever;
WebAppDataRetriever retriever;
retriever.GetWebApplicationInfo(
web_contents(),
base::BindOnce(
&BookmarkAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
base::BindOnce(&WebAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
run_loop.Run();
// If the WebApplicationInfo has no title, we fallback to the WebContents
......@@ -199,7 +194,7 @@ TEST_F(BookmarkAppDataRetrieverTest,
EXPECT_EQ(web_contents_title, web_app_info()->title);
}
TEST_F(BookmarkAppDataRetrieverTest,
TEST_F(WebAppDataRetrieverTest,
GetWebApplicationInfo_TitleAbsentFromWebContents) {
web_contents_tester()->NavigateAndCommit(GURL(kFooUrl));
......@@ -212,12 +207,11 @@ TEST_F(BookmarkAppDataRetrieverTest,
SetFakeChromeRenderFrame(&fake_chrome_render_frame);
base::RunLoop run_loop;
BookmarkAppDataRetriever retriever;
WebAppDataRetriever retriever;
retriever.GetWebApplicationInfo(
web_contents(),
base::BindOnce(
&BookmarkAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
base::BindOnce(&WebAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
run_loop.Run();
// If the WebApplicationInfo has no title and the WebContents has no title,
......@@ -226,51 +220,48 @@ TEST_F(BookmarkAppDataRetrieverTest,
web_app_info()->title);
}
TEST_F(BookmarkAppDataRetrieverTest,
GetWebApplicationInfo_WebContentsDestroyed) {
TEST_F(WebAppDataRetrieverTest, GetWebApplicationInfo_WebContentsDestroyed) {
web_contents_tester()->NavigateAndCommit(GURL(kFooUrl));
FakeChromeRenderFrame fake_chrome_render_frame{WebApplicationInfo()};
SetFakeChromeRenderFrame(&fake_chrome_render_frame);
base::RunLoop run_loop;
BookmarkAppDataRetriever retriever;
WebAppDataRetriever retriever;
retriever.GetWebApplicationInfo(
web_contents(),
base::BindOnce(
&BookmarkAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
base::BindOnce(&WebAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
DeleteContents();
run_loop.Run();
EXPECT_EQ(nullptr, web_app_info());
}
TEST_F(BookmarkAppDataRetrieverTest, GetWebApplicationInfo_FrameNavigated) {
TEST_F(WebAppDataRetrieverTest, GetWebApplicationInfo_FrameNavigated) {
web_contents_tester()->NavigateAndCommit(GURL(kFooUrl));
FakeChromeRenderFrame fake_chrome_render_frame{WebApplicationInfo()};
SetFakeChromeRenderFrame(&fake_chrome_render_frame);
base::RunLoop run_loop;
BookmarkAppDataRetriever retriever;
WebAppDataRetriever retriever;
retriever.GetWebApplicationInfo(
web_contents(),
base::BindOnce(
&BookmarkAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
base::BindOnce(&WebAppDataRetrieverTest::GetWebApplicationInfoCallback,
base::Unretained(this), run_loop.QuitClosure()));
web_contents_tester()->NavigateAndCommit(GURL(kFooUrl2));
run_loop.Run();
EXPECT_EQ(nullptr, web_app_info());
}
TEST_F(BookmarkAppDataRetrieverTest, GetIcons_NoIconsProvided) {
TEST_F(WebAppDataRetrieverTest, GetIcons_NoIconsProvided) {
base::RunLoop run_loop;
BookmarkAppDataRetriever retriever;
WebAppDataRetriever retriever;
retriever.GetIcons(
GURL(kFooUrl), std::vector<GURL>(),
base::BindOnce(&BookmarkAppDataRetrieverTest::GetIconsCallback,
base::BindOnce(&WebAppDataRetrieverTest::GetIconsCallback,
base::Unretained(this), run_loop.QuitClosure()));
run_loop.Run();
......@@ -291,4 +282,4 @@ TEST_F(BookmarkAppDataRetrieverTest, GetIcons_NoIconsProvided) {
}
}
} // namespace extensions
} // namespace web_app
chrome/browser/web_applications/extensions/BUILD.gn
View file @ f96d7cfc
......@@ -8,8 +8,6 @@ assert(enable_extensions)
source_set("extensions") {
sources = [
"bookmark_app_data_retriever.cc",
"bookmark_app_data_retriever.h",
"bookmark_app_installation_task.cc",
"bookmark_app_installation_task.h",
"bookmark_app_installer.cc",
......@@ -44,7 +42,6 @@ source_set("unit_tests") {
testonly = true
sources = [
"bookmark_app_data_retriever_unittest.cc",
"bookmark_app_installation_task_unittest.cc",
"bookmark_app_installer_unittest.cc",
"pending_bookmark_app_manager_unittest.cc",
......
chrome/browser/web_applications/extensions/bookmark_app_installation_task.cc
View file @ f96d7cfc
......@@ -13,7 +13,7 @@
#include "chrome/browser/favicon/favicon_utils.h"
#include "chrome/browser/installable/installable_manager.h"
#include "chrome/browser/ssl/security_state_tab_helper.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h"
#include "chrome/browser/web_applications/components/web_app_data_retriever.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_installer.h"
#include "chrome/common/web_application_info.h"
#include "content/public/browser/browser_thread.h"
......@@ -59,7 +59,7 @@ BookmarkAppInstallationTask::BookmarkAppInstallationTask(
: profile_(profile),
app_info_(std::move(app_info)),
helper_factory_(base::BindRepeating(&BookmarkAppHelperCreateWrapper)),
data_retriever_(std::make_unique<BookmarkAppDataRetriever>()),
data_retriever_(std::make_unique<web_app::WebAppDataRetriever>()),
installer_(std::make_unique<BookmarkAppInstaller>(profile)) {}
BookmarkAppInstallationTask::~BookmarkAppInstallationTask() = default;
......@@ -82,7 +82,7 @@ void BookmarkAppInstallationTask::SetBookmarkAppHelperFactoryForTesting(
}
void BookmarkAppInstallationTask::SetDataRetrieverForTesting(
std::unique_ptr<BookmarkAppDataRetriever> data_retriever) {
std::unique_ptr<web_app::WebAppDataRetriever> data_retriever) {
data_retriever_ = std::move(data_retriever);
}
......
chrome/browser/web_applications/extensions/bookmark_app_installation_task.h
View file @ f96d7cfc
......@@ -22,9 +22,12 @@ namespace content {
class WebContents;
}
namespace web_app {
class WebAppDataRetriever;
}
namespace extensions {
class BookmarkAppDataRetriever;
class BookmarkAppHelper;
class BookmarkAppInstaller;
class Extension;
......@@ -81,11 +84,11 @@ class BookmarkAppInstallationTask {
void SetBookmarkAppHelperFactoryForTesting(
BookmarkAppHelperFactory helper_factory);
void SetDataRetrieverForTesting(
std::unique_ptr<BookmarkAppDataRetriever> data_retriever);
std::unique_ptr<web_app::WebAppDataRetriever> data_retriever);
void SetInstallerForTesting(std::unique_ptr<BookmarkAppInstaller> installer);
protected:
BookmarkAppDataRetriever& data_retriever() { return *data_retriever_; }
web_app::WebAppDataRetriever& data_retriever() { return *data_retriever_; }
BookmarkAppInstaller& installer() { return *installer_; }
private:
......@@ -106,7 +109,7 @@ class BookmarkAppInstallationTask {
std::unique_ptr<BookmarkAppHelper> helper_;
BookmarkAppHelperFactory helper_factory_;
std::unique_ptr<BookmarkAppDataRetriever> data_retriever_;
std::unique_ptr<web_app::WebAppDataRetriever> data_retriever_;
std::unique_ptr<BookmarkAppInstaller> installer_;
base::WeakPtrFactory<BookmarkAppInstallationTask> weak_ptr_factory_{this};
......
chrome/browser/web_applications/extensions/bookmark_app_installation_task_unittest.cc
View file @ f96d7cfc
......@@ -21,7 +21,7 @@
#include "chrome/browser/extensions/extension_service.h"
#include "chrome/browser/extensions/test_extension_system.h"
#include "chrome/browser/installable/installable_data.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h"
#include "chrome/browser/web_applications/components/web_app_data_retriever.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_installer.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.h"
#include "chrome/common/web_application_info.h"
......@@ -86,9 +86,9 @@ class TestBookmarkAppHelper : public BookmarkAppHelper {
DISALLOW_COPY_AND_ASSIGN(TestBookmarkAppHelper);
};
// All BookmarkAppDataRetriever operations are async, so this class posts tasks
// All WebAppDataRetriever operations are async, so this class posts tasks
// when running callbacks to simulate async behavior in tests as well.
class TestDataRetriever : public BookmarkAppDataRetriever {
class TestDataRetriever : public web_app::WebAppDataRetriever {
public:
explicit TestDataRetriever(std::unique_ptr<WebApplicationInfo> web_app_info)
: web_app_info_(std::move(web_app_info)) {}
......
chrome/browser/web_applications/extensions/bookmark_app_shortcut_installation_task.cc
View file @ f96d7cfc
......@@ -11,7 +11,7 @@
#include "base/bind.h"
#include "base/callback.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_data_retriever.h"
#include "chrome/browser/web_applications/components/web_app_data_retriever.h"
#include "chrome/browser/web_applications/extensions/bookmark_app_installer.h"
#include "chrome/common/web_application_info.h"
#include "content/public/browser/browser_thread.h"
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment