NetworkService: Fix WebViewTest.ClearDataCache

This patch does 3 things to fix the test: 1. Register listeners before loading the actual guest page. See bug for more background. 2. Add |was_fetched_via_cache| to |ResourceResponseInfo|. 3. Call |NetworkContext::ClearHttpCache()| in |WebViewGuest::ClearData()| when Network Service was enabled. We cannot use |BrowsingDataRemover| because it doesn't support non-default |StoragePartition|. Bug: 878060 Cq-Include-Trybots: luci.chromium.try:linux_mojo Change-Id: I5f0ea8e62ee8f44944428cfb041f9d4e427bd67c Reviewed-on: https://chromium-review.googlesource.com/1195596 Reviewed-by: Tom Sepez <tsepez@chromium.org> Reviewed-by: Ken Rockot <rockot@chromium.org> Reviewed-by: Clark DuVall <cduvall@chromium.org> Commit-Queue: Chong Zhang <chongz@chromium.org> Cr-Commit-Position: refs/heads/master@{#587753}
FairyWorld · Aug 30, 2018 · b94164a · b94164a
1 parent ab1e95f
commit b94164a
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 22 deletions.
diff --git a/chrome/test/data/extensions/platform_apps/web_view/clear_data_cache/embedder.js b/chrome/test/data/extensions/platform_apps/web_view/clear_data_cache/embedder.js
@@ -96,9 +96,16 @@ ClearDataTester.prototype.testClearDataCache = function() {
   this.webview_.request.onResponseStarted.addListener(
       responseStartedHandler, {urls: ['<all_urls>']});
 
-  for (var i = 0; i < 5; ++i) {
-    this.requestXhrFromWebView_();
-  }
+  this.webview_.addEventListener('loadstop', () => {
+    for (var i = 0; i < 5; ++i) {
+      this.requestXhrFromWebView_();
+    }
+  });
+
+  // Load the actual guest URL to start the test.
+  var guestURL = this.webview_.getAttribute('src').replace(
+                    '/empty_guest.html', '/guest.html');
+  this.webview_.setAttribute('src', guestURL);
 };
 
 var tester = new ClearDataTester();
@@ -117,24 +124,26 @@ window.runTest = function(testName) {
 };
 // window.* exported functions end.
 
