Commit b4cf0af0 authored by Robert Ogden's avatar Robert Ogden Committed by Commit Bot

Safely set Server Lite Page Status during redirects and failures

This is causing infrequent crashes in the Canary/Dev experiment. See
bug. This is due to the WebLite client redirect issue that caused Issue
913639, but could in theory be caused by any navigation to the litepages
domain that is not started by our NavThrottle.

Bug: 919049
Change-Id: Ic309d9d4f63c7b3d3bc8de742ef58213042eedbe
Reviewed-on: https://chromium-review.googlesource.com/c/1396322Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620020}
parent 47803c37
...@@ -541,7 +541,7 @@ PreviewsLitePageNavigationThrottle::WillRedirectRequest() { ...@@ -541,7 +541,7 @@ PreviewsLitePageNavigationThrottle::WillRedirectRequest() {
// it is pointing towards the original page, it is considered a bypass. // it is pointing towards the original page, it is considered a bypass.
// Otherwise it is just a forwarded bypass. // Otherwise it is just a forwarded bypass.
if (GURL(original_url) == navigation_handle()->GetURL()) { if (GURL(original_url) == navigation_handle()->GetURL()) {
GetServerLitePageInfo()->status = previews::ServerLitePageStatus::kBypass; SetServerLitePageInfoStatus(previews::ServerLitePageStatus::kBypass);
manager_->AddSingleBypass(navigation_handle()->GetURL().spec()); manager_->AddSingleBypass(navigation_handle()->GetURL().spec());
UMA_HISTOGRAM_MEDIUM_TIMES( UMA_HISTOGRAM_MEDIUM_TIMES(
"Previews.ServerLitePage.HttpOnlyFallbackPenalty", "Previews.ServerLitePage.HttpOnlyFallbackPenalty",
...@@ -574,7 +574,7 @@ PreviewsLitePageNavigationThrottle::WillRedirectRequest() { ...@@ -574,7 +574,7 @@ PreviewsLitePageNavigationThrottle::WillRedirectRequest() {
// Otherwise fall out of this if and potentially trigger again. // Otherwise fall out of this if and potentially trigger again.
UMA_HISTOGRAM_ENUMERATION("Previews.ServerLitePage.ServerResponse", UMA_HISTOGRAM_ENUMERATION("Previews.ServerLitePage.ServerResponse",
ServerResponse::kRedirect); ServerResponse::kRedirect);
GetServerLitePageInfo()->status = previews::ServerLitePageStatus::kRedirect; SetServerLitePageInfoStatus(previews::ServerLitePageStatus::kRedirect);
} }
return MaybeNavigateToPreview(); return MaybeNavigateToPreview();
...@@ -590,7 +590,7 @@ PreviewsLitePageNavigationThrottle::WillFailRequest() { ...@@ -590,7 +590,7 @@ PreviewsLitePageNavigationThrottle::WillFailRequest() {
UMA_HISTOGRAM_ENUMERATION("Previews.ServerLitePage.ServerResponse", UMA_HISTOGRAM_ENUMERATION("Previews.ServerLitePage.ServerResponse",
ServerResponse::kFailed); ServerResponse::kFailed);
GetServerLitePageInfo()->status = previews::ServerLitePageStatus::kFailure; SetServerLitePageInfoStatus(previews::ServerLitePageStatus::kFailure);
// The Preview was triggered but there was some irrecoverable issue (like // The Preview was triggered but there was some irrecoverable issue (like
// there is no network connection). Load the original page and let it go // there is no network connection). Load the original page and let it go
...@@ -598,7 +598,8 @@ PreviewsLitePageNavigationThrottle::WillFailRequest() { ...@@ -598,7 +598,8 @@ PreviewsLitePageNavigationThrottle::WillFailRequest() {
LoadAndBypass( LoadAndBypass(
navigation_handle()->GetWebContents(), manager_, navigation_handle()->GetWebContents(), manager_,
MakeOpenURLParams(navigation_handle(), GURL(original_url), std::string()), MakeOpenURLParams(navigation_handle(), GURL(original_url), std::string()),
GetServerLitePageInfo()->Clone(), true); GetServerLitePageInfo() ? GetServerLitePageInfo()->Clone() : nullptr,
true);
return content::NavigationThrottle::CANCEL; return content::NavigationThrottle::CANCEL;
} }
...@@ -690,6 +691,15 @@ PreviewsLitePageNavigationThrottle::GetServerLitePageInfo() const { ...@@ -690,6 +691,15 @@ PreviewsLitePageNavigationThrottle::GetServerLitePageInfo() const {
return previews_data->server_lite_page_info(); return previews_data->server_lite_page_info();
} }
void PreviewsLitePageNavigationThrottle::SetServerLitePageInfoStatus(
previews::ServerLitePageStatus status) {
previews::PreviewsUserData::ServerLitePageInfo* info =
GetServerLitePageInfo();
if (!info)
return;
info->status = status;
}
previews::PreviewsUserData::ServerLitePageInfo* previews::PreviewsUserData::ServerLitePageInfo*
PreviewsLitePageNavigationThrottle::GetOrCreateServerLitePageInfo() const { PreviewsLitePageNavigationThrottle::GetOrCreateServerLitePageInfo() const {
PreviewsUITabHelper* ui_tab_helper = PreviewsUITabHelper::FromWebContents( PreviewsUITabHelper* ui_tab_helper = PreviewsUITabHelper::FromWebContents(
......
...@@ -142,6 +142,10 @@ class PreviewsLitePageNavigationThrottle : public content::NavigationThrottle { ...@@ -142,6 +142,10 @@ class PreviewsLitePageNavigationThrottle : public content::NavigationThrottle {
// navigation, if there is one. If not, returns nullptr. // navigation, if there is one. If not, returns nullptr.
previews::PreviewsUserData::ServerLitePageInfo* GetServerLitePageInfo() const; previews::PreviewsUserData::ServerLitePageInfo* GetServerLitePageInfo() const;
// Safely sets the status of the ServerLitePageInfo struct from an existing
// attempted lite page navigation, if there is one. If not, does nothing.
void SetServerLitePageInfoStatus(previews::ServerLitePageStatus status);
// Gets the ServerLitePageInfo struct from an existing attempted lite page // Gets the ServerLitePageInfo struct from an existing attempted lite page
// navigation, if there is one. If not, returns a new ServerLitePageInfo // navigation, if there is one. If not, returns a new ServerLitePageInfo
// initialized with metadata from navigation_handle() and |this| that is owned // initialized with metadata from navigation_handle() and |this| that is owned
......
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