Add a way to send the resource percentage signal to the RC.

chrome/browser/offline_pages/background_loader_offliner.cc

 // Copyright 2016 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/offline_pages/background_loader_offliner.h"
 
 #include "base/bind.h"
 #include "base/json/json_writer.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/sys_info.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "chrome/browser/android/offline_pages/offline_page_mhtml_archiver.h"
 #include "chrome/browser/android/offline_pages/offliner_helper.h"
+#include "chrome/browser/offline_pages/offliner_user_data.h"
+#include "chrome/browser/page_load_metrics/page_load_metrics_initialize.h"
 #include "chrome/browser/profiles/profile.h"
 #include "components/offline_pages/core/background/offliner_policy.h"
 #include "components/offline_pages/core/background/save_page_request.h"
 #include "components/offline_pages/core/client_namespace_constants.h"
 #include "components/offline_pages/core/offline_page_feature.h"
 #include "components/offline_pages/core/offline_page_model.h"
 #include "content/public/browser/browser_context.h"
 #include "content/public/browser/mhtml_extra_parts.h"
 #include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_user_data.h"
 
 namespace offline_pages {
 
 namespace {
 const char kContentType[] = "text/plain";
 const char kContentTransferEncodingBinary[] =
     "Content-Transfer-Encoding: binary";
 const char kXHeaderForSignals[] = "X-Chrome-Loading-Metrics-Data: 1";
 
-class OfflinerData : public content::WebContentsUserData<OfflinerData> {
- public:
-  static void AddToWebContents(content::WebContents* webcontents,
-                               BackgroundLoaderOffliner* offliner) {
-    DCHECK(offliner);
-    webcontents->SetUserData(UserDataKey(), std::unique_ptr<OfflinerData>(
-                                                new OfflinerData(offliner)));
-  }
-
-  explicit OfflinerData(BackgroundLoaderOffliner* offliner) {
-    offliner_ = offliner;
-  }
-  BackgroundLoaderOffliner* offliner() { return offliner_; }
-
- private:
-  // The offliner that the WebContents is attached to. The offliner owns the
-  // Delegate which owns the WebContents that this data is attached to.
-  // Therefore, its lifetime should exceed that of the WebContents, so this
-  // should always be non-null.
-  BackgroundLoaderOffliner* offliner_;
-};
-
 std::string AddHistogramSuffix(const ClientId& client_id,
                                const char* histogram_name) {
   if (client_id.name_space.empty()) {
     NOTREACHED();
     return histogram_name;
   }
   std::string adjusted_histogram_name(histogram_name);
   adjusted_histogram_name += "." + client_id.name_space;
   return adjusted_histogram_name;
 }
@@ skipping 30 matching lines @@
       weak_ptr_factory_(this) {
   DCHECK(offline_page_model_);
   DCHECK(browser_context_);
 }
 
 BackgroundLoaderOffliner::~BackgroundLoaderOffliner() {}
 
 // static
 BackgroundLoaderOffliner* BackgroundLoaderOffliner::FromWebContents(
     content::WebContents* contents) {
-  OfflinerData* data = OfflinerData::FromWebContents(contents);
-  if (data)
-    return data->offliner();
+  Offliner* offliner = OfflinerUserData::OfflinerFromWebContents(contents);
+
+  // Today we only have one kind of offliner that uses OfflinerUserData.  If we
+  // add other types, revisit this cast.
+  if (offliner)
+    return static_cast<BackgroundLoaderOffliner*>(offliner);
   return nullptr;
 }
 
 bool BackgroundLoaderOffliner::LoadAndSave(
     const SavePageRequest& request,
     const CompletionCallback& completion_callback,
     const ProgressCallback& progress_callback) {
   DCHECK(completion_callback);
   DCHECK(progress_callback);
   DCHECK(offline_page_model_);
@@ skipping 110 matching lines @@
       if (save_state_ == NONE) {
         did_snapshot_on_last_retry_ = true;
         StartSnapshot();
       }
       return true;
     }
   }
   return false;
 }
 