-function setUpTest(guestURL, doneCallback) {
+function setUpTest(emptyGuestURL, doneCallback) {
   var webview = document.createElement('webview');
 
-  webview.onloadstop = function(e) {
+  var listener = function(e) {
+    webview.removeEventListener('loadstop', listener);
     LOG('webview has loaded.');
     doneCallback(webview);
   };
+  webview.addEventListener('loadstop', listener);
 
-  webview.setAttribute('src', guestURL);
+  webview.setAttribute('src', emptyGuestURL);
   document.body.appendChild(webview);
 }
 
 onload = function() {
   chrome.test.getConfig(function(config) {
     LOG('config: ' + config.testServer.port);
-    var guestURL = 'http://localhost:' + config.testServer.port +
-        '/extensions/platform_apps/web_view/clear_data_cache/guest.html';
-    setUpTest(guestURL, function(webview) {
+    var emptyGuestURL = 'http://localhost:' + config.testServer.port +
+        '/extensions/platform_apps/web_view/clear_data_cache/empty_guest.html';
+    setUpTest(emptyGuestURL, function(webview) {
       LOG('Guest load completed.');
       //chrome.test.sendMessage('WebViewTest.LAUNCHED');
       chrome.test.sendMessage('Launched');

diff --git a/chrome/test/data/extensions/platform_apps/web_view/clear_data_cache/empty_guest.html b/chrome/test/data/extensions/platform_apps/web_view/clear_data_cache/empty_guest.html
@@ -0,0 +1,13 @@
+<!--
+ * Copyright 2018 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.
+-->
+<!doctype html>
+<html>
+<head>
+  <title>A simple guest that does nothing.</title>
+</head>
+<body>
+</body>
+</html>
diff --git a/extensions/browser/api/web_request/web_request_info.cc b/extensions/browser/api/web_request/web_request_info.cc
@@ -350,11 +350,7 @@ void WebRequestInfo::AddResponseInfoFromResourceResponse(
   if (response_headers)
     response_code = response_headers->response_code();
   response_ip = response.socket_address.host();
-
-  // TODO(https://crbug.com/721414): We have no apparent source for this
-  // information yet in the Network Service case. Should indicate whether or not
-  // the response data came from cache.
-  response_from_cache = false;
+  response_from_cache = response.was_fetched_via_cache;
 }
 
 void WebRequestInfo::InitializeWebViewAndFrameData(

diff --git a/extensions/browser/guest_view/web_view/web_view_guest.cc b/extensions/browser/guest_view/web_view/web_view_guest.cc
@@ -61,6 +61,7 @@
 #include "net/base/escape.h"
 #include "net/base/net_errors.h"
 #include "net/cookies/canonical_cookie.h"
+#include "services/network/public/cpp/features.h"
 #include "ui/base/models/simple_menu_model.h"
 #include "ui/events/keycodes/keyboard_codes.h"
 #include "url/url_constants.h"
@@ -755,13 +756,24 @@ bool WebViewGuest::ClearData(base::Time remove_since,
     web_cache::WebCacheManager::GetInstance()->ClearCacheForProcess(
         render_process_id);
 
-    base::Closure cache_removal_done_callback = base::Bind(
+    base::OnceClosure cache_removal_done_callback = base::BindOnce(
         &WebViewGuest::ClearDataInternal, weak_ptr_factory_.GetWeakPtr(),
         remove_since, removal_mask, callback);
-    // components/, move |ClearCache| to WebViewGuest: http//crbug.com/471287.
-    partition->ClearHttpAndMediaCaches(remove_since, base::Time::Now(),
-                                       base::Callback<bool(const GURL&)>(),
-                                       cache_removal_done_callback);
+
+    // We cannot use |BrowsingDataRemover| here since it doesn't support
+    // non-default StoragePartition.
+    if (base::FeatureList::IsEnabled(network::features::kNetworkService)) {
+      // Note that we've deprecated the concept of a media cache, and are now
+      // using a single cache for both purposes.
+      partition->GetNetworkContext()->ClearHttpCache(
+          remove_since, base::Time::Now(), nullptr /* ClearDataFilter */,
+          std::move(cache_removal_done_callback));
+    } else {
+      partition->ClearHttpAndMediaCaches(
+          remove_since, base::Time::Now(),
+          base::RepeatingCallback<bool(const GURL&)>(),
+          std::move(cache_removal_done_callback));
+    }
     return true;
   }
 

diff --git a/services/network/public/cpp/network_ipc_param_traits.h b/services/network/public/cpp/network_ipc_param_traits.h
@@ -191,6 +191,7 @@ IPC_STRUCT_TRAITS_BEGIN(network::ResourceResponseInfo)
   IPC_STRUCT_TRAITS_MEMBER(connection_info)
   IPC_STRUCT_TRAITS_MEMBER(alpn_negotiated_protocol)
   IPC_STRUCT_TRAITS_MEMBER(socket_address)
+  IPC_STRUCT_TRAITS_MEMBER(was_fetched_via_cache)
   IPC_STRUCT_TRAITS_MEMBER(was_fetched_via_service_worker)
   IPC_STRUCT_TRAITS_MEMBER(was_fallback_required_by_service_worker)
   IPC_STRUCT_TRAITS_MEMBER(url_list_via_service_worker)

diff --git a/services/network/public/cpp/resource_response.cc b/services/network/public/cpp/resource_response.cc
@@ -38,6 +38,7 @@ scoped_refptr<ResourceResponse> ResourceResponse::DeepCopy() const {
   new_response->head.connection_info = head.connection_info;
   new_response->head.alpn_negotiated_protocol = head.alpn_negotiated_protocol;
   new_response->head.socket_address = head.socket_address;
+  new_response->head.was_fetched_via_cache = head.was_fetched_via_cache;
   new_response->head.was_fetched_via_service_worker =
       head.was_fetched_via_service_worker;
   new_response->head.was_fallback_required_by_service_worker =

diff --git a/services/network/public/cpp/resource_response_info.h b/services/network/public/cpp/resource_response_info.h
@@ -108,6 +108,9 @@ struct COMPONENT_EXPORT(NETWORK_CPP_BASE) ResourceResponseInfo {
   // Remote address of the socket which fetched this resource.
   net::HostPortPair socket_address;
 
+  // True if the response came from cache.
+  bool was_fetched_via_cache = false;
+
   // True if the response was delivered through a proxy.
   bool was_fetched_via_proxy;
 

diff --git a/services/network/url_loader.cc b/services/network/url_loader.cc
@@ -66,6 +66,7 @@ void PopulateResourceResponse(net::URLRequest* request,
       response_info.alpn_negotiated_protocol;
   response->head.connection_info = response_info.connection_info;
   response->head.socket_address = response_info.socket_address;
+  response->head.was_fetched_via_cache = request->was_cached();
   response->head.was_fetched_via_proxy = request->was_fetched_via_proxy();
   response->head.network_accessed = response_info.network_accessed;
   response->head.async_revalidation_requested =

diff --git a/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter b/testing/buildbot/filters/mojo.fyi.network_browser_tests.filter
@@ -61,9 +61,6 @@
 -NetInternalsTest.netInternalsTimelineViewZoomOut
 -NetInternalsTest.netInternalsTourTabs
 
-# https://crbug.com/721398
--WebViewTest.ClearDataCache
-
 # https://crbug.com/853256
 -ClientHintsBrowserTest.ClientHintsLifetimeNotAttachedCookiesBlocked/0
 -ClientHintsBrowserTest.ClientHintsLifetimeNotAttachedCookiesBlocked/1