Skip to content

Commit

Permalink
Add kSpeculativeServiceWorkerStartup feature (disabled by default)
Browse files Browse the repository at this point in the history
This CL adds kSpeculativeServiceWorkerStartup feature which starts
Service Worker in a speculative manner on browser initiated and renderer
initiated navigation.

Bug: 1377753
Change-Id: If26bed4320dadcd3b5d583a7efd838817375f409
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3937789
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Rakina Zata Amni <rakina@chromium.org>
Reviewed-by: Alexander Timin <altimin@chromium.org>
Reviewed-by: Takashi Toyoshima <toyoshim@chromium.org>
Commit-Queue: Minoru Chikamune <chikamune@chromium.org>
Reviewed-by: Charlie Reis <creis@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Yoshisato Yanagisawa <yyanagisawa@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1068953}
  • Loading branch information
chikamune-cr authored and Chromium LUCI CQ committed Nov 9, 2022
1 parent 687207d commit 6d74bb0
Show file tree
Hide file tree
Showing 15 changed files with 211 additions and 0 deletions.
26 changes: 26 additions & 0 deletions content/browser/renderer_host/navigation_request.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
#include "content/browser/web_package/web_bundle_utils.h"
#include "content/common/content_constants_internal.h"
#include "content/common/debug_utils.h"
#include "content/common/features.h"
#include "content/common/navigation_params_utils.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_task_traits.h"
Expand Down Expand Up @@ -1525,6 +1526,8 @@ NavigationRequest::NavigationRequest(
? absl::make_optional(
FencedFrameURLMapping::FencedFrameProperties())
: absl::nullopt) {
TRACE_EVENT1("navigation", "NavigationRequest::NavigationRequest", "url",
GetURL());
DCHECK(!blink::IsRendererDebugURL(common_params_->url));
DCHECK(common_params_->method == "POST" || !common_params_->post_data);
DCHECK_EQ(common_params_->url, commit_params_->original_url);
Expand Down Expand Up @@ -1839,6 +1842,29 @@ NavigationRequest::NavigationRequest(
commit_params_->fallback_srcdoc_baseurl =
frame_tree_node_->parent()->GetBaseUrl();
}

// Ask the service worker context to speculatively start a service worker for
// the request URL if necessary for optimization purposes. Don't ask to do
// that if this request is for ReloadType::BYPASSING_CACHE that is supposed to
// skip a service worker. There are cases where we have already started the
// service worker (e.g, Prerendering or the previous navigation already
// started the service worker), but this call does nothing if the service
// worker already started for the URL.
if (reload_type_ != ReloadType::BYPASSING_CACHE &&
base::FeatureList::IsEnabled(kSpeculativeServiceWorkerStartup)) {
if (ServiceWorkerContext* context =
frame_tree_node_->navigator()
.controller()
.GetBrowserContext()
->GetStoragePartition(site_info_.storage_partition_config())
->GetServiceWorkerContext()) {
const blink::StorageKey key(GetTentativeOriginAtRequestTime());
if (context->MaybeHasRegistrationForStorageKey(key)) {
context->StartServiceWorkerForNavigationHint(GetURL(), key,
base::DoNothing());
}
}
}
}

NavigationRequest::~NavigationRequest() {
Expand Down
28 changes: 28 additions & 0 deletions content/browser/renderer_host/render_frame_host_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@
#include "content/common/associated_interfaces.mojom.h"
#include "content/common/content_navigation_policy.h"
#include "content/common/debug_utils.h"
#include "content/common/features.h"
#include "content/common/frame.mojom.h"
#include "content/common/frame_messages.mojom.h"
#include "content/common/navigation_client.mojom.h"
Expand Down Expand Up @@ -5402,6 +5403,33 @@ void RenderFrameHostImpl::RunBeforeUnloadConfirm(
std::move(dialog_closed_callback));
}

void RenderFrameHostImpl::WillPotentiallyStartNavigation(const GURL& url) {
TRACE_EVENT1("navigation",
"RenderFrameHostImpl::WillPotentiallyStartNavigation", "url",
url);

GURL filtered_url(url);
GetProcess()->FilterURL(/*empty_allowed=*/false, &filtered_url);
if (filtered_url == GURL(kBlockedURL))
return;

// Ask the service worker context to speculatively start a service worker for
// the request URL if necessary for optimization purposes. There are cases
// where we have already started the service worker (e.g, Prerendering or the
// previous navigation already started the service worker), but this call does
// nothing if the service worker already started for the URL.
if (base::FeatureList::IsEnabled(kSpeculativeServiceWorkerStartup)) {
if (ServiceWorkerContext* context =
GetStoragePartition()->GetServiceWorkerContext()) {
const blink::StorageKey key(url::Origin::Create(filtered_url));
if (context->MaybeHasRegistrationForStorageKey(key)) {
context->StartServiceWorkerForNavigationHint(filtered_url, key,
base::DoNothing());
}
}
}
}

