[LiteVideo] Make asynchronous decision and hint call for opt guide.

This change reworks the decision and hint logic to prepare for integrating
with the optimization guide. The observer functions with an asynchronous
callback so that hints can come either from the optimization guide or the
current hint mechanism via Finch.

The next change will start the integration with the optimization guide
behind a flag.

Bug: 1117064
Change-Id: I320635e795aafa2db2b2d413f766f94a4077cb9b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2367024Reviewed-by: Sophie Chang <sophiechang@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Commit-Queue: Michael Crouse <mcrouse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#800377}

chrome/browser/lite_video/lite_video_decider.cc

@@ -110,36 +110,41 @@ LiteVideoDecider::~LiteVideoDecider() {
   content::GetNetworkConnectionTracker()->RemoveNetworkConnectionObserver(this);
 }
 
-base::Optional<LiteVideoHint> LiteVideoDecider::CanApplyLiteVideo(
+void LiteVideoDecider::CanApplyLiteVideo(
     content::NavigationHandle* navigation_handle,
-    LiteVideoBlocklistReason* blocklist_reason) {
+    LiteVideoHintCallback callback) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  DCHECK(blocklist_reason);
-  if (blocklist_reason)
-    *blocklist_reason = LiteVideoBlocklistReason::kUnknown;
-
+  LiteVideoBlocklistReason blocklist_reason =
+      LiteVideoBlocklistReason::kUnknown;
   if (!IsLiteVideoAllowedForUser(Profile::FromBrowserContext(
           navigation_handle->GetWebContents()->GetBrowserContext()))) {
-    return base::nullopt;
+    std::move(callback).Run(base::nullopt, blocklist_reason);
+    return;
   }
 
   if (switches::ShouldOverrideLiteVideoDecision()) {
     // Return a default configured hint.
-    return LiteVideoHint(switches::GetDefaultDownlinkBandwidthKbps(),
+    std::move(callback).Run(
+        LiteVideoHint(switches::GetDefaultDownlinkBandwidthKbps(),
                       features::LiteVideoTargetDownlinkRTTLatency(),
                       features::LiteVideoKilobytesToBufferBeforeThrottle(),
-                         features::LiteVideoMaxThrottlingDelay());
+                      features::LiteVideoMaxThrottlingDelay()),
+        blocklist_reason);
+    return;
   }
 
   if (!CanApplyOnCurrentNetworkConditions(is_cellular_network_,
                                           current_effective_connection_type_)) {
-    return base::nullopt;
+    std::move(callback).Run(base::nullopt, blocklist_reason);
+    return;
   }
 
   GURL url = navigation_handle->GetURL();
 
-  if (!url.SchemeIsHTTPOrHTTPS())
-    return base::nullopt;
+  if (!url.SchemeIsHTTPOrHTTPS()) {
+    std::move(callback).Run(base::nullopt, blocklist_reason);
+    return;
+  }
 
   // Reloads and Forward-Back navigations are considered opt-outs and are added
   // to the blocklist so that a host that is frequently reloaded on does not get
@@ -149,26 +154,29 @@ base::Optional<LiteVideoHint> LiteVideoDecider::CanApplyLiteVideo(
   if (is_reload || (navigation_handle->GetPageTransition() &
                     ui::PAGE_TRANSITION_FORWARD_BACK)) {
     user_blocklist_->AddNavigationToBlocklist(navigation_handle, true);
-    *blocklist_reason = is_reload
+    blocklist_reason = is_reload
                            ? LiteVideoBlocklistReason::kNavigationReload
                            : LiteVideoBlocklistReason::kNavigationForwardBack;
     ScopedLiteVideoDecisionRecorder scoped_decision_recorder(
-        *blocklist_reason, navigation_handle->IsInMainFrame());
-    return base::nullopt;
+        blocklist_reason, navigation_handle->IsInMainFrame());
+    std::move(callback).Run(base::nullopt, blocklist_reason);
+    return;
   }
 
-  *blocklist_reason =
+  blocklist_reason =
       user_blocklist_->IsLiteVideoAllowedOnNavigation(navigation_handle);
   ScopedLiteVideoDecisionRecorder scoped_decision_recorder(
-      *blocklist_reason, navigation_handle->IsInMainFrame());
+      blocklist_reason, navigation_handle->IsInMainFrame());
 
   base::Optional<LiteVideoHint> hint =
       hint_cache_->GetHintForNavigationURL(url);
   if (hint)
     scoped_decision_recorder.set_has_hint_for_host(true);
 
-  if (*blocklist_reason != LiteVideoBlocklistReason::kAllowed || !hint)
-    return base::nullopt;
+  if (blocklist_reason != LiteVideoBlocklistReason::kAllowed || !hint) {
+    std::move(callback).Run(base::nullopt, blocklist_reason);
+    return;
+  }
 
   // The navigation will have the LiteVideo optimization triggered so
   // update the blocklist.
