Commit 7e3808b6 authored by Robert Ogden's avatar Robert Ogden Committed by Commit Bot

Fix double-use of getting a new page id in HTTPS Server Previews

GetNewPageId() is only called in one place now, GetOrCreateSLPInfo, and
this value is used for all restarted navigations, persisted by the Info.

Bug: 952523
Change-Id: Id9cdc8aead427c6cfefabd195db10ed251c6f787
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1567101Reviewed-by: default avatarRyan Sturm <ryansturm@chromium.org>
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#650642}
parent 295ad6e6
......@@ -1455,6 +1455,15 @@ IN_PROC_BROWSER_TEST_P(PreviewsLitePageServerBrowserTest,
ui_test_utils::NavigateToURL(browser(), HttpsLitePageURL(kSuccess));
VerifyPreviewLoaded();
PreviewsUITabHelper* ui_tab_helper =
PreviewsUITabHelper::FromWebContents(GetWebContents());
previews::PreviewsUserData* previews_data =
ui_tab_helper->previews_user_data();
// Grab the page id and session now because they may change after the reload.
uint64_t expected_page_id = previews_data->server_lite_page_info()->page_id;
std::string expected_session_key =
previews_data->server_lite_page_info()->drp_session_key;
// Starting a new page load will send a pingback for the previous page load.
GetWebContents()->GetController().Reload(content::ReloadType::NORMAL, false);
pingback_client->WaitForPingback();
......@@ -1470,20 +1479,8 @@ IN_PROC_BROWSER_TEST_P(PreviewsLitePageServerBrowserTest,
if (GetParam())
return;
PreviewsUITabHelper* ui_tab_helper =
PreviewsUITabHelper::FromWebContents(GetWebContents());
previews::PreviewsUserData* previews_data =
ui_tab_helper->previews_user_data();
EXPECT_EQ(data->session_key(),
previews_data->server_lite_page_info()->drp_session_key);
// TODO(crbug.com/952523): The page id is being incremented for every
// restarted navigation. Fix and remove the early return.
return;
EXPECT_EQ(data->page_id().value(),
previews_data->server_lite_page_info()->page_id);
EXPECT_EQ(data->session_key(), expected_session_key);
EXPECT_EQ(data->page_id().value(), expected_page_id);
}
class PreviewsLitePageServerTimeoutBrowserTest
......
......@@ -116,26 +116,27 @@ class PreviewsWebContentsLifetimeHelper
// restart. Note: This could be a navigation to the litepages server, or to
// the original URL.
if (restarted_navigation_url_ == handle->GetURL()) {
// Get a new page id.
PreviewsService* previews_service = PreviewsServiceFactory::GetForProfile(
Profile::FromBrowserContext(web_contents()->GetBrowserContext()));
PreviewsLitePageNavigationThrottleManager* manager =
previews_service->previews_lite_page_decider();
uint64_t page_id = manager->GeneratePageID();
if (!info_) {
// Create a new info_ if needed. This will use the previous page_id,
// which is desired.
PreviewsService* previews_service =
PreviewsServiceFactory::GetForProfile(Profile::FromBrowserContext(
web_contents()->GetBrowserContext()));
PreviewsLitePageNavigationThrottleManager* manager =
previews_service->previews_lite_page_decider();
info_ =
PreviewsLitePageNavigationThrottle::GetOrCreateServerLitePageInfo(
handle, manager)
->Clone();
}
// Create a new PreviewsUserData if needed.
PreviewsUITabHelper* ui_tab_helper =
PreviewsUITabHelper::FromWebContents(web_contents());
previews::PreviewsUserData* previews_data =
ui_tab_helper->CreatePreviewsUserDataForNavigationHandle(handle,
page_id);
// Set the lite page state on the user data.
if (!info_) {
info_ =
std::make_unique<previews::PreviewsUserData::ServerLitePageInfo>();
info_->original_navigation_start = handle->NavigationStart();
}
ui_tab_helper->CreatePreviewsUserDataForNavigationHandle(
handle, info_->page_id);
previews_data->set_server_lite_page_info(std::move(info_));
// Reset member state.
......
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