// TODO(crbug.com/1213863): Move this method to content::PageImpl.
void RenderFrameHostImpl::UpdateFaviconURL(
std::vector<blink::mojom::FaviconURLPtr> favicon_urls) {
Expand Down
1 change: 1 addition & 0 deletions content/browser/renderer_host/render_frame_host_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2214,6 +2214,7 @@ class CONTENT_EXPORT RenderFrameHostImpl
RunModalPromptDialogCallback callback) override;
void RunBeforeUnloadConfirm(bool is_reload,
RunBeforeUnloadConfirmCallback callback) override;
void WillPotentiallyStartNavigation(const GURL& url) override;
void UpdateFaviconURL(
std::vector<blink::mojom::FaviconURLPtr> favicon_urls) override;
void DownloadURL(blink::mojom::DownloadURLParamsPtr params) override;
Expand Down
103 changes: 103 additions & 0 deletions content/browser/service_worker/service_worker_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4129,4 +4129,107 @@ IN_PROC_BROWSER_TEST_P(ServiceWorkerBrowserTestWithStoragePartitioning,
}
#endif

enum class SpeculativeStartupNavigationType {
kBrowserInitiatedNavigation,
kRendererInitiatedNavigation
};

// This is a test class to verify an optimization to speculatively start a
// service worker for navigation before the "beforeunload" event.
class ServiceWorkerSpeculativeStartupBrowserTest
: public ServiceWorkerBrowserTest,
public testing::WithParamInterface<SpeculativeStartupNavigationType> {
public:
ServiceWorkerSpeculativeStartupBrowserTest() {
feature_list_.InitFromCommandLine("SpeculativeServiceWorkerStartup", "");
}
~ServiceWorkerSpeculativeStartupBrowserTest() override = default;

void SetUpOnMainThread() override {
ServiceWorkerBrowserTest::SetUpOnMainThread();
StartServerAndNavigateToSetup();
}

WebContents* web_contents() const { return shell()->web_contents(); }

RenderFrameHost* GetPrimaryMainFrame() {
return web_contents()->GetPrimaryMainFrame();
}

base::HistogramTester& histogram_tester() { return histogram_tester_; }

private:
base::test::ScopedFeatureList feature_list_;
base::HistogramTester histogram_tester_;
};

INSTANTIATE_TEST_SUITE_P(
All,
ServiceWorkerSpeculativeStartupBrowserTest,
testing::Values(
SpeculativeStartupNavigationType::kBrowserInitiatedNavigation,
SpeculativeStartupNavigationType::kRendererInitiatedNavigation));

