Get LoadStatus working with network service.

Bug: 819663

Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: I5a79280401e460954a53de4db529061c3f9bae49
Reviewed-on: https://chromium-review.googlesource.com/1171257
Commit-Queue: John Abd-El-Malek <jam@chromium.org>
Reviewed-by: Charlie Harrison <csharrison@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#582502}

content/browser/loader/resource_dispatcher_host_impl.cc

@@ -1909,6 +1909,7 @@ net::URLRequest* ResourceDispatcherHostImpl::GetURLRequest(
 }
 
 // static
+// This is duplicated in services/network/network_service.cc.
 bool ResourceDispatcherHostImpl::LoadInfoIsMoreInteresting(const LoadInfo& a,
                                                            const LoadInfo& b) {
   // Set |*_uploading_size| to be the size of the corresponding upload body if

content/browser/loader/resource_dispatcher_host_impl.h

@@ -330,6 +330,7 @@ class CONTENT_EXPORT ResourceDispatcherHostImpl
 
  private:
   class ScheduledResourceRequestAdapter;
+  friend class NetworkServiceClient;
   friend class ResourceDispatcherHostTest;
 
   FRIEND_TEST_ALL_PREFIXES(ResourceDispatcherHostTest,

content/browser/network_service_client.cc

@@ -425,4 +425,34 @@ void NetworkServiceClient::OnCookieChange(int process_id,
       process_id, routing_id, url, first_party_url, cookie, blocked_by_policy);
 }
 
+void NetworkServiceClient::OnLoadingStateUpdate(
+    std::vector<network::mojom::LoadInfoPtr> infos,
+    OnLoadingStateUpdateCallback callback) {
+  auto rdh_infos = std::make_unique<ResourceDispatcherHostImpl::LoadInfoList>();
+
+  // TODO(jam): once ResourceDispatcherHost is gone remove the translation
+  // (other than adding the WebContents callback).
+  for (auto& info : infos) {
+    ResourceDispatcherHostImpl::LoadInfo load_info;
+    load_info.host = std::move(info->host);
+    load_info.load_state.state = static_cast<net::LoadState>(info->load_state);
+    load_info.load_state.param = std::move(info->state_param);
+    load_info.upload_position = info->upload_position;
+    load_info.upload_size = info->upload_size;
+    load_info.web_contents_getter =
+        info->process_id
+            ? base::BindRepeating(WebContentsImpl::FromRenderFrameHostID,
+                                  info->process_id, info->routing_id)
+            : base::BindRepeating(WebContents::FromFrameTreeNodeId,
+                                  info->routing_id);
+    rdh_infos->push_back(std::move(load_info));
+  }
+
+  auto* rdh = ResourceDispatcherHostImpl::Get();
+  ResourceDispatcherHostImpl::UpdateLoadStateOnUI(rdh->loader_delegate_,
+                                                  std::move(rdh_infos));
+
+  std::move(callback).Run();
+}
+
 }  // namespace content

content/browser/network_service_client.h

@@ -63,6 +63,8 @@ class CONTENT_EXPORT NetworkServiceClient
                       const GURL& first_party_url,
                       const net::CanonicalCookie& cookie,
                       bool blocked_by_policy) override;
+  void OnLoadingStateUpdate(std::vector<network::mojom::LoadInfoPtr> infos,
+                            OnLoadingStateUpdateCallback callback) override;
 
  private:
   mojo::Binding<network::mojom::NetworkServiceClient> binding_;

content/browser/web_contents/web_contents_impl_browsertest.cc

@@ -57,6 +57,7 @@
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/controllable_http_response.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "services/network/public/cpp/features.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "url/gurl.h"
 