+void BackgroundLoaderOffliner::ObserveResourceTracking(
+    const ResourceDataType type,
+    int64_t started_count,
+    int64_t completed_count) {
+  // Add the signal to extra data, and use for tracking.
+  if (type == ResourceDataType::IMAGE)
+    AddResourceSignal(type, started_count, completed_count);
+}
+
 void BackgroundLoaderOffliner::MarkLoadStartTime() {
   load_start_time_ = base::TimeTicks::Now();
 }
 
 void BackgroundLoaderOffliner::DocumentAvailableInMainFrame() {
   snapshot_controller_->DocumentAvailableInMainFrame();
   is_low_bar_met_ = true;
 
   // Add this signal to signal_data_.
   AddLoadingSignal("DocumentAvailableInMainFrame");
@@ skipping 207 matching lines @@
 }
 
 void BackgroundLoaderOffliner::ResetLoader() {
   loader_.reset(
       new background_loader::BackgroundLoaderContents(browser_context_));
 }
 
 void BackgroundLoaderOffliner::AttachObservers() {
   content::WebContents* contents = loader_->web_contents();
   content::WebContentsObserver::Observe(contents);
-  OfflinerData::AddToWebContents(contents, this);
+  OfflinerUserData::AddToWebContents(contents, this);
+
+  // Attach the metrics observers to the web contents so we can get resoure
+  // loading signals.
+  chrome::InitializePageLoadMetricsForWebContents(

[Bryan McQuade, 2017/05/23 05:00:01]

do we want to bring up the full page load metrics infrastructure, including all
observers, for this background loader offliner mode? My sense is we probably
don't want to do this - for example I don't think we want to be recording core
page load metrics for this context. In general we only want to bring up the page
load metrics infrastructure for tabs that are actually being displayed to users.
On first glance, I think this is probably out of scope for page load metrics.

I'll need to look at this change in a bit more detail to better understand the
goals & constraints - will try to follow up soon.

[Pete Williamson, 2017/05/24 01:01:50]

As the code is written now, InitializePageLoadMetricsForWebContents will only
initialize the observers that we use while doing background offlining, instead
of all of them. 

My intent moving forwards is to leverage other page load metrics for right
moment detection.  We've talked before about using the FirstMeaningfulPaint and
TimeToInteractive(once it is available) metrics.

+      contents, true /* background loading */);
 }
 
 void BackgroundLoaderOffliner::OnApplicationStateChange(
     base::android::ApplicationState application_state) {
   if (pending_request_ && is_low_end_device_ &&
       application_state ==
           base::android::APPLICATION_STATE_HAS_RUNNING_ACTIVITIES) {
     DVLOG(1) << "App became active, canceling current offlining request";
     // No need to check the return value or complete early, as false would
     // indicate that there was no request, in which case the state change is
     // ignored.
     Cancel(
         base::Bind(HandleApplicationStateChangeCancel, completion_callback_));
   }
 }
 
 void BackgroundLoaderOffliner::AddLoadingSignal(const char* signal_name) {
   base::TimeTicks current_time = base::TimeTicks::Now();
   base::TimeDelta delay_so_far = current_time - load_start_time_;
   // We would prefer to use int64_t here, but JSON does not support that type.
   // Given the choice between int and double, we choose to implicitly convert to
   // a double since it maintains more precision (we can get a longer time in
   // milliseconds than we can with a 2 bit int, 53 bits vs 32).
   double delay = delay_so_far.InMilliseconds();
   signal_data_.SetDouble(signal_name, delay);
 }
 
+void BackgroundLoaderOffliner::AddResourceSignal(const ResourceDataType type,
+                                                 int64_t started_count,
+                                                 int64_t completed_count) {
+  double percentage = 100.0 * static_cast<double>(completed_count) /

[Dmitry Titov, 2017/05/18 02:28:10]

Not clear why do we need the percentage here in addition to started/completed.
It is always computable if needed later.

[Pete Williamson, 2017/05/24 01:01:50]

OK, removed percentage.

+                      static_cast<double>(started_count);
+  // TODO(petewil): Use actual signal type instead of hardcoding name to image.
+  signal_data_.SetDouble("ImagePercentage", percentage);
+  signal_data_.SetDouble("StartedImages", started_count);
+  signal_data_.SetDouble("CompletedImages", completed_count);
+}
+
 }  // namespace offline_pages
-
-DEFINE_WEB_CONTENTS_USER_DATA_KEY(offline_pages::OfflinerData);