@@ -179,7 +187,7 @@ base::Optional<LiteVideoHint> LiteVideoDecider::CanApplyLiteVideo(
       : DidMediaRebuffer(
             navigation_handle->GetWebContents()->GetLastCommittedURL(),
             navigation_handle->GetURL(), false);
-  return hint;
+  std::move(callback).Run(hint, blocklist_reason);
 }
 
 void LiteVideoDecider::OnUserBlocklistedStatusChange(bool blocklisted) {

chrome/browser/lite_video/lite_video_decider.h

@@ -22,6 +22,10 @@ class OptOutStore;
 
 namespace lite_video {
 
+using LiteVideoHintCallback =
+    base::OnceCallback<void(base::Optional<LiteVideoHint> hint,
+                            LiteVideoBlocklistReason blocklist_reason)>;
+
 // The LiteVideoDecider makes the decision on whether LiteVideos should be
 // applied to a navigation and provides the parameters to use when
 // throttling media requests.
@@ -34,14 +38,13 @@ class LiteVideoDecider
                    base::Clock* clock);
   ~LiteVideoDecider() override;
 
-  // Determine if the navigation can have the LiteVideo optimization
-  // applied and returns the LiteVideoHint to use for throttling if one exists.
-  // This also updates the blocklist based on the navigation provided and should
-  // be limited to one call per navigation. |blocklist_reason| will be
-  // populated, if applicable.
-  base::Optional<LiteVideoHint> CanApplyLiteVideo(
-      content::NavigationHandle* navigation_handle,
-      LiteVideoBlocklistReason* blocklist_reason);
+  // Determine if the navigation can have the LiteVideo optimization applied. It
+  // invokes |callback| with the blocklist result and a LiteVideoHint if
+  // available for the |navigation_handle|. This also updates the blocklist
+  // based on the navigation provided and should be limited to one call per
+  // navigation.
+  void CanApplyLiteVideo(content::NavigationHandle* navigation_handle,
+                         LiteVideoHintCallback callback);
 
   // Override the blocklist used by |this| for testing.
   void SetUserBlocklistForTesting(

chrome/browser/lite_video/lite_video_hint.h

@@ -34,10 +34,10 @@ class LiteVideoHint {
   base::TimeDelta max_throttling_delay() const { return max_throttling_delay_; }
 
  private:
-  const int target_downlink_bandwidth_kbps_;
-  const base::TimeDelta target_downlink_rtt_latency_;
-  const int kilobytes_to_buffer_before_throttle_;
-  const base::TimeDelta max_throttling_delay_;
+  int target_downlink_bandwidth_kbps_;
+  base::TimeDelta target_downlink_rtt_latency_;
+  int kilobytes_to_buffer_before_throttle_;
+  base::TimeDelta max_throttling_delay_;
 };
 
 }  // namespace lite_video

chrome/browser/lite_video/lite_video_navigation_metrics.cc

@@ -23,4 +23,12 @@ void LiteVideoNavigationMetrics::SetThrottleResult(
   throttle_result_ = throttle_result;
 }
 
+void LiteVideoNavigationMetrics::SetDecision(LiteVideoDecision decision) {
+  decision_ = decision;
+}
+void LiteVideoNavigationMetrics::SetBlocklistReason(
+    LiteVideoBlocklistReason blocklist_reason) {
+  blocklist_reason_ = blocklist_reason;
+}
+
 }  // namespace lite_video

chrome/browser/lite_video/lite_video_navigation_metrics.h