IN_PROC_BROWSER_TEST_P(ServiceWorkerSpeculativeStartupBrowserTest,
NavigationWillBeCanceledByBeforeUnload) {
const GURL create_service_worker_url(embedded_test_server()->GetURL(
"/service_worker/create_service_worker.html"));
const GURL out_scope_url(embedded_test_server()->GetURL("/empty.html"));
const GURL in_scope_url(
embedded_test_server()->GetURL("/service_worker/empty.html"));

// Register a service worker.
WorkerRunningStatusObserver observer1(public_context());
EXPECT_TRUE(NavigateToURL(shell(), create_service_worker_url));
EXPECT_EQ("DONE",
EvalJs(GetPrimaryMainFrame(), "register('fetch_event.js');"));
observer1.WaitUntilRunning();

scoped_refptr<ServiceWorkerVersion> version =
wrapper()->GetLiveVersion(observer1.version_id());
EXPECT_EQ(EmbeddedWorkerStatus::RUNNING, version->running_status());

// Stop the current running service worker.
StopServiceWorker(version.get());
EXPECT_EQ(EmbeddedWorkerStatus::STOPPED, version->running_status());

// Navigate away from the service worker's scope.
EXPECT_TRUE(NavigateToURL(shell(), out_scope_url));
EXPECT_EQ(EmbeddedWorkerStatus::STOPPED, version->running_status());

// Cancel the next navigation with beforeunload.
EXPECT_TRUE(
ExecJs(GetPrimaryMainFrame(), "window.onbeforeunload = () => 'x';"));
EXPECT_TRUE(web_contents()->NeedToFireBeforeUnloadOrUnloadEvents());
PrepContentsForBeforeUnloadTest(web_contents());
SetShouldProceedOnBeforeUnload(shell(),
/*proceed=*/true,
/*success=*/false);

// Confirm that the service worker speculatively started even when the
// navigation was canceled.
WorkerRunningStatusObserver observer2(public_context());
AppModalDialogWaiter dialog_waiter(shell());
switch (GetParam()) {
case SpeculativeStartupNavigationType::kBrowserInitiatedNavigation:
shell()->LoadURL(in_scope_url);
break;
case SpeculativeStartupNavigationType::kRendererInitiatedNavigation:
EXPECT_TRUE(BeginNavigateToURLFromRenderer(shell(), in_scope_url));
break;
}
dialog_waiter.Wait();
EXPECT_TRUE(dialog_waiter.WasDialogRequestedCallbackCalled());
observer2.WaitUntilRunning();
EXPECT_EQ(
EmbeddedWorkerStatus::RUNNING,
wrapper()->GetLiveVersion(observer2.version_id())->running_status());
histogram_tester().ExpectBucketCount(
"ServiceWorker.StartWorker.Purpose",
static_cast<int>(ServiceWorkerMetrics::EventType::NAVIGATION_HINT), 1);
histogram_tester().ExpectBucketCount(
"ServiceWorker.StartWorker.StatusByPurpose_NAVIGATION_HINT",
static_cast<int>(blink::ServiceWorkerStatusCode::kOk), 1);
}

} // namespace content
13 changes: 13 additions & 0 deletions content/browser/service_worker/service_worker_context_wrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
#include "content/public/browser/webui_config_map.h"
#include "content/public/common/content_client.h"
#include "content/public/common/content_features.h"
#include "content/public/common/origin_util.h"
#include "content/public/common/url_constants.h"
#include "mojo/public/cpp/bindings/pending_remote.h"
#include "mojo/public/cpp/bindings/self_owned_receiver.h"
Expand Down Expand Up @@ -810,6 +811,18 @@ void ServiceWorkerContextWrapper::StartServiceWorkerForNavigationHint(
std::move(callback).Run(StartServiceWorkerForNavigationHintResult::FAILED);
return;
}

// Checking this is for performance optimization. Without this check,
// following FindRegistrationForClientUrl() can detect if the given URL has
// service worker registration or not. But FindRegistrationForClientUrl()
// takes time to compute. Hence avoid calling it when the given URL clearly
// doesn't register service workers.
if (!OriginCanAccessServiceWorkers(document_url)) {
std::move(callback).Run(StartServiceWorkerForNavigationHintResult::
NO_SERVICE_WORKER_REGISTRATION);
return;
}

context_core_->registry()->FindRegistrationForClientUrl(
net::SimplifyUrlForRequest(document_url), key,
base::BindOnce(
Expand Down
4 changes: 4 additions & 0 deletions content/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ BASE_FEATURE(kQueueNavigationsWhileWaitingForCommit,
"QueueNavigationsWhileWaitingForPendingCommit",
base::FEATURE_DISABLED_BY_DEFAULT);

BASE_FEATURE(kSpeculativeServiceWorkerStartup,
"SpeculativeServiceWorkerStartup",
base::FEATURE_DISABLED_BY_DEFAULT);

// Please keep features in alphabetical order.

} // namespace content
3 changes: 3 additions & 0 deletions content/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ BASE_DECLARE_FEATURE(kOnShowWithPageVisibility);
// See https://crbug.com/838348 and https://crbug.com/1220337.
BASE_DECLARE_FEATURE(kQueueNavigationsWhileWaitingForCommit);

// (crbug/1377753): Speculatively start service worker before BeforeUnload runs.
BASE_DECLARE_FEATURE(kSpeculativeServiceWorkerStartup);

// Please keep features in alphabetical order.

} // namespace content
Expand Down
7 changes: 7 additions & 0 deletions content/renderer/render_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
#include "content/common/associated_interfaces.mojom.h"
#include "content/common/content_navigation_policy.h"
#include "content/common/debug_utils.h"
#include "content/common/features.h"
#include "content/common/frame.mojom.h"
#include "content/common/frame_messages.mojom.h"
#include "content/common/main_frame_counter.h"
Expand Down Expand Up @@ -5144,6 +5145,12 @@ void RenderFrameImpl::BeginNavigation(
}
}

