Commit 632966a4 authored by Robert Ogden's avatar Robert Ogden Committed by Commit Bot

HTTPS Server Previews: Use Previews State to trigger

This has two side effects:
* We no longer need to check that a navigation is in the main frame,
  but I did leave a DCHECK there anyways.
* The temp hack for ORIGINAL_REQUEST_URL is removed.

Bug: 898557, 864646
Change-Id: Ib767fd0b2e8d7675cf1b6e5ae7a4eec282378680
Reviewed-on: https://chromium-review.googlesource.com/c/1344551Reviewed-by: default avatarDoug Arnett <dougarnett@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: Robert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609913}
parent 7fa1fc03
...@@ -678,32 +678,6 @@ IN_PROC_BROWSER_TEST_F(PreviewsLitePageServerBrowserTest, ...@@ -678,32 +678,6 @@ IN_PROC_BROWSER_TEST_F(PreviewsLitePageServerBrowserTest,
false, 1); false, 1);
} }
{
// Verify a subframe navigation does not trigger a preview.
const base::string16 kSubframeTitle = base::ASCIIToUTF16("Subframe");
base::HistogramTester histogram_tester;
ui_test_utils::NavigateToURL(browser(), subframe_url());
// Navigate in the subframe and wait for it to finish. The waiting is
// accomplished by |ExecuteScriptAndExtractString| which waits for
// |window.domAutomationController.send| in the HTML page.
std::string result;
EXPECT_TRUE(ExecuteScriptAndExtractString(
GetWebContents()->GetMainFrame(),
"window.open(\"" + base_https_lite_page_url().spec() +
"\", \"subframe\")",
&result));
EXPECT_EQ(kSubframeTitle, base::ASCIIToUTF16(result));
histogram_tester.ExpectBucketCount(
"Previews.ServerLitePage.IneligibleReasons",
PreviewsLitePageNavigationThrottle::IneligibleReason::
kSubframeNavigation,
1);
histogram_tester.ExpectBucketCount("Previews.ServerLitePage.Triggered",
false, 2);
}
{ {
// Verify a preview is only shown on slow networks. // Verify a preview is only shown on slow networks.
base::HistogramTester histogram_tester; base::HistogramTester histogram_tester;
...@@ -734,15 +708,16 @@ IN_PROC_BROWSER_TEST_F(PreviewsLitePageServerBrowserTest, ...@@ -734,15 +708,16 @@ IN_PROC_BROWSER_TEST_F(PreviewsLitePageServerBrowserTest,
GetWebContents()->GetController().Reload(content::ReloadType::NORMAL, false); GetWebContents()->GetController().Reload(content::ReloadType::NORMAL, false);
VerifyPreviewLoaded(); VerifyPreviewLoaded();
}
base::HistogramTester histogram_tester; IN_PROC_BROWSER_TEST_F(PreviewsLitePageServerBrowserTest,
GetWebContents()->GetController().Reload( DISABLE_ON_WIN_MAC(LitePagePreviewsLoadOriginal)) {
content::ReloadType::ORIGINAL_REQUEST_URL, false); ui_test_utils::NavigateToURL(browser(), HttpsLitePageURL(kSuccess));
VerifyPreviewLoaded();
PreviewsUITabHelper::FromWebContents(GetWebContents())
->ReloadWithoutPreviews();
VerifyPreviewNotLoaded(); VerifyPreviewNotLoaded();
histogram_tester.ExpectBucketCount(
"Previews.ServerLitePage.IneligibleReasons",
PreviewsLitePageNavigationThrottle::IneligibleReason::kLoadOriginalReload,
1);
} }
IN_PROC_BROWSER_TEST_F(PreviewsLitePageServerBrowserTest, IN_PROC_BROWSER_TEST_F(PreviewsLitePageServerBrowserTest,
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "chrome/browser/previews/previews_lite_page_navigation_throttle.h" #include "chrome/browser/previews/previews_lite_page_navigation_throttle.h"
#include "chrome/browser/previews/previews_service.h" #include "chrome/browser/previews/previews_service.h"
#include "chrome/browser/previews/previews_service_factory.h" #include "chrome/browser/previews/previews_service_factory.h"
#include "chrome/browser/previews/previews_ui_tab_helper.h"
#include "chrome/browser/profiles/profile.h" #include "chrome/browser/profiles/profile.h"
#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_metrics.h"
#include "components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h" #include "components/data_reduction_proxy/core/browser/data_reduction_proxy_service.h"
...@@ -24,6 +25,7 @@ ...@@ -24,6 +25,7 @@
#include "components/data_use_measurement/core/data_use_user_data.h" #include "components/data_use_measurement/core/data_use_user_data.h"
#include "components/pref_registry/pref_registry_syncable.h" #include "components/pref_registry/pref_registry_syncable.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
#include "components/previews/content/previews_user_data.h"
#include "components/previews/core/previews_experiments.h" #include "components/previews/core/previews_experiments.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/navigation_entry.h" #include "content/public/browser/navigation_entry.h"
...@@ -184,19 +186,22 @@ PreviewsLitePageDecider::MaybeCreateThrottleFor( ...@@ -184,19 +186,22 @@ PreviewsLitePageDecider::MaybeCreateThrottleFor(
return nullptr; return nullptr;
DCHECK(!browser_context->IsOffTheRecord()); DCHECK(!browser_context->IsOffTheRecord());
PreviewsLitePageDecider* decider = PreviewsUITabHelper* tab_helper =
previews_service->previews_lite_page_decider(); PreviewsUITabHelper::FromWebContents(handle->GetWebContents());
DCHECK(decider); if (!tab_helper)
return nullptr;
// TODO(crbug/898557): Replace this logic with PreviewsState. previews::PreviewsUserData* previews_data =
bool drp_enabled = decider->drp_settings_->IsDataReductionProxyEnabled(); tab_helper->GetPreviewsUserData(handle);
bool preview_enabled = previews::params::ArePreviewsAllowed() && if (!previews_data)
previews::params::IsLitePageServerPreviewsEnabled(); return nullptr;
if (drp_enabled && preview_enabled) { if (previews_data->allowed_previews_state() &
return std::make_unique<PreviewsLitePageNavigationThrottle>(handle, content::LITE_PAGE_REDIRECT_ON) {
decider); return std::make_unique<PreviewsLitePageNavigationThrottle>(
handle, previews_service->previews_lite_page_decider());
} }
return nullptr; return nullptr;
} }
......
...@@ -184,6 +184,10 @@ PreviewsLitePageNavigationThrottle::~PreviewsLitePageNavigationThrottle() = ...@@ -184,6 +184,10 @@ PreviewsLitePageNavigationThrottle::~PreviewsLitePageNavigationThrottle() =
default; default;
bool PreviewsLitePageNavigationThrottle::IsEligibleForPreview() const { bool PreviewsLitePageNavigationThrottle::IsEligibleForPreview() const {
DCHECK(navigation_handle()->IsInMainFrame());
DCHECK_NE(navigation_handle()->GetReloadType(),
content::ReloadType::ORIGINAL_REQUEST_URL);
// Check if the parameters of the navigation are not eligible for the preview. // Check if the parameters of the navigation are not eligible for the preview.
std::vector<IneligibleReason> ineligible_reasons; std::vector<IneligibleReason> ineligible_reasons;
const GURL& url = navigation_handle()->GetURL(); const GURL& url = navigation_handle()->GetURL();
...@@ -193,9 +197,6 @@ bool PreviewsLitePageNavigationThrottle::IsEligibleForPreview() const { ...@@ -193,9 +197,6 @@ bool PreviewsLitePageNavigationThrottle::IsEligibleForPreview() const {
if (navigation_handle()->IsPost()) if (navigation_handle()->IsPost())
ineligible_reasons.push_back(IneligibleReason::kHttpPost); ineligible_reasons.push_back(IneligibleReason::kHttpPost);
if (!navigation_handle()->IsInMainFrame())
ineligible_reasons.push_back(IneligibleReason::kSubframeNavigation);
if (manager_->IsServerUnavailable()) if (manager_->IsServerUnavailable())
ineligible_reasons.push_back(IneligibleReason::kServerUnavailable); ineligible_reasons.push_back(IneligibleReason::kServerUnavailable);
...@@ -390,22 +391,6 @@ PreviewsLitePageNavigationThrottle::TriggerPreview() const { ...@@ -390,22 +391,6 @@ PreviewsLitePageNavigationThrottle::TriggerPreview() const {
content::NavigationThrottle::ThrottleCheckResult content::NavigationThrottle::ThrottleCheckResult
PreviewsLitePageNavigationThrottle::MaybeNavigateToPreview() const { PreviewsLitePageNavigationThrottle::MaybeNavigateToPreview() const {
// First check if the user is attempting to load the original page on a
// preview.
std::string original_url;
if (navigation_handle()->GetReloadType() ==
content::ReloadType::ORIGINAL_REQUEST_URL &&
previews::ExtractOriginalURLFromLitePageRedirectURL(
navigation_handle()->GetURL(), &original_url)) {
LoadAndBypass(navigation_handle()->GetWebContents(), manager_,
MakeOpenURLParams(navigation_handle(), GURL(original_url),
std::string()),
true);
UMA_HISTOGRAM_ENUMERATION("Previews.ServerLitePage.IneligibleReasons",
IneligibleReason::kLoadOriginalReload);
return content::NavigationThrottle::CANCEL;
}
const bool trigger = const bool trigger =
IsEligibleForPreview() && IsEligibleForPreview() &&
!manager_->CheckSingleBypass(navigation_handle()->GetURL().spec()); !manager_->CheckSingleBypass(navigation_handle()->GetURL().spec());
......
...@@ -29,8 +29,10 @@ ...@@ -29,8 +29,10 @@
#include "components/previews/content/previews_ui_service.h" #include "components/previews/content/previews_ui_service.h"
#include "components/previews/core/previews_experiments.h" #include "components/previews/core/previews_experiments.h"
#include "components/previews/core/previews_features.h" #include "components/previews/core/previews_features.h"
#include "components/previews/core/previews_lite_page_url_handler.h"
#include "content/public/browser/browser_context.h" #include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h" #include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/navigation_handle.h" #include "content/public/browser/navigation_handle.h"
#include "content/public/browser/render_frame_host.h" #include "content/public/browser/render_frame_host.h"
#include "content/public/browser/web_contents.h" #include "content/public/browser/web_contents.h"
...@@ -94,6 +96,22 @@ bool ShouldShowUIForPreviewsType(previews::PreviewsType type) { ...@@ -94,6 +96,22 @@ bool ShouldShowUIForPreviewsType(previews::PreviewsType type) {
return true; return true;
} }
void LoadOriginalForLitePageRedirect(content::WebContents* web_contents) {
std::string original_url;
bool extracted = previews::ExtractOriginalURLFromLitePageRedirectURL(
web_contents->GetController().GetLastCommittedEntry()->GetURL(),
&original_url);
ALLOW_UNUSED_LOCAL(extracted);
DCHECK(extracted);
content::OpenURLParams url_params(GURL(original_url), content::Referrer(),
WindowOpenDisposition::CURRENT_TAB,
ui::PAGE_TRANSITION_RELOAD,
false /* is_render_initiated */);
url_params.user_gesture = true;
url_params.started_from_context_menu = false;
web_contents->OpenURL(url_params);
}
} // namespace } // namespace
PreviewsUITabHelper::~PreviewsUITabHelper() { PreviewsUITabHelper::~PreviewsUITabHelper() {
...@@ -215,7 +233,6 @@ void PreviewsUITabHelper::ReloadWithoutPreviews( ...@@ -215,7 +233,6 @@ void PreviewsUITabHelper::ReloadWithoutPreviews(
std::move(on_dismiss_callback_).Run(true); std::move(on_dismiss_callback_).Run(true);
switch (previews_type) { switch (previews_type) {
case previews::PreviewsType::LITE_PAGE: case previews::PreviewsType::LITE_PAGE:
case previews::PreviewsType::LITE_PAGE_REDIRECT:
case previews::PreviewsType::OFFLINE: case previews::PreviewsType::OFFLINE:
case previews::PreviewsType::NOSCRIPT: case previews::PreviewsType::NOSCRIPT:
case previews::PreviewsType::RESOURCE_LOADING_HINTS: case previews::PreviewsType::RESOURCE_LOADING_HINTS:
...@@ -227,6 +244,9 @@ void PreviewsUITabHelper::ReloadWithoutPreviews( ...@@ -227,6 +244,9 @@ void PreviewsUITabHelper::ReloadWithoutPreviews(
case previews::PreviewsType::LOFI: case previews::PreviewsType::LOFI:
web_contents()->ReloadLoFiImages(); web_contents()->ReloadLoFiImages();
break; break;
case previews::PreviewsType::LITE_PAGE_REDIRECT:
LoadOriginalForLitePageRedirect(web_contents());
break;
case previews::PreviewsType::NONE: case previews::PreviewsType::NONE:
case previews::PreviewsType::UNSPECIFIED: case previews::PreviewsType::UNSPECIFIED:
case previews::PreviewsType::LAST: case previews::PreviewsType::LAST:
......
...@@ -17,6 +17,7 @@ ...@@ -17,6 +17,7 @@
#include "chrome/browser/loader/chrome_navigation_data.h" #include "chrome/browser/loader/chrome_navigation_data.h"
#include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h" #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings.h"
#include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h" #include "chrome/browser/net/spdyproxy/data_reduction_proxy_chrome_settings_factory.h"
#include "chrome/browser/previews/previews_lite_page_navigation_throttle.h"
#include "chrome/browser/previews/previews_ui_tab_helper.h" #include "chrome/browser/previews/previews_ui_tab_helper.h"
#include "chrome/test/base/chrome_render_view_host_test_harness.h" #include "chrome/test/base/chrome_render_view_host_test_harness.h"
#include "chrome/test/base/testing_profile.h" #include "chrome/test/base/testing_profile.h"
...@@ -34,6 +35,7 @@ ...@@ -34,6 +35,7 @@
#include "components/previews/content/previews_user_data.h" #include "components/previews/content/previews_user_data.h"
#include "components/previews/core/previews_features.h" #include "components/previews/core/previews_features.h"
#include "components/proxy_config/proxy_config_pref_names.h" #include "components/proxy_config/proxy_config_pref_names.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/navigation_handle.h" #include "content/public/browser/navigation_handle.h"
#include "content/public/common/previews_state.h" #include "content/public/common/previews_state.h"
#include "content/public/test/web_contents_tester.h" #include "content/public/test/web_contents_tester.h"
...@@ -406,3 +408,22 @@ TEST_F(PreviewsUITabHelperUnitTest, TestPreviewsCallbackCalledNonOptOut) { ...@@ -406,3 +408,22 @@ TEST_F(PreviewsUITabHelperUnitTest, TestPreviewsCallbackCalledNonOptOut) {
EXPECT_TRUE(on_dismiss_value); EXPECT_TRUE(on_dismiss_value);
EXPECT_FALSE(on_dismiss_value.value()); EXPECT_FALSE(on_dismiss_value.value());
} }
TEST_F(PreviewsUITabHelperUnitTest, TestReloadWithoutPreviewsLitePageRedirect) {
SimulateWillProcessResponse();
CallDidFinishNavigation();
base::RunLoop().RunUntilIdle();
GURL original_url("https://porgs.com");
GURL previews_url =
PreviewsLitePageNavigationThrottle::GetPreviewsURLForURL(original_url);
content::WebContentsTester::For(web_contents())
->NavigateAndCommit(previews_url);
PreviewsUITabHelper::FromWebContents(web_contents())
->ReloadWithoutPreviews(previews::PreviewsType::LITE_PAGE_REDIRECT);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(previews_url,
web_contents()->GetController().GetLastCommittedEntry()->GetURL());
}
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