Adds new observer method to DRPDataUseObserver for DidFinishLoad

This is plumbing preparation for estimating data savings for the
NoScript preview (determined when page load finished event occurs).

Bug: 781885
Change-Id: I145b9d270327c757ea1fbfbc44aa5932822c592e
Reviewed-on: https://chromium-review.googlesource.com/820891
Commit-Queue: Doug Arnett <dougarnett@chromium.org>
Reviewed-by: rajendrant <rajendrant@chromium.org>
Cr-Commit-Position: refs/heads/master@{#524231}

chrome/browser/data_use_measurement/chrome_data_use_ascriber.cc

@@ -242,7 +242,7 @@ void ChromeDataUseAscriber::OnUrlRequestDestroyed(net::URLRequest* request) {
   // If all requests are done for |entry| and no more requests can be attributed
   // to it, it is safe to delete.
   if (entry->IsDataUseComplete() && !page_load_is_tracked) {
-    NotifyDataUseCompleted(entry);
+    NotifyPageLoadConcluded(entry);
     data_use_recorders_.erase(entry);
   }
 }
@@ -290,7 +290,7 @@ void ChromeDataUseAscriber::RenderFrameDeleted(int render_process_id,
     if (main_render_frame_entry_map_.end() != main_frame_it) {
       DataUseRecorderEntry entry = main_frame_it->second.data_use_recorder;
       if (entry->IsDataUseComplete()) {
-        NotifyDataUseCompleted(entry);
+        NotifyPageLoadConcluded(entry);
         data_use_recorders_.erase(entry);
       }
       main_render_frame_entry_map_.erase(main_frame_it);
@@ -362,7 +362,7 @@ void ChromeDataUseAscriber::DidFinishMainFrameNavigation(
       main_frame_it->second.data_use_recorder = data_use_recorders_.end();
       NotifyPageLoadCommit(old_frame_entry);
       if (old_frame_entry->IsDataUseComplete()) {
-        NotifyDataUseCompleted(old_frame_entry);
+        NotifyPageLoadConcluded(old_frame_entry);
         data_use_recorders_.erase(old_frame_entry);
       }
 
@@ -379,12 +379,12 @@ void ChromeDataUseAscriber::DidFinishMainFrameNavigation(
   entry->set_main_frame_id(main_frame);
 
   // If the frame has already been deleted then mark this navigation as having
-  // completed its data use.
+  // concluded its data use.
   if (main_frame_it == main_render_frame_entry_map_.end()) {
     entry->set_page_transition(page_transition);
     NotifyPageLoadCommit(entry);
     if (entry->IsDataUseComplete()) {
-      NotifyDataUseCompleted(entry);
+      NotifyPageLoadConcluded(entry);
       data_use_recorders_.erase(entry);
     }
     return;
@@ -441,7 +441,7 @@ void ChromeDataUseAscriber::DidFinishMainFrameNavigation(
       }
     }
     if (old_frame_entry->IsDataUseComplete()) {
-      NotifyDataUseCompleted(old_frame_entry);
+      NotifyPageLoadConcluded(old_frame_entry);
       data_use_recorders_.erase(old_frame_entry);
     }
     entry->set_is_visible(main_frame_it->second.is_visible);
@@ -450,14 +450,41 @@ void ChromeDataUseAscriber::DidFinishMainFrameNavigation(
   }
 }
 
+void ChromeDataUseAscriber::DidFinishLoad(int render_process_id,
+                                          int render_frame_id,
+                                          const GURL& validated_url) {
+  // Only continue for validated HTTP* URLs (e.g., not internal error pages).
+  if (!validated_url.SchemeIsHTTPOrHTTPS())
+    return;
+
+  RenderFrameHostID main_frame(render_process_id, render_frame_id);
+
+  auto main_frame_it = main_render_frame_entry_map_.find(main_frame);
+  if (main_frame_it == main_render_frame_entry_map_.end())
+    return;
+
+  // Check that the DataUse entry has a committed URL.
+  DataUseRecorderEntry entry = main_frame_it->second.data_use_recorder;
+  DataUse& data_use = entry->data_use();
+  if (data_use.url().is_valid()) {
+    NotifyDidFinishLoad(entry);
+  }
+}
+
 void ChromeDataUseAscriber::NotifyPageLoadCommit(DataUseRecorderEntry entry) {
   for (auto& observer : observers_)
     observer.OnPageLoadCommit(&entry->data_use());
 }
 
-void ChromeDataUseAscriber::NotifyDataUseCompleted(DataUseRecorderEntry entry) {
+void ChromeDataUseAscriber::NotifyDidFinishLoad(DataUseRecorderEntry entry) {
+  for (auto& observer : observers_)
+    observer.OnPageDidFinishLoad(&entry->data_use());
+}
+
+void ChromeDataUseAscriber::NotifyPageLoadConcluded(
+    DataUseRecorderEntry entry) {
   for (auto& observer : observers_)
-    observer.OnPageLoadComplete(&entry->data_use());
+    observer.OnPageLoadConcluded(&entry->data_use());
 }
 
 std::unique_ptr<URLRequestClassifier>

chrome/browser/data_use_measurement/chrome_data_use_ascriber.h

@@ -111,6 +111,11 @@ class ChromeDataUseAscriber : public DataUseAscriber {
                               int new_render_process_id,
                               int new_render_frame_id);
 
+  // Called when the load is finished.
+  void DidFinishLoad(int render_process_id,
+                     int render_frame_id,
+                     const GURL& validated_url);
+
  private:
   friend class ChromeDataUseAscriberTest;
 
@@ -167,7 +172,8 @@ class ChromeDataUseAscriber : public DataUseAscriber {
       net::URLRequest* request);
 
   void NotifyPageLoadCommit(DataUseRecorderEntry entry);
-  void NotifyDataUseCompleted(DataUseRecorderEntry entry);
+  void NotifyDidFinishLoad(DataUseRecorderEntry entry);
+  void NotifyPageLoadConcluded(DataUseRecorderEntry entry);
 
   DataUseRecorderEntry CreateNewDataUseRecorder(
       net::URLRequest* request,

chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.cc

@@ -186,6 +186,22 @@ void ChromeDataUseAscriberService::DidFinishNavigation(
                      base::TimeTicks::Now()));
 }
 
+void ChromeDataUseAscriberService::DidFinishLoad(
+    content::RenderFrameHost* main_render_frame_host,
+    const GURL& validated_url) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+
+  if (!ascriber_)
+    return;
+
+  content::BrowserThread::PostTask(
+      content::BrowserThread::IO, FROM_HERE,
+      base::BindOnce(&ChromeDataUseAscriber::DidFinishLoad,
+                     base::Unretained(ascriber_),
+                     main_render_frame_host->GetProcess()->GetID(),
+                     main_render_frame_host->GetRoutingID(), validated_url));
+}
+
 void ChromeDataUseAscriberService::SetDataUseAscriber(
     ChromeDataUseAscriber* ascriber) {
   DCHECK(!is_initialized_);

chrome/browser/data_use_measurement/chrome_data_use_ascriber_service.h

@@ -10,6 +10,7 @@
 
 #include "base/macros.h"
 #include "components/keyed_service/core/keyed_service.h"
+#include "url/gurl.h"
 
 namespace content {
 class NavigationHandle;
@@ -66,6 +67,10 @@ class ChromeDataUseAscriberService : public KeyedService {
   // Forwarded from DataUseWebContentsObserver
   void DidFinishNavigation(content::NavigationHandle* navigation_handle);
 
+  // Forwarded from DataUseWebContentsObserver
+  void DidFinishLoad(content::RenderFrameHost* main_render_frame_host,
+                     const GURL& validated_url);
+
  private:
   friend class ChromeDataUseAscriberServiceTest;
 

chrome/browser/data_use_measurement/chrome_data_use_ascriber_unittest.cc

@@ -37,7 +37,9 @@ class MockPageLoadObserver
   MOCK_METHOD2(OnPageResourceLoad,
                void(const net::URLRequest& request,
                     data_use_measurement::DataUse* data_use));
-  MOCK_METHOD1(OnPageLoadComplete,
+  MOCK_METHOD1(OnPageDidFinishLoad,
+               void(data_use_measurement::DataUse* data_use));
+  MOCK_METHOD1(OnPageLoadConcluded,
                void(data_use_measurement::DataUse* data_use));
 };
 
@@ -483,7 +485,7 @@ TEST_F(ChromeDataUseAscriberTest, PageLoadObserverNotified) {
   ascriber()->OnUrlRequestCompleted(*request, false);
 
   EXPECT_CALL(mock_observer, OnPageLoadCommit(data_use)).Times(1);
-  EXPECT_CALL(mock_observer, OnPageLoadComplete(testing::_)).Times(1);
+  EXPECT_CALL(mock_observer, OnPageLoadConcluded(testing::_)).Times(1);
   ascriber()->DidFinishMainFrameNavigation(
       kRenderProcessId, kRenderFrameId, GURL("http://mobile.test.com"), false,
       kPageTransition, base::TimeTicks::Now());
@@ -495,9 +497,70 @@ TEST_F(ChromeDataUseAscriberTest, PageLoadObserverNotified) {
   EXPECT_EQ(content::GlobalRequestID(kRenderProcessId, 0),
             recorder_entry.main_frame_request_id());
   EXPECT_EQ(GURL("http://mobile.test.com"), recorder_entry.data_use().url());
-  EXPECT_CALL(mock_observer, OnPageLoadComplete(&recorder_entry.data_use()))
+
+  EXPECT_CALL(mock_observer, OnPageDidFinishLoad(&recorder_entry.data_use()))
+      .Times(1);
+  ascriber()->DidFinishLoad(kRenderProcessId, kRenderFrameId,
+                            GURL("http://mobile.test.com"));
+
+  EXPECT_CALL(mock_observer, OnPageLoadConcluded(&recorder_entry.data_use()))
       .Times(1);
+  ascriber()->RenderFrameDeleted(kRenderProcessId, kRenderFrameId, -1, -1);
+  ascriber()->OnUrlRequestDestroyed(request.get());
+
+  EXPECT_EQ(0u, recorders().size());
+}
+
+TEST_F(ChromeDataUseAscriberTest, PageLoadObserverForErrorPageValidatedURL) {
+  MockPageLoadObserver mock_observer;
+  ascriber()->AddObserver(&mock_observer);
+
+  std::unique_ptr<net::URLRequest> request = CreateNewRequest(
+      "http://test.com", true, kRequestId, kRenderProcessId, kRenderFrameId);
+
+  // Mainframe is created.
+  ascriber()->RenderFrameCreated(kRenderProcessId, kRenderFrameId, -1, -1);
+  EXPECT_EQ(1u, recorders().size());
+
+  ascriber()->OnBeforeUrlRequest(request.get());
+
+  // Navigation starts.
+  ascriber()->DidStartMainFrameNavigation(GURL("http://test.com"),
+                                          kRenderProcessId, kRenderFrameId,
+                                          kNavigationHandle);
 
+  ascriber()->ReadyToCommitMainFrameNavigation(
+      content::GlobalRequestID(kRenderProcessId, 0), kRenderProcessId,
+      kRenderFrameId);
+
+  EXPECT_EQ(2u, recorders().size());
+  DataUse* data_use = &recorders().back().data_use();
+
+  EXPECT_CALL(mock_observer, OnPageResourceLoad(testing::_, data_use)).Times(1);
+  ascriber()->OnUrlRequestCompleted(*request, false);
+
+  EXPECT_CALL(mock_observer, OnPageLoadCommit(data_use)).Times(1);
+  EXPECT_CALL(mock_observer, OnPageLoadConcluded(testing::_)).Times(1);
+  ascriber()->DidFinishMainFrameNavigation(
+      kRenderProcessId, kRenderFrameId, GURL("http://mobile.test.com"), false,
+      kPageTransition, base::TimeTicks::Now());
+
+  EXPECT_EQ(1u, recorders().size());
+  auto& recorder_entry = recorders().front();
+  EXPECT_EQ(RenderFrameHostID(kRenderProcessId, kRenderFrameId),
+            recorder_entry.main_frame_id());
+  EXPECT_EQ(content::GlobalRequestID(kRenderProcessId, 0),
+            recorder_entry.main_frame_request_id());
+  EXPECT_EQ(GURL("http://mobile.test.com"), recorder_entry.data_use().url());
+
+  // Now expect no DidFinishLoad observer call for "about:blank" validated URL.
+  EXPECT_CALL(mock_observer, OnPageDidFinishLoad(&recorder_entry.data_use()))
+      .Times(0);
+  ascriber()->DidFinishLoad(kRenderProcessId, kRenderFrameId,
+                            GURL("about:blank"));
+
+  EXPECT_CALL(mock_observer, OnPageLoadConcluded(&recorder_entry.data_use()))
+      .Times(1);
   ascriber()->RenderFrameDeleted(kRenderProcessId, kRenderFrameId, -1, -1);
   ascriber()->OnUrlRequestDestroyed(request.get());
 

chrome/browser/data_use_measurement/data_use_web_contents_observer.cc

@@ -93,4 +93,10 @@ void DataUseWebContentsObserver::DidFinishNavigation(
   service_->DidFinishNavigation(navigation_handle);
 }
 
+void DataUseWebContentsObserver::DidFinishLoad(
+    content::RenderFrameHost* render_frame_host,
+    const GURL& validated_url) {
+  service_->DidFinishLoad(render_frame_host, validated_url);
+}
+
 }  // namespace data_use_measurement

chrome/browser/data_use_measurement/data_use_web_contents_observer.h

@@ -45,6 +45,8 @@ class DataUseWebContentsObserver
                               content::RenderFrameHost* new_host) override;
   void DidFinishNavigation(
       content::NavigationHandle* navigation_handle) override;
+  void DidFinishLoad(content::RenderFrameHost* render_frame_host,
+                     const GURL& validated_url) override;
 
  private:
   friend class content::WebContentsUserData<DataUseWebContentsObserver>;

components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.cc

@@ -132,4 +132,10 @@ void DataReductionProxyDataUseObserver::OnPageResourceLoad(
   }
 }
 
+void DataReductionProxyDataUseObserver::OnPageDidFinishLoad(
+    data_use_measurement::DataUse* data_use) {
+  DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
+  // TODO(dougarnett): 781885 - estimate data saving for NoScript.
+}
+
 }  // namespace data_reduction_proxy

components/data_reduction_proxy/core/browser/data_reduction_proxy_data_use_observer.h

@@ -37,7 +37,8 @@ class DataReductionProxyDataUseObserver
   void OnPageLoadCommit(data_use_measurement::DataUse* data_use) override;
   void OnPageResourceLoad(const net::URLRequest& request,
                           data_use_measurement::DataUse* data_use) override;
-  void OnPageLoadComplete(data_use_measurement::DataUse* data_use) override {}
+  void OnPageDidFinishLoad(data_use_measurement::DataUse* data_use) override;
+  void OnPageLoadConcluded(data_use_measurement::DataUse* data_use) override {}
 
   // |data_reduction_proxy_io_data_| owns |this| and is destroyed only after
   // |this| is destroyed in the IO thread.

components/data_use_measurement/core/data_use_ascriber.h

@@ -48,9 +48,14 @@ class DataUseAscriber {
     virtual void OnPageResourceLoad(const net::URLRequest& request,
                                     DataUse* data_use) = 0;
 
+    // The DidFinishLoad event occurred for the main frame. That is, the page
+    // load is nominally done (however, the page can still issue more network
+    // requests between this event and |OnPageLoadConcluded|.
+    virtual void OnPageDidFinishLoad(DataUse* data_use) = 0;
+
     // The page load completed. This is when the tab is closed or another
     // navigation starts due to omnibox search, link clicks, page reload, etc.
-    virtual void OnPageLoadComplete(DataUse* data_use) = 0;
+    virtual void OnPageLoadConcluded(DataUse* data_use) = 0;
   };
 
   DataUseAscriber();