if (IsTopLevelNavigation(frame_) && url.SchemeIsHTTPOrHTTPS() &&
!url.is_empty() &&
base::FeatureList::IsEnabled(kSpeculativeServiceWorkerStartup)) {
frame_->WillPotentiallyStartNavigation(url);
}

if (info->navigation_policy == blink::kWebNavigationPolicyCurrentTab) {
// Execute the BeforeUnload event. If asked not to proceed or the frame is
// destroyed, ignore the navigation.
Expand Down
9 changes: 9 additions & 0 deletions third_party/blink/public/mojom/frame/frame.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,15 @@ interface LocalFrameHost {
[Sync]
RunBeforeUnloadConfirm(bool is_reload) => (bool success);

// Notifies that a renderer-initiated navigation to |url| will
// potentially start soon. This is fired before the beforeunload event
// (if it's needed) gets dispatched in the renderer, so that the
// browser can speculatively start service worker before processing
// beforeunload event, which might take a long time. Note that the
// navigation might not actually start, e.g. if it gets canceled by
// beforeunload.
WillPotentiallyStartNavigation(url.mojom.Url url);

// Notifies that the urls for the favicon of a site has been determined.
UpdateFaviconURL(array<FaviconURL> favicon_urls);

Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/public/web/web_navigation_control.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ class WebNavigationControl : public WebLocalFrame {
virtual void SetIsNotOnInitialEmptyDocument() = 0;
virtual bool IsOnInitialEmptyDocument() = 0;

virtual void WillPotentiallyStartNavigation(const WebURL&) const = 0;

// Marks the frame as loading, before WebLocalFrameClient issues a navigation
// request through the browser process on behalf of the frame.
// This runs some JavaScript event listeners, which may cancel the navigation
Expand Down
6 changes: 6 additions & 0 deletions third_party/blink/renderer/core/frame/local_frame.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1877,6 +1877,12 @@ bool LocalFrame::CanNavigate(const Frame& target_frame,
return false;
}

void LocalFrame::WillPotentiallyStartNavigation(const KURL& url) const {
TRACE_EVENT1("navigation", "LocalFrame::WillPotentiallyStartNavigation",
"url", url);
GetLocalFrameHostRemote().WillPotentiallyStartNavigation(url);
}

ContentCaptureManager* LocalFrame::GetOrResetContentCaptureManager() {
DCHECK(Client());
if (!IsLocalRoot())
Expand Down
2 changes: 2 additions & 0 deletions third_party/blink/renderer/core/frame/local_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,8 @@ class CORE_EXPORT LocalFrame final
// navigation at a later time.
bool CanNavigate(const Frame&, const KURL& destination_url = KURL());

void WillPotentiallyStartNavigation(const KURL& url) const;

// Whether a navigation should replace the current history entry or not.
// Note this isn't exhaustive; there are other cases where a navigation does a
// replacement which this function doesn't cover.
Expand Down
5 changes: 5 additions & 0 deletions third_party/blink/renderer/core/frame/web_local_frame_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2688,6 +2688,11 @@ void WebLocalFrameImpl::DownloadURL(
std::move(blob_url_token));
}

void WebLocalFrameImpl::WillPotentiallyStartNavigation(
const WebURL& url) const {
GetFrame()->WillPotentiallyStartNavigation(url);
}

bool WebLocalFrameImpl::WillStartNavigation(const WebNavigationInfo& info) {
DCHECK(!info.url_request.IsNull());
DCHECK(!info.url_request.Url().ProtocolIs("javascript"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,7 @@ class CORE_EXPORT WebLocalFrameImpl final
bool is_browser_initiated) override;
void SetIsNotOnInitialEmptyDocument() override;
bool IsOnInitialEmptyDocument() override;
void WillPotentiallyStartNavigation(const WebURL&) const override;
bool WillStartNavigation(const WebNavigationInfo&) override;
void DidDropNavigation() override;
void DownloadURL(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ class FakeLocalFrameHost : public mojom::blink::LocalFrameHost {
RunModalPromptDialogCallback callback) override;
void RunBeforeUnloadConfirm(bool is_reload,
RunBeforeUnloadConfirmCallback callback) override;
void WillPotentiallyStartNavigation(const KURL& url) override {}
void UpdateFaviconURL(
WTF::Vector<blink::mojom::blink::FaviconURLPtr> favicon_urls) override;
void DownloadURL(mojom::blink::DownloadURLParamsPtr params) override;
Expand Down

0 comments on commit 6d74bb0

Please sign in to comment.