Commit 9e6addde authored by Ryan Sturm's avatar Ryan Sturm Committed by Commit Bot

Timing out the HTTPS Previews URLLoader impl

When the request has been on the network for 30 seconds, the HTTPS
Preview URLLoader implementation should fallback to default behavior for
the URL hop.

Bug: 935771
Change-Id: Id76e8d116ef32d86bd7c311022723f93a6fb4c04
Reviewed-on: https://chromium-review.googlesource.com/c/1488117
Commit-Queue: Ryan Sturm <ryansturm@chromium.org>
Reviewed-by: default avatarRobert Ogden <robertogden@chromium.org>
Cr-Commit-Position: refs/heads/master@{#635733}
parent 665a7f07
...@@ -1341,8 +1341,6 @@ INSTANTIATE_TEST_SUITE_P(URLLoaderImplementation, ...@@ -1341,8 +1341,6 @@ INSTANTIATE_TEST_SUITE_P(URLLoaderImplementation,
IN_PROC_BROWSER_TEST_P(PreviewsLitePageServerTimeoutBrowserTest, IN_PROC_BROWSER_TEST_P(PreviewsLitePageServerTimeoutBrowserTest,
DISABLE_ON_WIN_MAC(LitePagePreviewsTimeout)) { DISABLE_ON_WIN_MAC(LitePagePreviewsTimeout)) {
if (GetParam())
return;
{ {
// Ensure that a hung previews navigation doesn't wind up at the previews // Ensure that a hung previews navigation doesn't wind up at the previews
// server. // server.
......
...@@ -9,6 +9,7 @@ ...@@ -9,6 +9,7 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/bind_helpers.h" #include "base/bind_helpers.h"
#include "base/location.h"
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "base/metrics/histogram_macros.h" #include "base/metrics/histogram_macros.h"
#include "base/rand_util.h" #include "base/rand_util.h"
...@@ -153,10 +154,28 @@ void PreviewsLitePageServingURLLoader::StartNetworkRequest( ...@@ -153,10 +154,28 @@ void PreviewsLitePageServingURLLoader::StartNetworkRequest(
mojo::MakeRequest(&network_url_loader_), frame_tree_node_id_, 0, mojo::MakeRequest(&network_url_loader_), frame_tree_node_id_, 0,
network::mojom::kURLLoadOptionNone, request, std::move(client), network::mojom::kURLLoadOptionNone, request, std::move(client),
net::MutableNetworkTrafficAnnotationTag(kPreviewsTrafficAnnotation)); net::MutableNetworkTrafficAnnotationTag(kPreviewsTrafficAnnotation));
timeout_timer_.Start(
FROM_HERE, previews::params::LitePagePreviewsNavigationTimeoutDuration(),
base::BindOnce(&PreviewsLitePageServingURLLoader::Timeout,
weak_ptr_factory_.GetWeakPtr()));
} }
PreviewsLitePageServingURLLoader::~PreviewsLitePageServingURLLoader() = default; PreviewsLitePageServingURLLoader::~PreviewsLitePageServingURLLoader() = default;
void PreviewsLitePageServingURLLoader::Timeout() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// If the result is already determined, don't do anything.
if (result_callback_.is_null())
return;
UMA_HISTOGRAM_ENUMERATION(
"Previews.ServerLitePage.ServerResponse",
PreviewsLitePageNavigationThrottle::ServerResponse::kTimeout);
Fallback();
}
void PreviewsLitePageServingURLLoader::Fallback() { void PreviewsLitePageServingURLLoader::Fallback() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
std::move(result_callback_) std::move(result_callback_)
......
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/optional.h" #include "base/optional.h"
#include "base/sequence_checker.h" #include "base/sequence_checker.h"
#include "base/timer/timer.h"
#include "content/public/browser/url_loader_request_interceptor.h" #include "content/public/browser/url_loader_request_interceptor.h"
#include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/system/data_pipe.h" #include "mojo/public/cpp/system/data_pipe.h"
...@@ -93,6 +94,12 @@ class PreviewsLitePageServingURLLoader ...@@ -93,6 +94,12 @@ class PreviewsLitePageServingURLLoader
// Calls |result_callback_| with kFallback and cleans up. // Calls |result_callback_| with kFallback and cleans up.
void Fallback(); void Fallback();
// Calls timeout if the result was not previously determined (i.e., the
// response has not started yet). This method is called
// |LitePagePreviewsNavigationTimeoutDuration()| after the network request has
// started.
void Timeout();
// Sets up mojo forwarding to the navigation path. Resumes // Sets up mojo forwarding to the navigation path. Resumes
// |network_url_loader_| calls. Serves the start of the response to the // |network_url_loader_| calls. Serves the start of the response to the
// navigation path. // navigation path.
...@@ -121,6 +128,9 @@ class PreviewsLitePageServingURLLoader ...@@ -121,6 +128,9 @@ class PreviewsLitePageServingURLLoader
// The previews URL that is being requested. // The previews URL that is being requested.
GURL previews_url_; GURL previews_url_;
// The timer that triggers a timeout when the request takes too long.
base::OneShotTimer timeout_timer_;
// Forwarding client binding. // Forwarding client binding.
mojo::Binding<network::mojom::URLLoader> binding_; mojo::Binding<network::mojom::URLLoader> binding_;
network::mojom::URLLoaderClientPtr forwarding_client_; network::mojom::URLLoaderClientPtr forwarding_client_;
......
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