Commit 455e7762 authored by Sophie Chang's avatar Sophie Chang Committed by Commit Bot

Continue evaluating hint logic even if target is not painful

Bug: 969558
Change-Id: I842006ad992bec31b814205b6fae6ddb0f698414
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1856662Reviewed-by: default avatarMichael Crouse <mcrouse@chromium.org>
Reviewed-by: default avatarTarun Bansal <tbansal@chromium.org>
Commit-Queue: Sophie Chang <sophiechang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#708604}
parent 09bcdef1
...@@ -744,20 +744,22 @@ void OptimizationGuideHintsManager::CanApplyOptimization( ...@@ -744,20 +744,22 @@ void OptimizationGuideHintsManager::CanApplyOptimization(
*optimization_type_decision = *optimization_type_decision =
optimization_guide::OptimizationTypeDecision::kUnknown; optimization_guide::OptimizationTypeDecision::kUnknown;
// We only support the optimization target bool should_update_optimization_target_decision = true;
// |OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD|, so just return that we don't know
// if the target doesn't match that.
if (optimization_target != if (optimization_target !=
optimization_guide::proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD) { optimization_guide::proto::OPTIMIZATION_TARGET_PAINFUL_PAGE_LOAD) {
return; *optimization_target_decision = optimization_guide::
OptimizationTargetDecision::kModelNotAvailableOnClient;
should_update_optimization_target_decision = false;
} }
const auto& url = navigation_handle->GetURL(); const auto& url = navigation_handle->GetURL();
// If the URL doesn't have a host, we cannot query the hint for it, so just // If the URL doesn't have a host, we cannot query the hint for it, so just
// return early. // return early.
if (!url.has_host()) { if (!url.has_host()) {
if (should_update_optimization_target_decision) {
*optimization_target_decision = *optimization_target_decision =
optimization_guide::OptimizationTargetDecision::kPageLoadDoesNotMatch; optimization_guide::OptimizationTargetDecision::kPageLoadDoesNotMatch;
}
*optimization_type_decision = *optimization_type_decision =
optimization_guide::OptimizationTypeDecision::kNoHintAvailable; optimization_guide::OptimizationTypeDecision::kNoHintAvailable;
return; return;
...@@ -794,17 +796,19 @@ void OptimizationGuideHintsManager::CanApplyOptimization( ...@@ -794,17 +796,19 @@ void OptimizationGuideHintsManager::CanApplyOptimization(
matched_page_hint->max_ect_trigger()); matched_page_hint->max_ect_trigger());
} }
if (should_update_optimization_target_decision) {
if (current_effective_connection_type_ == if (current_effective_connection_type_ ==
net::EffectiveConnectionType::EFFECTIVE_CONNECTION_TYPE_UNKNOWN || net::EffectiveConnectionType::EFFECTIVE_CONNECTION_TYPE_UNKNOWN ||
current_effective_connection_type_ > max_ect_trigger) { current_effective_connection_type_ > max_ect_trigger) {
// The current network is not slow enough, so this navigation is likely not // The current network is not slow enough, so this navigation is likely
// going to be painful. // not going to be painful.
*optimization_target_decision = *optimization_target_decision =
optimization_guide::OptimizationTargetDecision::kPageLoadDoesNotMatch; optimization_guide::OptimizationTargetDecision::kPageLoadDoesNotMatch;
} else { } else {
*optimization_target_decision = *optimization_target_decision =
optimization_guide::OptimizationTargetDecision::kPageLoadMatches; optimization_guide::OptimizationTargetDecision::kPageLoadMatches;
} }
}
// Check if the URL should be filtered out if we have an optimization filter // Check if the URL should be filtered out if we have an optimization filter
// for the type. // for the type.
......
...@@ -1592,21 +1592,22 @@ TEST_F(OptimizationGuideHintsManagerTest, ...@@ -1592,21 +1592,22 @@ TEST_F(OptimizationGuideHintsManagerTest,
optimization_guide::proto::OPTIMIZATION_TARGET_UNKNOWN, optimization_guide::proto::OPTIMIZATION_TARGET_UNKNOWN,
optimization_guide::proto::NOSCRIPT, &optimization_target_decision, optimization_guide::proto::NOSCRIPT, &optimization_target_decision,
&optimization_type_decision, &optimization_metadata); &optimization_type_decision, &optimization_metadata);
// Make sure metadata is cleared. // Make sure metadata is populated.
EXPECT_EQ(0, optimization_metadata.previews_metadata.inflation_percent()); EXPECT_EQ(1234, optimization_metadata.previews_metadata.inflation_percent());
// Make sure decisions are logged correctly. // Make sure decisions are logged correctly.
EXPECT_EQ(optimization_guide::OptimizationTargetDecision::kUnknown, EXPECT_EQ(optimization_guide::OptimizationTargetDecision::
kModelNotAvailableOnClient,
optimization_target_decision); optimization_target_decision);
EXPECT_EQ(optimization_guide::OptimizationTypeDecision::kUnknown, EXPECT_EQ(optimization_guide::OptimizationTypeDecision::kAllowedByHint,
optimization_type_decision); optimization_type_decision);
// Make sure navigation data is populated correctly. // Make sure navigation data is populated correctly.
OptimizationGuideNavigationData* navigation_data = OptimizationGuideNavigationData* navigation_data =
GetOptimizationGuideNavigationData(navigation_handle.get()); GetOptimizationGuideNavigationData(navigation_handle.get());
EXPECT_TRUE(navigation_data->has_hint_before_commit().value()); EXPECT_TRUE(navigation_data->has_hint_before_commit().value());
EXPECT_EQ(base::nullopt, navigation_data->has_hint_after_commit()); EXPECT_TRUE(navigation_data->has_hint_after_commit().value());
EXPECT_EQ(base::nullopt, navigation_data->serialized_hint_version_string()); EXPECT_EQ("someversion", navigation_data->serialized_hint_version_string());
EXPECT_FALSE(navigation_data->has_page_hint_value()); EXPECT_TRUE(navigation_data->has_page_hint_value());
} }
TEST_F(OptimizationGuideHintsManagerTest, TEST_F(OptimizationGuideHintsManagerTest,
......
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