@@ -58,6 +58,13 @@ class LiteVideoNavigationMetrics {
   // Update the throttling result of the current navigation.
   void SetThrottleResult(LiteVideoThrottleResult throttle_result);
 
+  // Update the decision to made on applying LiteVideos to the current
+  // navigation.
+  void SetDecision(LiteVideoDecision decision);
+
+  // Update the blocklist reason for the current navigation.
+  void SetBlocklistReason(LiteVideoBlocklistReason blocklist_reason);
+
  private:
   int64_t nav_id_;
   LiteVideoDecision decision_;

chrome/browser/lite_video/lite_video_observer.cc

@@ -4,6 +4,7 @@
 
 #include "chrome/browser/lite_video/lite_video_observer.h"
 
+#include "base/bind.h"
 #include "base/metrics/histogram_macros_local.h"
 #include "base/optional.h"
 #include "base/rand_util.h"
@@ -18,6 +19,8 @@
 #include "chrome/browser/lite_video/lite_video_util.h"
 #include "chrome/browser/profiles/profile.h"
 #include "content/public/browser/navigation_handle.h"
+#include "content/public/browser/render_frame_host.h"
+#include "content/public/browser/render_process_host.h"
 #include "content/public/browser/web_contents.h"
 #include "services/metrics/public/cpp/ukm_builders.h"
 #include "services/metrics/public/cpp/ukm_recorder.h"
@@ -80,29 +83,53 @@ void LiteVideoObserver::DidFinishNavigation(
 
   lite_video::LiteVideoBlocklistReason blocklist_reason =
       lite_video::LiteVideoBlocklistReason::kUnknown;
-  base::Optional<lite_video::LiteVideoHint> hint =
-      lite_video_decider_->CanApplyLiteVideo(navigation_handle,
-                                             &blocklist_reason);
-
-  MaybeUpdateCoinflipExperimentState(navigation_handle);
-
-  lite_video::LiteVideoDecision decision =
-      MakeLiteVideoDecision(navigation_handle, hint);
 
   if (navigation_handle->IsInMainFrame()) {
     FlushUKMMetrics();
     nav_metrics_ = lite_video::LiteVideoNavigationMetrics(
-        navigation_handle->GetNavigationId(), decision, blocklist_reason,
+        navigation_handle->GetNavigationId(),
+        lite_video::LiteVideoDecision::kUnknown, blocklist_reason,
         lite_video::LiteVideoThrottleResult::kThrottledWithoutStop);
   }
 
+  MaybeUpdateCoinflipExperimentState(navigation_handle);
+  auto* render_frame_host = navigation_handle->GetRenderFrameHost();
+  if (!render_frame_host)
+    return;
+  lite_video_decider_->CanApplyLiteVideo(
+      navigation_handle,
+      base::BindOnce(&LiteVideoObserver::OnHintAvailable,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     content::GlobalFrameRoutingId(
+                         render_frame_host->GetProcess()->GetID(),
+                         render_frame_host->GetRoutingID())));
+}
+
+void LiteVideoObserver::OnHintAvailable(
+    content::GlobalFrameRoutingId render_frame_host_routing_id,
+    base::Optional<lite_video::LiteVideoHint> hint,
+    lite_video::LiteVideoBlocklistReason blocklist_reason) {
+  auto* render_frame_host =
+      content::RenderFrameHost::FromID(render_frame_host_routing_id);
+  if (!render_frame_host)
+    return;
+
+  lite_video::LiteVideoDecision decision = MakeLiteVideoDecision(hint);
+
   LOCAL_HISTOGRAM_BOOLEAN("LiteVideo.Navigation.HasHint", hint ? true : false);
 
-  if (decision == lite_video::LiteVideoDecision::kNotAllowed)
+  if (nav_metrics_ && render_frame_host->GetMainFrame()) {
+    nav_metrics_->SetDecision(decision);
+    nav_metrics_->SetBlocklistReason(blocklist_reason);
+  }
+
+  // Only proceed to passing hints if the decision is allowed.
+  if (decision != lite_video::LiteVideoDecision::kAllowed)
+    return;
+
+  if (!hint)
     return;
 
-  content::RenderFrameHost* render_frame_host =
-      navigation_handle->GetRenderFrameHost();
   if (!render_frame_host || !render_frame_host->GetProcess())
     return;
 
@@ -125,7 +152,6 @@ void LiteVideoObserver::DidFinishNavigation(
 }
 
 lite_video::LiteVideoDecision LiteVideoObserver::MakeLiteVideoDecision(
-    content::NavigationHandle* navigation_handle,
     base::Optional<lite_video::LiteVideoHint> hint) const {
   if (hint) {
     return is_coinflip_holdback_ ? lite_video::LiteVideoDecision::kHoldback

chrome/browser/lite_video/lite_video_observer.h

@@ -44,7 +44,6 @@ class LiteVideoObserver
   // Determines the LiteVideoDecision based on |hint| and the coinflip
   // holdback state.
   lite_video::LiteVideoDecision MakeLiteVideoDecision(
-      content::NavigationHandle* navigation_handle,
       base::Optional<lite_video::LiteVideoHint> hint) const;
 
   // Records the metrics for LiteVideos applied to any frames associated with
@@ -56,6 +55,13 @@ class LiteVideoObserver
   void MaybeUpdateCoinflipExperimentState(
       content::NavigationHandle* navigation_handle);
 
+  // Callback run after a hint and blocklist reason is available for use
+  // within the agent associated with |render_frame_host_routing_id|.
+  void OnHintAvailable(
+      content::GlobalFrameRoutingId render_frame_host_routing_id,
+      base::Optional<lite_video::LiteVideoHint> hint,
+      lite_video::LiteVideoBlocklistReason blocklist_reason);
+
   // The decider capable of making decisions about whether LiteVideos should be
   // applied and the params to use when throttling media requests.
   lite_video::LiteVideoDecider* lite_video_decider_ = nullptr;
@@ -70,6 +76,9 @@ class LiteVideoObserver
   // commits.
   bool is_coinflip_holdback_ = false;
 
+  // Used to get a weak pointer to |this|.
+  base::WeakPtrFactory<LiteVideoObserver> weak_ptr_factory_{this};
+
   WEB_CONTENTS_USER_DATA_KEY_DECL();
 };