@@ -2367,7 +2368,12 @@ IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest, UpdateLoadState) {
   // Wait for the response to be ready, but never finish it.
   EXPECT_TRUE(frame_pauser.WaitForResponse());
   EXPECT_FALSE(frame_pauser.was_successful());
-  waiter.Wait(net::LOAD_STATE_WAITING_FOR_DELEGATE, paused_host);
+  // Note: the pausing only works for the non-network service path because of
+  // http://crbug.com/791049.
+  if (base::FeatureList::IsEnabled(network::features::kNetworkService))
+    waiter.Wait(net::LOAD_STATE_IDLE, base::string16());
+  else
+    waiter.Wait(net::LOAD_STATE_WAITING_FOR_DELEGATE, paused_host);
 
   load_resource(a_frame, "/a_img");
   a_response->WaitForRequest();
@@ -2390,10 +2396,12 @@ IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest, UpdateLoadState) {
   waiter.Wait(net::LOAD_STATE_READING_RESPONSE, a_host);
   a_response->Done();
 
-  // Now the only request in flight should be the delayed frame.
-  waiter.Wait(net::LOAD_STATE_WAITING_FOR_DELEGATE, paused_host);
-  frame_pauser.ResumeNavigation();
-  waiter.Wait(net::LOAD_STATE_IDLE, base::string16());
+  if (!base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+    // Now the only request in flight should be the delayed frame.
+    waiter.Wait(net::LOAD_STATE_WAITING_FOR_DELEGATE, paused_host);
+    frame_pauser.ResumeNavigation();
+    waiter.Wait(net::LOAD_STATE_IDLE, base::string16());
+  }
 }
 
 IN_PROC_BROWSER_TEST_F(WebContentsImplBrowserTest, NotifyPreferencesChanged) {

services/network/network_context.cc

@@ -249,6 +249,8 @@ class NetworkContext::ContextNetworkDelegate
                                   GURL* new_url) override {
     if (!enable_referrers_)
       request->SetReferrer(std::string());
+    if (network_context_->network_service())
+      network_context_->network_service()->OnBeforeURLRequest();
   }
 
   void OnCompletedInternal(net::URLRequest* request,

services/network/network_service.cc

@@ -13,6 +13,7 @@
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/task/post_task.h"
+#include "base/timer/timer.h"
 #include "base/values.h"
 #include "components/certificate_transparency/sth_distributor.h"
 #include "components/certificate_transparency/sth_observer.h"
@@ -27,11 +28,14 @@
 #include "net/log/file_net_log_observer.h"
 #include "net/log/net_log.h"
 #include "net/log/net_log_util.h"
+#include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_builder.h"
 #include "services/network/mojo_net_log.h"
 #include "services/network/network_context.h"
 #include "services/network/network_usage_accumulator.h"
+#include "services/network/public/cpp/features.h"
 #include "services/network/public/cpp/network_switches.h"
+#include "services/network/url_loader.h"
 #include "services/network/url_request_context_builder_mojo.h"
 
 #if defined(OS_ANDROID) && defined(ARCH_CPU_ARMEL)
@@ -50,6 +54,10 @@ namespace {
 
 NetworkService* g_network_service = nullptr;
 
+// The interval for calls to NetworkService::UpdateLoadStates
+constexpr auto kUpdateLoadStatesInterval =
+    base::TimeDelta::FromMilliseconds(250);
+
 std::unique_ptr<net::NetworkChangeNotifier>
 CreateNetworkChangeNotifierIfNeeded() {
   // There is a global singleton net::NetworkChangeNotifier if NetworkService
@@ -87,6 +95,26 @@ std::unique_ptr<net::HostResolver> CreateHostResolver(net::NetLog* net_log) {
   return std::move(remapped_host_resolver);
 }
 
+// This is duplicated in content/browser/loader/resource_dispatcher_host_impl.cc
+bool LoadInfoIsMoreInteresting(const mojom::LoadInfo& a,
+                               const mojom::LoadInfo& b) {
+  // Set |*_uploading_size| to be the size of the corresponding upload body if
+  // it's currently being uploaded.
+
+  uint64_t a_uploading_size = 0;
+  if (a.load_state == net::LOAD_STATE_SENDING_REQUEST)
+    a_uploading_size = a.upload_size;
+
+  uint64_t b_uploading_size = 0;
+  if (b.load_state == net::LOAD_STATE_SENDING_REQUEST)
+    b_uploading_size = b.upload_size;
+
+  if (a_uploading_size != b_uploading_size)
+    return a_uploading_size > b_uploading_size;
+
+  return a.load_state > b.load_state;
+}
+
 }  // namespace
 
 NetworkService::NetworkService(
@@ -405,6 +433,11 @@ net::HttpAuthHandlerFactory* NetworkService::GetHttpAuthHandlerFactory() {
   return http_auth_handler_factory_.get();
 }
 
+void NetworkService::OnBeforeURLRequest() {
+  if (base::FeatureList::IsEnabled(features::kNetworkService))
+    MaybeStartUpdateLoadInfoTimer();
+}
+
 certificate_transparency::STHReporter* NetworkService::sth_reporter() {
   return sth_distributor_.get();
 }
@@ -451,6 +484,92 @@ void NetworkService::OnNetworkContextConnectionClosed(
   owned_network_contexts_.erase(it);
 }
 
+void NetworkService::MaybeStartUpdateLoadInfoTimer() {
+  if (waiting_on_load_state_ack_ || update_load_info_timer_.IsRunning())
+    return;
+
+  bool has_loader = false;
+  for (auto* network_context : network_contexts_) {
+    if (!network_context->url_request_context()->url_requests()->empty()) {
+      has_loader = true;
+      break;
+    }
+  }
+
+  if (!has_loader)
+    return;
+
+  update_load_info_timer_.Start(FROM_HERE, kUpdateLoadStatesInterval, this,
+                                &NetworkService::UpdateLoadInfo);
+}
+
+void NetworkService::UpdateLoadInfo() {
+  // For requests from the same {process_id, routing_id} pair, pick the most
+  // important. For ones from the browser, return all of them.
+  std::vector<mojom::LoadInfoPtr> infos;
+  std::map<std::pair<uint32_t, uint32_t>, mojom::LoadInfoPtr> frame_infos;
+
+  for (auto* network_context : network_contexts_) {
+    for (auto* loader :
+         *network_context->url_request_context()->url_requests()) {
+      auto* url_loader = URLLoader::ForRequest(*loader);
+      if (!url_loader)
+        continue;
+
+      auto process_id = url_loader->GetProcessId();
+      auto routing_id = url_loader->GetRenderFrameId();
+      if (routing_id == static_cast<uint32_t>(MSG_ROUTING_NONE)) {
+        // If there is no routing_id, then the browser can't associate this with
+        // a page so no need to send.
+        continue;
+      }
+
+      auto load_info = mojom::LoadInfo::New();
+      load_info->process_id = process_id;
+      load_info->routing_id = routing_id;
+      load_info->host = loader->url().host();
+      auto load_state = loader->GetLoadState();
+      load_info->load_state = static_cast<uint32_t>(load_state.state);
+      load_info->state_param = std::move(load_state.param);
+      auto upload_progress = loader->GetUploadProgress();
+      load_info->upload_size = upload_progress.size();
+      load_info->upload_position = upload_progress.position();
+
+      if (process_id == 0) {
+        // Requests from the browser can't be compared to ones from child
+        // processes, so send them all without looking for the most interesting.
+        infos.push_back(std::move(load_info));
+        continue;
+      }
+
+      auto key = std::make_pair(process_id, routing_id);
+      auto existing = frame_infos.find(key);
+      if (existing == frame_infos.end() ||
+          LoadInfoIsMoreInteresting(*load_info, *existing->second)) {
+        frame_infos[key] = std::move(load_info);
+      }
+    }
+  }
+
+  for (auto& it : frame_infos)
+    infos.push_back(std::move(it.second));
+
+  if (infos.empty())
+    return;
+
+  DCHECK(!waiting_on_load_state_ack_);
+  waiting_on_load_state_ack_ = true;
+  client_->OnLoadingStateUpdate(
+      std::move(infos), base::BindOnce(&NetworkService::AckUpdateLoadInfo,
+                                       base::Unretained(this)));
+}
+
+void NetworkService::AckUpdateLoadInfo() {
+  DCHECK(waiting_on_load_state_ack_);
+  waiting_on_load_state_ack_ = false;
+  MaybeStartUpdateLoadInfoTimer();
+}
+
 void NetworkService::Bind(mojom::NetworkServiceRequest request) {
   DCHECK(!binding_.is_bound());
   binding_.Bind(std::move(request));

services/network/network_service.h

@@ -14,6 +14,7 @@
 #include "base/macros.h"
 #include "base/memory/scoped_refptr.h"
 #include "base/optional.h"
+#include "base/time/time.h"
 #include "build/build_config.h"
 #include "mojo/public/cpp/bindings/binding.h"
 #include "net/http/http_auth_preferences.h"
@@ -147,6 +148,9 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
   // one if needed.
   net::HttpAuthHandlerFactory* GetHttpAuthHandlerFactory();
 
+  // Notification that a URLLoader is about to start.
+  void OnBeforeURLRequest();
+
   bool quic_disabled() const { return quic_disabled_; }
   bool HasRawHeadersAccess(uint32_t process_id) const;
 
@@ -181,6 +185,19 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
   // context.
   void OnNetworkContextConnectionClosed(NetworkContext* network_context);
 
+  // Starts the timer to call NetworkServiceClient::OnLoadingStateUpdate(), if
+  // timer isn't already running, |waiting_on_load_state_ack_| is false, and
+  // there are live URLLoaders.
+  // Only works when network service is enabled.
+  void MaybeStartUpdateLoadInfoTimer();
+
+  // Checks all pending requests and updates the load info if necessary.
+  void UpdateLoadInfo();
+
+  // Invoked once the browser has acknowledged receiving the previous LoadInfo.
+  // Starts timer call UpdateLoadInfo() again, if needed.
+  void AckUpdateLoadInfo();
+
   std::unique_ptr<MojoNetLog> owned_net_log_;
   // TODO(https://crbug.com/767450): Remove this, once Chrome no longer creates
   // its own NetLog.
@@ -237,6 +254,14 @@ class COMPONENT_EXPORT(NETWORK_SERVICE) NetworkService
 
   std::unique_ptr<certificate_transparency::STHDistributor> sth_distributor_;
 
+  // A timer that periodically calls UpdateLoadInfo while there are pending
+  // loads and not waiting on an ACK from the client for the last sent
+  // LoadInfo callback.
+  base::OneShotTimer update_load_info_timer_;
+  // True if a LoadInfoList has been sent to the client, but has yet to be
+  // acknowledged.
+  bool waiting_on_load_state_ack_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(NetworkService);
 };
 

services/network/network_service_unittest.cc

@@ -967,6 +967,11 @@ class AllowBadCertsNetworkServiceClient : public mojom::NetworkServiceClient {
     NOTREACHED();
   }
 
+  void OnLoadingStateUpdate(std::vector<mojom::LoadInfoPtr> infos,
+                            OnLoadingStateUpdateCallback callback) override {
+    NOTREACHED();
+  }
+
  private:
   mojo::Binding<mojom::NetworkServiceClient> binding_;
 

services/network/public/mojom/network_service.mojom

@@ -5,7 +5,8 @@
 module network.mojom;
 
 import "mojo/public/mojom/base/file.mojom";
-import "mojo/public/mojom/base/file_path.mojom";
+import "mojo/public/mojom/base/file_path.mojom";
+import "mojo/public/mojom/base/string16.mojom";
 import "mojo/public/mojom/base/values.mojom";
 import "services/network/public/mojom/cookie_manager.mojom";
 import "services/network/public/mojom/network_change_manager.mojom";
@@ -40,6 +41,16 @@ interface AuthChallengeResponder {
    OnAuthCredentials(AuthCredentials? credentials);
 };
 
+struct LoadInfo {
+  uint32 process_id;
+  uint32 routing_id;
+  string host;
+  uint32 load_state;  // net::LoadState enum
+  mojo_base.mojom.String16 state_param;
+  uint64 upload_position;
+  uint64 upload_size;
+};
+
 // Network service interface to the browser.
 interface NetworkServiceClient {
   // Called when we receive an authentication failure.
@@ -109,6 +120,11 @@ interface NetworkServiceClient {
       int32 process_id, int32 routing_id, url.mojom.Url url,
       url.mojom.Url frame_url, array<CanonicalCookie> cookie_list,
       bool blocked_by_policy);
+
+  // Called periodically to update the client about progress of the current
+  // loads. To avoid flooding the client, it has to ack the update before it can
+  // receive the next update.
+  OnLoadingStateUpdate(array<LoadInfo> infos) => ();
 };
 
 // An HTTPS server to send DNS queries to, per the DNS Queries over HTTPS spec.

services/network/test/test_network_service_client.cc

@@ -104,4 +104,8 @@ void TestNetworkServiceClient::OnFileUploadRequested(
   std::move(callback).Run(net::OK, std::move(files));
 }
 
+void TestNetworkServiceClient::OnLoadingStateUpdate(
+    std::vector<mojom::LoadInfoPtr> infos,
+    OnLoadingStateUpdateCallback callback) {}
+
 }  // namespace network

services/network/test/test_network_service_client.h

@@ -65,6 +65,8 @@ class TestNetworkServiceClient : public network::mojom::NetworkServiceClient {
                              bool async,
                              const std::vector<base::FilePath>& file_paths,
                              OnFileUploadRequestedCallback callback) override;
+  void OnLoadingStateUpdate(std::vector<mojom::LoadInfoPtr> infos,
+                            OnLoadingStateUpdateCallback callback) override;
 
  private:
   bool enable_uploads_;

services/network/url_loader_unittest.cc

@@ -1982,6 +1982,11 @@ class TestAuthNetworkServiceClient : public mojom::NetworkServiceClient {
     NOTREACHED();
   }
 
+  void OnLoadingStateUpdate(std::vector<mojom::LoadInfoPtr> infos,
+                            OnLoadingStateUpdateCallback callback) override {
+    NOTREACHED();
+  }
+
   void set_credentials_response(CredentialsResponse credentials_response) {
     credentials_response_ = credentials_response;
   }

testing/buildbot/filters/mojo.fyi.network_content_browsertests.filter

@@ -37,10 +37,6 @@
 # https://crbug.com/827318
 -RenderThreadImplBrowserTest.NonResourceDispatchIPCTasksDontGoThroughScheduler
 
-# Network service path needs some way to update the per-WebContents load state.
-# https://crbug.com/819663.
--WebContentsImplBrowserTest.UpdateLoadState
-
 # NOTE: if adding an exclusion for an existing failure (e.g. additional test for
 # feature X that is already not working), please add it beside the existing
 # failures. Otherwise please reach out to network-service-dev@.