Commit c60c0b7a authored by Hiroki Nakagawa's avatar Hiroki Nakagawa Committed by Commit Bot

OptimizationGuide: Move inquiry logic from WebContentsObserver

For code simplification, this CL moves logic to inquire about
optimization hints for Blink from
BlinkOptimizationGuideWebContentsObserver to
BlinkOptimizationGuideInquirer. The observer creates and starts the
inquirer when a main frame navigation gets ready to commit. If the
inquirer for the previous navigation exists, it's destroyed at that
point to abort inflight inquiries.

> Motivation of this change

Before this CL, the observer had mainly 2 responsibilities:
(1) observing navigation events on WebContents, (2) inquiring the
optimization guide service about hints. (1) is bound with the
lifetime of WebContents, while (2) is bound with the lifetime of
the main frame. The mismatch exists here. In terms of class modeling,
(1) and (2) should have been in separate classes. To fix this mismatch,
this CL moves (2) into BlinkOptimizationGuideInquirer.

Change-Id: I713da144e4716c0d68f9a14fdc7d4ff6930fa77a
Bug: 1113980
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2358948
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799023}
parent 0903a279
......@@ -946,6 +946,10 @@ static_library("browser") {
"offline_items_collection/offline_content_aggregator_factory.cc",
"offline_items_collection/offline_content_aggregator_factory.h",
"omnibox/common/omnibox_features.h",
"optimization_guide/blink/blink_optimization_guide_feature_flag_helper.cc",
"optimization_guide/blink/blink_optimization_guide_feature_flag_helper.h",
"optimization_guide/blink/blink_optimization_guide_inquirer.cc",
"optimization_guide/blink/blink_optimization_guide_inquirer.h",
"optimization_guide/blink/blink_optimization_guide_web_contents_observer.cc",
"optimization_guide/blink/blink_optimization_guide_web_contents_observer.h",
"optimization_guide/optimization_guide_hints_manager.cc",
......
......@@ -3,6 +3,7 @@
// found in the LICENSE file.
#include "base/feature_list.h"
#include "chrome/browser/optimization_guide/blink/blink_optimization_guide_inquirer.h"
#include "chrome/browser/optimization_guide/blink/blink_optimization_guide_web_contents_observer.h"
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h"
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h"
......@@ -40,6 +41,10 @@ class BlinkOptimizationGuideBrowserTestBase : public InProcessBrowserTest {
FromWebContents(browser()->tab_strip_model()->GetActiveWebContents());
}
BlinkOptimizationGuideInquirer* GetCurrentInquirer() {
return GetObserverForActiveWebContents()->current_inquirer();
}
GURL GetURLWithMockHost(const std::string& relative_url) const {
// The optimization guide service doesn't work with the localhost. Instead,
// resolve the relative url with the mock host.
......@@ -152,11 +157,12 @@ IN_PROC_BROWSER_TEST_P(BlinkOptimizationGuideBrowserTest, Basic) {
// is enabled.
ui_test_utils::NavigateToURL(browser(), GetURLWithMockHost("/simple.html"));
{
auto& hints = GetObserverForActiveWebContents()->sent_hints_for_testing();
blink::mojom::BlinkOptimizationGuideHintsPtr hints =
GetCurrentInquirer()->GetHints();
if (IsFeatureFlagEnabled()) {
EXPECT_TRUE(CheckIfHintsAvailable(hints));
EXPECT_TRUE(CheckIfHintsAvailable(*hints));
} else {
EXPECT_FALSE(CheckIfHintsAvailable(hints));
EXPECT_FALSE(CheckIfHintsAvailable(*hints));
}
}
......@@ -164,19 +170,21 @@ IN_PROC_BROWSER_TEST_P(BlinkOptimizationGuideBrowserTest, Basic) {
ui_test_utils::NavigateToURL(browser(),
GetURLWithMockHost("/simple.html?different"));
{
auto& hints = GetObserverForActiveWebContents()->sent_hints_for_testing();
EXPECT_FALSE(CheckIfHintsAvailable(hints));
blink::mojom::BlinkOptimizationGuideHintsPtr hints =
GetCurrentInquirer()->GetHints();
EXPECT_FALSE(CheckIfHintsAvailable(*hints));
}
// Navigation to the URL again should see the same hints as long as the
// optimization guide is enabled.
ui_test_utils::NavigateToURL(browser(), GetURLWithMockHost("/simple.html"));
{
auto& hints = GetObserverForActiveWebContents()->sent_hints_for_testing();
blink::mojom::BlinkOptimizationGuideHintsPtr hints =
GetCurrentInquirer()->GetHints();
if (IsFeatureFlagEnabled()) {
EXPECT_TRUE(CheckIfHintsAvailable(hints));
EXPECT_TRUE(CheckIfHintsAvailable(*hints));
} else {
EXPECT_FALSE(CheckIfHintsAvailable(hints));
EXPECT_FALSE(CheckIfHintsAvailable(*hints));
}
}
}
......@@ -189,9 +197,9 @@ IN_PROC_BROWSER_TEST_P(BlinkOptimizationGuideBrowserTest, NoMetadata) {
// Navigation to the URL shouldn't see the hints.
ui_test_utils::NavigateToURL(browser(), GetURLWithMockHost("/simple.html"));
blink::mojom::BlinkOptimizationGuideHints& hints =
GetObserverForActiveWebContents()->sent_hints_for_testing();
EXPECT_FALSE(CheckIfHintsAvailable(hints));
blink::mojom::BlinkOptimizationGuideHintsPtr hints =
GetCurrentInquirer()->GetHints();
EXPECT_FALSE(CheckIfHintsAvailable(*hints));
}
// Tests behavior when the optimization guide service is disabled.
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/optimization_guide/blink/blink_optimization_guide_feature_flag_helper.h"
#include "base/feature_list.h"
#include "third_party/blink/public/common/features.h"
namespace optimization_guide {
bool ShouldUseOptimizationGuideForDelayAsyncScript() {
static const bool is_feature_enabled =
base::FeatureList::IsEnabled(
blink::features::kDelayAsyncScriptExecution) &&
blink::features::kDelayAsyncScriptExecutionDelayParam.Get() ==
blink::features::DelayAsyncScriptDelayType::kUseOptimizationGuide;
return is_feature_enabled;
}
} // namespace optimization_guide
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_OPTIMIZATION_GUIDE_BLINK_BLINK_OPTIMIZATION_GUIDE_FEATURE_FLAG_HELPER_H_
#define CHROME_BROWSER_OPTIMIZATION_GUIDE_BLINK_BLINK_OPTIMIZATION_GUIDE_FEATURE_FLAG_HELPER_H_
namespace optimization_guide {
bool ShouldUseOptimizationGuideForDelayAsyncScript();
} // namespace optimization_guide
#endif // CHROME_BROWSER_OPTIMIZATION_GUIDE_BLINK_BLINK_OPTIMIZATION_GUIDE_FEATURE_FLAG_HELPER_H_
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chrome/browser/optimization_guide/blink/blink_optimization_guide_inquirer.h"
#include "base/memory/ptr_util.h"
#include "chrome/browser/optimization_guide/blink/blink_optimization_guide_feature_flag_helper.h"
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h"
#include "components/optimization_guide/proto/delay_async_script_execution_metadata.pb.h"
#include "components/optimization_guide/proto/hints.pb.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_handle.h"
namespace optimization_guide {
// static
std::unique_ptr<BlinkOptimizationGuideInquirer>
BlinkOptimizationGuideInquirer::CreateAndStart(
content::NavigationHandle& navigation_handle,
OptimizationGuideDecider& decider) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
auto inquirer = base::WrapUnique(new BlinkOptimizationGuideInquirer());
inquirer->InquireHints(navigation_handle, decider);
return inquirer;
}
BlinkOptimizationGuideInquirer::~BlinkOptimizationGuideInquirer() = default;
BlinkOptimizationGuideInquirer::BlinkOptimizationGuideInquirer()
: optimization_guide_hints_(
blink::mojom::BlinkOptimizationGuideHints::New()) {}
void BlinkOptimizationGuideInquirer::InquireHints(
content::NavigationHandle& navigation_handle,
OptimizationGuideDecider& decider) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
std::vector<proto::OptimizationType> supported_optimization_types;
if (ShouldUseOptimizationGuideForDelayAsyncScript()) {
supported_optimization_types.push_back(
proto::OptimizationType::DELAY_ASYNC_SCRIPT_EXECUTION);
}
for (auto optimization_type : supported_optimization_types) {
// CanApplyOptimizationAsync() synchronously runs the callback when the
// hints are already available.
decider.CanApplyOptimizationAsync(
&navigation_handle, optimization_type,
base::BindOnce(&BlinkOptimizationGuideInquirer::DidInquireHints,
weak_ptr_factory_.GetWeakPtr(), optimization_type));
}
}
void BlinkOptimizationGuideInquirer::DidInquireHints(
proto::OptimizationType optimization_type,
OptimizationGuideDecision decision,
const OptimizationMetadata& metadata) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
switch (decision) {
case OptimizationGuideDecision::kTrue:
break;
case OptimizationGuideDecision::kUnknown:
case OptimizationGuideDecision::kFalse:
// The optimization guide service decided not to provide the hints.
return;
}
switch (optimization_type) {
case proto::OptimizationType::DELAY_ASYNC_SCRIPT_EXECUTION:
PopulateHintsForDelayAsyncScriptExecution(metadata);
break;
default:
NOTREACHED();
break;
}
}
void BlinkOptimizationGuideInquirer::PopulateHintsForDelayAsyncScriptExecution(
const OptimizationMetadata& optimization_metadata) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
// Give up providing the hints when the metadata is not available.
base::Optional<proto::DelayAsyncScriptExecutionMetadata> metadata =
optimization_metadata
.ParsedMetadata<proto::DelayAsyncScriptExecutionMetadata>();
if (!metadata || !metadata->delay_type())
return;
// Populate the metadata into the hints.
using blink::mojom::DelayAsyncScriptExecutionDelayType;
auto hints = blink::mojom::DelayAsyncScriptExecutionHints::New();
switch (metadata->delay_type()) {
case proto::DelayType::DELAY_TYPE_UNKNOWN:
hints->delay_type = DelayAsyncScriptExecutionDelayType::kUnknown;
break;
case proto::DelayType::DELAY_TYPE_FINISHED_PARSING:
hints->delay_type = DelayAsyncScriptExecutionDelayType::kFinishedParsing;
break;
case proto::DelayType::DELAY_TYPE_FIRST_PAINT_OR_FINISHED_PARSING:
hints->delay_type =
DelayAsyncScriptExecutionDelayType::kFirstPaintOrFinishedParsing;
break;
}
DCHECK(!optimization_guide_hints_->delay_async_script_execution_hints);
optimization_guide_hints_->delay_async_script_execution_hints =
std::move(hints);
}
} // namespace optimization_guide
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROME_BROWSER_OPTIMIZATION_GUIDE_BLINK_BLINK_OPTIMIZATION_GUIDE_INQUIRER_H_
#define CHROME_BROWSER_OPTIMIZATION_GUIDE_BLINK_BLINK_OPTIMIZATION_GUIDE_INQUIRER_H_
#include "base/memory/weak_ptr.h"
#include "components/optimization_guide/optimization_guide_decider.h"
#include "third_party/blink/public/mojom/optimization_guide/optimization_guide.mojom.h"
namespace content {
class NavigationHandle;
} // namespace content
namespace optimization_guide {
// BlinkOptimizationGuideInquirer asks the optimization guide service about
// hints for optimization types for Blink. This is instantiated per main frame
// by BlinkOptimizationGuideWebContentsObserver, and destroyed when the next
// main frame navigation gets ready to commit.
class BlinkOptimizationGuideInquirer {
public:
// Creates an instance of this class, and starts an inquiry.
static std::unique_ptr<BlinkOptimizationGuideInquirer> CreateAndStart(
content::NavigationHandle& navigation_handle,
OptimizationGuideDecider& decider);
~BlinkOptimizationGuideInquirer();
BlinkOptimizationGuideInquirer(const BlinkOptimizationGuideInquirer&) =
delete;
BlinkOptimizationGuideInquirer& operator=(
const BlinkOptimizationGuideInquirer&) = delete;
BlinkOptimizationGuideInquirer(BlinkOptimizationGuideInquirer&&) = delete;
BlinkOptimizationGuideInquirer& operator=(BlinkOptimizationGuideInquirer&&) =
delete;
// Returns a snapshot of the hints currently available. This may not contain
// information for some optimization types yet because the optimization guide
// service needs network access to provide the hints when they are not locally
// cached.
blink::mojom::BlinkOptimizationGuideHintsPtr GetHints() {
return optimization_guide_hints_.Clone();
}
private:
BlinkOptimizationGuideInquirer();
// Asks the optimization guide service about the hints. When the hints are
// locally cached in the service, this synchronously updates
// |optimization_guide_hints_|.
void InquireHints(content::NavigationHandle& navigation_handle,
OptimizationGuideDecider& decider);
void DidInquireHints(proto::OptimizationType optimization_type,
OptimizationGuideDecision decision,
const OptimizationMetadata& metadata);
void PopulateHintsForDelayAsyncScriptExecution(
const OptimizationMetadata& optimization_metadata);
// The hints currently available.
blink::mojom::BlinkOptimizationGuideHintsPtr optimization_guide_hints_;
base::WeakPtrFactory<BlinkOptimizationGuideInquirer> weak_ptr_factory_{this};
};
} // namespace optimization_guide
#endif // CHROME_BROWSER_OPTIMIZATION_GUIDE_BLINK_BLINK_OPTIMIZATION_GUIDE_INQUIRER_H_
......@@ -7,110 +7,19 @@
#include <utility>
#include <vector>
#include "base/feature_list.h"
#include "base/memory/ref_counted.h"
#include "chrome/browser/optimization_guide/blink/blink_optimization_guide_feature_flag_helper.h"
#include "chrome/browser/optimization_guide/blink/blink_optimization_guide_inquirer.h"
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service.h"
#include "chrome/browser/optimization_guide/optimization_guide_keyed_service_factory.h"
#include "chrome/browser/profiles/profile.h"
#include "components/optimization_guide/proto/delay_async_script_execution_metadata.pb.h"
#include "components/optimization_guide/proto/hints.pb.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/navigation_handle.h"
#include "third_party/blink/public/common/associated_interfaces/associated_interface_provider.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/mojom/loader/previews_resource_loading_hints.mojom.h"
namespace optimization_guide {
namespace {
bool IsDelayAsyncScriptExecutionEnabled() {
static const bool is_feature_enabled =
base::FeatureList::IsEnabled(
blink::features::kDelayAsyncScriptExecution) &&
blink::features::kDelayAsyncScriptExecutionDelayParam.Get() ==
blink::features::DelayAsyncScriptDelayType::kUseOptimizationGuide;
return is_feature_enabled;
}
// Used for storing results of CanApplyOptimizationAsync().
struct QueryResult {
bool is_ready = false;
OptimizationGuideDecision decision;
OptimizationMetadata metadata;
};
blink::mojom::DelayAsyncScriptExecutionHintsPtr
CreateDelayAsyncScriptExecutionHints(
content::NavigationHandle* navigation_handle,
OptimizationGuideDecider* decider) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
DCHECK(IsDelayAsyncScriptExecutionEnabled());
// CanApplyOptimizationAsync() synchronously runs the callback when the hints
// are already available. The following code assumes the case.
// TODO(https://crbug.com/1113980): Support the case where the hints get
// available after this point.
// Creates an object to share the result between the current calling function
// and the callback for CanApplyOptimizationAsync(). This should be refcounted
// because the callback may outlive the current calling function and vice
// versa.
auto result = base::MakeRefCounted<base::RefCountedData<QueryResult>>();
decider->CanApplyOptimizationAsync(
navigation_handle, proto::OptimizationType::DELAY_ASYNC_SCRIPT_EXECUTION,
base::BindOnce(
[](scoped_refptr<base::RefCountedData<QueryResult>> result,
OptimizationGuideDecision decision,
const OptimizationMetadata& metadata) {
result->data = {true, decision, metadata};
},
result));
// TODO(https://crbug.com/1113980): Add UMAs to record if the hints are
// available on navigation commit ready.
// Didn't get the hints synchronously.
if (!result->data.is_ready)
return nullptr;
switch (result->data.decision) {
case OptimizationGuideDecision::kTrue:
break;
case OptimizationGuideDecision::kUnknown:
case OptimizationGuideDecision::kFalse:
// The optimization guide service decided not to provide the hints.
return nullptr;
}
// Give up providing the hints when the metadata is not available.
base::Optional<proto::DelayAsyncScriptExecutionMetadata> metadata =
result->data.metadata
.ParsedMetadata<proto::DelayAsyncScriptExecutionMetadata>();
if (!metadata || !metadata->delay_type())
return nullptr;
// Populate the decision into the hints.
using blink::mojom::DelayAsyncScriptExecutionDelayType;
auto hints = blink::mojom::DelayAsyncScriptExecutionHints::New();
switch (metadata->delay_type()) {
case proto::DelayType::DELAY_TYPE_UNKNOWN:
hints->delay_type = DelayAsyncScriptExecutionDelayType::kUnknown;
break;
case proto::DelayType::DELAY_TYPE_FINISHED_PARSING:
hints->delay_type = DelayAsyncScriptExecutionDelayType::kFinishedParsing;
break;
case proto::DelayType::DELAY_TYPE_FIRST_PAINT_OR_FINISHED_PARSING:
hints->delay_type =
DelayAsyncScriptExecutionDelayType::kFirstPaintOrFinishedParsing;
break;
}
return hints;
}
} // namespace
BlinkOptimizationGuideWebContentsObserver::
~BlinkOptimizationGuideWebContentsObserver() = default;
......@@ -124,6 +33,9 @@ void BlinkOptimizationGuideWebContentsObserver::ReadyToCommitNavigation(
return;
}
// Abort the inquiry for the previous main frame navigation.
current_inquirer_.reset();
// Don't support non-HTTP(S) navigation.
if (!navigation_handle->GetURL().SchemeIsHTTPOrHTTPS())
return;
......@@ -133,11 +45,18 @@ void BlinkOptimizationGuideWebContentsObserver::ReadyToCommitNavigation(
if (!decider)
return;
auto hints = blink::mojom::BlinkOptimizationGuideHints::New();
if (IsDelayAsyncScriptExecutionEnabled()) {
hints->delay_async_script_execution_hints =
CreateDelayAsyncScriptExecutionHints(navigation_handle, decider);
}
// Creates and starts a new inquiry for this main frame navigation.
current_inquirer_ = BlinkOptimizationGuideInquirer::CreateAndStart(
*navigation_handle, *decider);
// The hints may not be ready for some optimization types because the inquirer
// asynchronously asks the optimization guide server if the hints are not
// locally cached.
// TODO(https://crbug.com/1113980): Support the case where the hints get
// available after navigation commit.
// TODO(https://crbug.com/1113980): Add UMAs to record if the hints are
// available on navigation commit ready.
blink::mojom::BlinkOptimizationGuideHintsPtr hints =
current_inquirer_->GetHints();
// Tentatively use the Previews interface to talk with the renderer.
// TODO(https://crbug.com/1113980): Implement our own interface.
......@@ -150,13 +69,9 @@ void BlinkOptimizationGuideWebContentsObserver::ReadyToCommitNavigation(
->GetInterface(&hints_receiver_associated);
}
// Keeping the hints to be sent for testing.
// TODO(https://crbug.com/1113980): Stop doing this, and replace this with a
// less intrusive way.
sent_hints_for_testing_ = hints.Clone();
// Sends the hints to the renderer.
hints_receiver_associated->SetBlinkOptimizationGuideHints(std::move(hints));
// Sends the hints currently available to the renderer.
hints_receiver_associated->SetBlinkOptimizationGuideHints(
current_inquirer_->GetHints());
}
BlinkOptimizationGuideWebContentsObserver::
......@@ -173,7 +88,7 @@ BlinkOptimizationGuideWebContentsObserver::
// Register the optimization types which we want to subscribe to.
std::vector<proto::OptimizationType> opts;
if (IsDelayAsyncScriptExecutionEnabled())
if (ShouldUseOptimizationGuideForDelayAsyncScript())
opts.push_back(proto::OptimizationType::DELAY_ASYNC_SCRIPT_EXECUTION);
if (!opts.empty())
decider->RegisterOptimizationTypes(opts);
......
......@@ -13,6 +13,8 @@ class Profile;
namespace optimization_guide {
class BlinkOptimizationGuideInquirer;
// BlinkOptimizationGuideWebContentsObserver observes navigation events, queries
// the optimization guide service about optimization hints for Blink, and then
// sends them to a renderer immediately before navigation commit.
......@@ -42,9 +44,8 @@ class BlinkOptimizationGuideWebContentsObserver final
void ReadyToCommitNavigation(
content::NavigationHandle* navigation_handle) override;
// Used for testing.
blink::mojom::BlinkOptimizationGuideHints& sent_hints_for_testing() {
return *sent_hints_for_testing_;
BlinkOptimizationGuideInquirer* current_inquirer() {
return current_inquirer_.get();
}
private:
......@@ -57,8 +58,8 @@ class BlinkOptimizationGuideWebContentsObserver final
Profile* const profile_;
// Used for testing.
blink::mojom::BlinkOptimizationGuideHintsPtr sent_hints_for_testing_;
// Reset every time the main frame navigation gets ready to commit.
std::unique_ptr<BlinkOptimizationGuideInquirer> current_inquirer_;
};
} // namespace optimization_guide
......
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