Commit a42c1a7b authored by Alexey Baskakov's avatar Alexey Baskakov Committed by Commit Bot

dpwa: Fix WebAppDataRetriever::GetWebApplicationInfo().

Sometimes last_committed_nav_entry_unique_id != entry->GetUniqueID()
in WebAppDataRetriever::OnGetWebApplicationInfo() which resulted in
kGetWebApplicationInfoFailed.
And kGetWebApplicationInfoFailed is a showstopper: generated icons.

WebAppDataRetriever::GetWebApplicationInfo() should return some initial
info to unblock CheckInstallabilityAndRetrieveManifest() stage.

This CL doesn't introduce significant behavior changes.
It returns non-nullptr info for the
`entry->GetUniqueID() != last_committed_nav_entry_unique_id`
edge case.

Bug: 1093251
Change-Id: Ia3bbcbdf229a3c74fc93f173892ebec404a2fc36
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2239727
Commit-Queue: Alexey Baskakov <loyso@chromium.org>
Reviewed-by: default avatarAlan Cutter <alancutter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#776897}
parent 6d23fb14
......@@ -49,6 +49,16 @@ void WebAppDataRetriever::GetWebApplicationInfo(
return;
}
// Makes a copy of WebContents fields right after Commit but before a mojo
// request to the renderer process.
default_web_application_info_ = std::make_unique<WebApplicationInfo>();
default_web_application_info_->app_url = web_contents->GetLastCommittedURL();
default_web_application_info_->title = web_contents->GetTitle();
if (default_web_application_info_->title.empty()) {
default_web_application_info_->title =
base::UTF8ToUTF16(default_web_application_info_->app_url.spec());
}
mojo::AssociatedRemote<chrome::mojom::ChromeRenderFrame> chrome_render_frame;
web_contents->GetMainFrame()->GetRemoteAssociatedInterfaces()->GetInterface(
&chrome_render_frame);
......@@ -135,24 +145,31 @@ void WebAppDataRetriever::OnGetWebApplicationInfo(
if (ShouldStopRetrieval())
return;
DCHECK(default_web_application_info_);
content::WebContents* contents = web_contents();
Observe(nullptr);
std::unique_ptr<WebApplicationInfo> info;
content::NavigationEntry* entry =
contents->GetController().GetLastCommittedEntry();
if (!entry || last_committed_nav_entry_unique_id != entry->GetUniqueID()) {
std::move(get_web_app_info_callback_).Run(nullptr);
return;
}
auto info = std::make_unique<WebApplicationInfo>(web_app_info);
if (info->app_url.is_empty())
info->app_url = contents->GetLastCommittedURL();
if (entry) {
if (entry->GetUniqueID() == last_committed_nav_entry_unique_id) {
info = std::make_unique<WebApplicationInfo>(web_app_info);
if (info->app_url.is_empty())
info->app_url = std::move(default_web_application_info_->app_url);
if (info->title.empty())
info->title = std::move(default_web_application_info_->title);
} else {
// WebContents navigation state changed during the call. Ignore the mojo
// request result. Use default initial info instead.
info = std::move(default_web_application_info_);
}
}
if (info->title.empty())
info->title = contents->GetTitle();
if (info->title.empty())
info->title = base::UTF8ToUTF16(info->app_url.spec());
default_web_application_info_.reset();
std::move(get_web_app_info_callback_).Run(std::move(info));
}
......@@ -190,6 +207,8 @@ void WebAppDataRetriever::CallCallbackOnError() {
Observe(nullptr);
DCHECK(ShouldStopRetrieval());
default_web_application_info_.reset();
// Call a callback as a tail call. The callback may destroy |this|.
if (get_web_app_info_callback_) {
std::move(get_web_app_info_callback_).Run(nullptr);
......
......@@ -88,7 +88,9 @@ class WebAppDataRetriever : content::WebContentsObserver {
void CallCallbackOnError();
bool ShouldStopRetrieval() const;
std::unique_ptr<WebApplicationInfo> default_web_application_info_;
GetWebApplicationInfoCallback get_web_app_info_callback_;
CheckInstallabilityCallback check_installability_callback_;
GetIconsCallback get_icons_callback_;
......
......@@ -350,6 +350,9 @@ TEST_F(WebAppDataRetrieverTest, GetIcons_WebContentsDestroyed) {
TEST_F(WebAppDataRetrieverTest, GetWebApplicationInfo_FrameNavigated) {
SetFakeChromeRenderFrame();
const auto web_contents_title = base::UTF8ToUTF16(kFooTitle);
web_contents_tester()->SetTitle(web_contents_title);
web_contents_tester()->NavigateAndCommit(FooUrl());
base::RunLoop run_loop;
......@@ -361,7 +364,8 @@ TEST_F(WebAppDataRetrieverTest, GetWebApplicationInfo_FrameNavigated) {
web_contents_tester()->NavigateAndCommit(FooUrl2());
run_loop.Run();
EXPECT_EQ(nullptr, web_app_info());
EXPECT_EQ(FooUrl(), web_app_info()->app_url);
EXPECT_EQ(web_contents_title, web_app_info()->title);
}
TEST_F(WebAppDataRetrieverTest, CheckInstallabilityAndRetrieveManifest) {
......
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