Commit 55b706b0 authored by Alexey Baskakov's avatar Alexey Baskakov Committed by Commit Bot

dpwas: Navigate to about:blank url to flush the WebContents state.

We should reset WebContents state between enqueued web app
installations.

WebContents::Stop() or WebContents::ClosePage() don't help: we need
full reset with completion callback.

Probably this CL will be merged into M-84.

TBR=treib@chromium.org

Bug: 1086778
Change-Id: If5ea81a62d5de1c0c95745f386dbd2ff3ae7fa6c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2237542Reviewed-by: default avatarAlexey Baskakov <loyso@chromium.org>
Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#777307}
parent c4d6b6f2
...@@ -202,7 +202,15 @@ IN_PROC_BROWSER_TEST_P(TwoClientWebAppsSyncTest, AppFieldsChangeDoesNotSync) { ...@@ -202,7 +202,15 @@ IN_PROC_BROWSER_TEST_P(TwoClientWebAppsSyncTest, AppFieldsChangeDoesNotSync) {
info_a.theme_color = SK_ColorBLUE; info_a.theme_color = SK_ColorBLUE;
AppId app_id_a = InstallApp(info_a, GetProfile(0)); AppId app_id_a = InstallApp(info_a, GetProfile(0));
// Wait for InstallBookmarkAppFromSync completion.
EXPECT_EQ(WebAppInstallObserver(GetProfile(1)).AwaitNextInstall(), app_id_a); EXPECT_EQ(WebAppInstallObserver(GetProfile(1)).AwaitNextInstall(), app_id_a);
if (!IsBookmarkAppsSync()) {
// Wait for InstallWebAppsAfterSync completion. TODO(crbug.com/1020037):
// Fix double installation if InstallWebAppsAfterSync happens second.
EXPECT_EQ(WebAppInstallObserver(GetProfile(1)).AwaitNextInstall(),
app_id_a);
}
EXPECT_EQ(base::UTF8ToUTF16(registrar1.GetAppShortName(app_id_a)), EXPECT_EQ(base::UTF8ToUTF16(registrar1.GetAppShortName(app_id_a)),
info_a.title); info_a.title);
if (IsBookmarkAppsSync()) { if (IsBookmarkAppsSync()) {
......
...@@ -81,6 +81,10 @@ class LoaderTask : public content::WebContentsObserver { ...@@ -81,6 +81,10 @@ class LoaderTask : public content::WebContentsObserver {
return; return;
} }
// Flush all DidFinishLoad events until about:blank loaded.
if (url_.IsAboutBlank() && !validated_url.IsAboutBlank())
return;
timer_.Stop(); timer_.Stop();
if (validated_url == content::kUnreachableWebDataURL) { if (validated_url == content::kUnreachableWebDataURL) {
...@@ -108,6 +112,10 @@ class LoaderTask : public content::WebContentsObserver { ...@@ -108,6 +112,10 @@ class LoaderTask : public content::WebContentsObserver {
return; return;
} }
// Flush all DidFailLoad events until about:blank loaded.
if (url_.IsAboutBlank())
return;
timer_.Stop(); timer_.Stop();
LOG(ERROR) << "Error loading " << url_ << " page failed to load."; LOG(ERROR) << "Error loading " << url_ << " page failed to load.";
......
...@@ -80,6 +80,11 @@ void TestWebAppUrlLoader::SetAboutBlankResultLoaded() { ...@@ -80,6 +80,11 @@ void TestWebAppUrlLoader::SetAboutBlankResultLoaded() {
WebAppUrlLoader::Result::kUrlLoaded); WebAppUrlLoader::Result::kUrlLoaded);
} }
void TestWebAppUrlLoader::AddAboutBlankResults(
const std::vector<Result>& results) {
AddNextLoadUrlResults(GURL("about:blank"), results);
}
TestWebAppUrlLoader::UrlResponses::UrlResponses() = default; TestWebAppUrlLoader::UrlResponses::UrlResponses() = default;
TestWebAppUrlLoader::UrlResponses::~UrlResponses() = default; TestWebAppUrlLoader::UrlResponses::~UrlResponses() = default;
......
...@@ -44,6 +44,7 @@ class TestWebAppUrlLoader : public WebAppUrlLoader { ...@@ -44,6 +44,7 @@ class TestWebAppUrlLoader : public WebAppUrlLoader {
// Sets the result for the next about:blank load to be ok. // Sets the result for the next about:blank load to be ok.
void SetAboutBlankResultLoaded(); void SetAboutBlankResultLoaded();
void AddAboutBlankResults(const std::vector<Result>& results);
private: private:
bool should_save_requests_ = false; bool should_save_requests_ = false;
......
...@@ -417,7 +417,12 @@ void WebAppInstallManager::OnQueuedTaskCompleted(WebAppInstallTask* task, ...@@ -417,7 +417,12 @@ void WebAppInstallManager::OnQueuedTaskCompleted(WebAppInstallTask* task,
web_contents_.reset(); web_contents_.reset();
web_contents_ready_ = false; web_contents_ready_ = false;
} else { } else {
MaybeStartQueuedTask(); // Load about:blank to clean up the renderer process.
url_loader_->LoadUrl(
GURL("about:blank"), web_contents_.get(),
WebAppUrlLoader::UrlComparison::kExact,
base::BindOnce(&WebAppInstallManager::OnWebContentsReady,
weak_ptr_factory_.GetWeakPtr()));
} }
} }
......
...@@ -156,8 +156,6 @@ class WebAppInstallManagerTest : public WebAppTest { ...@@ -156,8 +156,6 @@ class WebAppInstallManagerTest : public WebAppTest {
auto test_url_loader = std::make_unique<TestWebAppUrlLoader>(); auto test_url_loader = std::make_unique<TestWebAppUrlLoader>();
test_url_loader->SetAboutBlankResultLoaded();
test_url_loader_ = test_url_loader.get(); test_url_loader_ = test_url_loader.get();
install_manager_->SetUrlLoaderForTesting(std::move(test_url_loader)); install_manager_->SetUrlLoaderForTesting(std::move(test_url_loader));
...@@ -443,6 +441,9 @@ class WebAppInstallManagerTest : public WebAppTest { ...@@ -443,6 +441,9 @@ class WebAppInstallManagerTest : public WebAppTest {
TEST_F(WebAppInstallManagerTest, TEST_F(WebAppInstallManagerTest,
InstallWebAppsAfterSync_TwoConcurrentInstallsAreRunInOrder) { InstallWebAppsAfterSync_TwoConcurrentInstallsAreRunInOrder) {
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded,
WebAppUrlLoader::Result::kUrlLoaded});
const GURL url1{"https://example.com/path"}; const GURL url1{"https://example.com/path"};
const AppId app1_id = GenerateAppIdFromURL(url1); const AppId app1_id = GenerateAppIdFromURL(url1);
...@@ -625,6 +626,7 @@ TEST_F(WebAppInstallManagerTest, ...@@ -625,6 +626,7 @@ TEST_F(WebAppInstallManagerTest,
WebApp* web_app = controller().mutable_registrar().GetAppByIdMutable(app_id); WebApp* web_app = controller().mutable_registrar().GetAppByIdMutable(app_id);
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded});
url_loader().SetNextLoadUrlResult(launch_url, url_loader().SetNextLoadUrlResult(launch_url,
WebAppUrlLoader::Result::kUrlLoaded); WebAppUrlLoader::Result::kUrlLoaded);
...@@ -690,6 +692,7 @@ TEST_F(WebAppInstallManagerTest, InstallWebAppsAfterSync_Success) { ...@@ -690,6 +692,7 @@ TEST_F(WebAppInstallManagerTest, InstallWebAppsAfterSync_Success) {
WebApp* app = controller().mutable_registrar().GetAppByIdMutable( WebApp* app = controller().mutable_registrar().GetAppByIdMutable(
expected_app->app_id()); expected_app->app_id());
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded});
url_loader().SetNextLoadUrlResult(url, WebAppUrlLoader::Result::kUrlLoaded); url_loader().SetNextLoadUrlResult(url, WebAppUrlLoader::Result::kUrlLoaded);
install_manager().SetDataRetrieverFactoryForTesting( install_manager().SetDataRetrieverFactoryForTesting(
...@@ -755,6 +758,7 @@ TEST_F(WebAppInstallManagerTest, InstallWebAppsAfterSync_Fallback) { ...@@ -755,6 +758,7 @@ TEST_F(WebAppInstallManagerTest, InstallWebAppsAfterSync_Fallback) {
// Simulate if the web app publisher's website is down. // Simulate if the web app publisher's website is down.
url_loader().SetNextLoadUrlResult( url_loader().SetNextLoadUrlResult(
url, WebAppUrlLoader::Result::kFailedPageTookTooLong); url, WebAppUrlLoader::Result::kFailedPageTookTooLong);
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded});
install_manager().SetDataRetrieverFactoryForTesting( install_manager().SetDataRetrieverFactoryForTesting(
base::BindLambdaForTesting([]() { base::BindLambdaForTesting([]() {
...@@ -958,7 +962,10 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_LoadSuccess) { ...@@ -958,7 +962,10 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_LoadSuccess) {
const auto url1 = GURL("https://example.com/"); const auto url1 = GURL("https://example.com/");
const auto url2 = GURL("https://example.org/"); const auto url2 = GURL("https://example.org/");
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded,
WebAppUrlLoader::Result::kUrlLoaded});
url_loader().SetNextLoadUrlResult(url1, WebAppUrlLoader::Result::kUrlLoaded); url_loader().SetNextLoadUrlResult(url1, WebAppUrlLoader::Result::kUrlLoaded);
install_manager().SetDataRetrieverFactoryForTesting( install_manager().SetDataRetrieverFactoryForTesting(
base::BindLambdaForTesting([&]() { base::BindLambdaForTesting([&]() {
auto data_retriever = std::make_unique<TestDataRetriever>(); auto data_retriever = std::make_unique<TestDataRetriever>();
...@@ -974,7 +981,7 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_LoadSuccess) { ...@@ -974,7 +981,7 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_LoadSuccess) {
InstallBookmarkAppFromSync(url1, /*server_open_as_window=*/true); InstallBookmarkAppFromSync(url1, /*server_open_as_window=*/true);
url_loader().SetNextLoadUrlResult(url2, WebAppUrlLoader::Result::kUrlLoaded); url_loader().SetNextLoadUrlResult(url2, WebAppUrlLoader::Result::kUrlLoaded);
url_loader().SetAboutBlankResultLoaded();
install_manager().SetDataRetrieverFactoryForTesting( install_manager().SetDataRetrieverFactoryForTesting(
base::BindLambdaForTesting([&]() { base::BindLambdaForTesting([&]() {
auto data_retriever = std::make_unique<TestDataRetriever>(); auto data_retriever = std::make_unique<TestDataRetriever>();
...@@ -1008,6 +1015,10 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_LoadFailed) { ...@@ -1008,6 +1015,10 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_LoadFailed) {
const auto url1 = GURL("https://example.com/"); const auto url1 = GURL("https://example.com/");
const auto url2 = GURL("https://example.org/"); const auto url2 = GURL("https://example.org/");
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded,
WebAppUrlLoader::Result::kUrlLoaded});
// Induce a load failure: // Induce a load failure:
url_loader().SetNextLoadUrlResult( url_loader().SetNextLoadUrlResult(
url1, WebAppUrlLoader::Result::kRedirectedUrlLoaded); url1, WebAppUrlLoader::Result::kRedirectedUrlLoaded);
...@@ -1017,7 +1028,6 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_LoadFailed) { ...@@ -1017,7 +1028,6 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_LoadFailed) {
auto app_id1 = auto app_id1 =
InstallBookmarkAppFromSync(url1, /*server_open_as_window=*/false); InstallBookmarkAppFromSync(url1, /*server_open_as_window=*/false);
url_loader().SetAboutBlankResultLoaded();
auto app_id2 = auto app_id2 =
InstallBookmarkAppFromSync(url2, /*server_open_as_window=*/true); InstallBookmarkAppFromSync(url2, /*server_open_as_window=*/true);
...@@ -1041,6 +1051,8 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_TwoIcons_Success) { ...@@ -1041,6 +1051,8 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_TwoIcons_Success) {
const GURL url{"https://example.com/path"}; const GURL url{"https://example.com/path"};
const GURL icon1_url{"https://example.com/path/icon1.png"}; const GURL icon1_url{"https://example.com/path/icon1.png"};
const GURL icon2_url{"https://example.com/path/icon2.png"}; const GURL icon2_url{"https://example.com/path/icon2.png"};
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded});
url_loader().SetNextLoadUrlResult(url, WebAppUrlLoader::Result::kUrlLoaded); url_loader().SetNextLoadUrlResult(url, WebAppUrlLoader::Result::kUrlLoaded);
const AppId app_id = GenerateAppIdFromURL(url); const AppId app_id = GenerateAppIdFromURL(url);
...@@ -1113,6 +1125,8 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_TwoIcons_Fallback) { ...@@ -1113,6 +1125,8 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_TwoIcons_Fallback) {
const GURL url{"https://example.com/path"}; const GURL url{"https://example.com/path"};
const GURL icon1_url{"https://example.com/path/icon1.png"}; const GURL icon1_url{"https://example.com/path/icon1.png"};
const GURL icon2_url{"https://example.com/path/icon2.png"}; const GURL icon2_url{"https://example.com/path/icon2.png"};
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded});
// Induce a load failure: // Induce a load failure:
url_loader().SetNextLoadUrlResult( url_loader().SetNextLoadUrlResult(
url, WebAppUrlLoader::Result::kRedirectedUrlLoaded); url, WebAppUrlLoader::Result::kRedirectedUrlLoaded);
...@@ -1169,6 +1183,8 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_NoIcons) { ...@@ -1169,6 +1183,8 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_NoIcons) {
InitEmptyRegistrar(); InitEmptyRegistrar();
const GURL url{"https://example.com/path"}; const GURL url{"https://example.com/path"};
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded});
// Induce a load failure: // Induce a load failure:
url_loader().SetNextLoadUrlResult( url_loader().SetNextLoadUrlResult(
url, WebAppUrlLoader::Result::kRedirectedUrlLoaded); url, WebAppUrlLoader::Result::kRedirectedUrlLoaded);
...@@ -1201,6 +1217,8 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_ExpectAppIdFailed) { ...@@ -1201,6 +1217,8 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_ExpectAppIdFailed) {
InitEmptyRegistrar(); InitEmptyRegistrar();
const GURL old_url{"https://example.com/path"}; const GURL old_url{"https://example.com/path"};
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded});
url_loader().SetNextLoadUrlResult(old_url, url_loader().SetNextLoadUrlResult(old_url,
WebAppUrlLoader::Result::kUrlLoaded); WebAppUrlLoader::Result::kUrlLoaded);
...@@ -1237,6 +1255,8 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_QueueNewInstall) { ...@@ -1237,6 +1255,8 @@ TEST_F(WebAppInstallManagerTest, InstallBookmarkAppFromSync_QueueNewInstall) {
EXPECT_EQ(0u, install_manager().tasks_size_for_testing()); EXPECT_EQ(0u, install_manager().tasks_size_for_testing());
const GURL url{"https://example.com/path"}; const GURL url{"https://example.com/path"};
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded});
url_loader().SetNextLoadUrlResult(url, WebAppUrlLoader::Result::kUrlLoaded); url_loader().SetNextLoadUrlResult(url, WebAppUrlLoader::Result::kUrlLoaded);
UseDefaultDataRetriever(url); UseDefaultDataRetriever(url);
...@@ -1326,6 +1346,8 @@ TEST_F(WebAppInstallManagerTest, ...@@ -1326,6 +1346,8 @@ TEST_F(WebAppInstallManagerTest,
const GURL url{"https://example.com/path"}; const GURL url{"https://example.com/path"};
const AppId app_id = GenerateAppIdFromURL(url); const AppId app_id = GenerateAppIdFromURL(url);
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded,
WebAppUrlLoader::Result::kUrlLoaded});
// The web site url will be loaded twice in a sequence. // The web site url will be loaded twice in a sequence.
url_loader().AddNextLoadUrlResults(url, url_loader().AddNextLoadUrlResults(url,
{WebAppUrlLoader::Result::kUrlLoaded, {WebAppUrlLoader::Result::kUrlLoaded,
...@@ -1385,6 +1407,8 @@ TEST_F(WebAppInstallManagerTest, ...@@ -1385,6 +1407,8 @@ TEST_F(WebAppInstallManagerTest,
const GURL url{"https://example.com/path"}; const GURL url{"https://example.com/path"};
const AppId app_id = GenerateAppIdFromURL(url); const AppId app_id = GenerateAppIdFromURL(url);
url_loader().AddAboutBlankResults({WebAppUrlLoader::Result::kUrlLoaded,
WebAppUrlLoader::Result::kUrlLoaded});
// The web site url will be loaded twice in a sequence. The second load fails // The web site url will be loaded twice in a sequence. The second load fails
// (the web app). // (the web app).
url_loader().AddNextLoadUrlResults( url_loader().AddNextLoadUrlResults(
......
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