Fill in the network isolation key for server push

Only support it when socket pools are also split by the
NIK, because the key is grabbed from the session key and
will be empty when socket pools are not partitioned.

Bug: 1021617
Change-Id: Ib0f601b8b20deb2c4bb85ddb4e212eb71380d086
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1900556Reviewed-by: Zhongyi Shi <zhongyi@chromium.org>
Commit-Queue: Yao Xiao <yaoxia@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719321}

net/http/http_cache_lookup_manager.cc

@@ -45,6 +45,7 @@ int HttpCacheLookupManager::LookupTransaction::StartLookup(
   });
 
   request_->url = push_helper_->GetURL();
+  request_->network_isolation_key = push_helper_->GetNetworkIsolationKey();
   request_->method = "GET";
   request_->load_flags = LOAD_ONLY_FROM_CACHE | LOAD_SKIP_CACHE_VALIDATION;
   cache->CreateTransaction(DEFAULT_PRIORITY, &transaction_);

net/http/http_cache_lookup_manager_unittest.cc

@@ -16,6 +16,7 @@
 #include "net/http/mock_http_cache.h"
 #include "net/test/gtest_util.h"
 #include "testing/gmock/include/gmock/gmock.h"
+#include "testing/gtest/include/gtest/gtest-param-test.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 using net::test::IsOk;
@@ -26,14 +27,27 @@ namespace {
 
 class MockServerPushHelper : public ServerPushDelegate::ServerPushHelper {
  public:
-  explicit MockServerPushHelper(const GURL& url) : request_url_(url) {}
+  explicit MockServerPushHelper(const GURL& url)
+      : request_url_(url),
+        network_isolation_key_(url::Origin::Create(url),
+                               url::Origin::Create(url)) {}
 
   const GURL& GetURL() const override { return request_url_; }
 
+  NetworkIsolationKey GetNetworkIsolationKey() const override {
+    return network_isolation_key_;
+  }
+
+  void set_network_isolation_key(
+      const net::NetworkIsolationKey& network_isolation_key) {
+    network_isolation_key_ = network_isolation_key;
+  }
+
   MOCK_METHOD0(Cancel, void());
 
  private:
   const GURL request_url_;
+  NetworkIsolationKey network_isolation_key_;
 };
 
 std::unique_ptr<MockTransaction> CreateMockTransaction(const GURL& url) {
@@ -136,13 +150,17 @@ TEST(HttpCacheLookupManagerTest, ServerPushDoNotCreateCacheEntry) {
   EXPECT_EQ(0, mock_cache.disk_cache()->create_count());
 }
 
-TEST(HttpCacheLookupManagerTest, ServerPushHitCache) {
-  // Skip test if split cache is enabled, as it breaks push.
-  // crbug.com/1009619
-  if (base::FeatureList::IsEnabled(
-          net::features::kSplitCacheByNetworkIsolationKey)) {
-    return;
-  }
+// Parameterized by whether the network isolation key are the same for the
+// server push and corresponding cache entry.
+class HttpCacheLookupManagerTest_NetworkIsolationKey
+    : public ::testing::Test,
+      public ::testing::WithParamInterface<
+          bool /* use_same_network_isolation_key */> {};
+
+TEST_P(HttpCacheLookupManagerTest_NetworkIsolationKey, ServerPushCacheStatus) {
+  bool use_same_network_isolation_key = GetParam();
+  bool split_cache_enabled = base::FeatureList::IsEnabled(
+      net::features::kSplitCacheByNetworkIsolationKey);
 
   base::test::TaskEnvironment task_environment;
   MockHttpCache mock_cache;
@@ -163,32 +181,41 @@ TEST(HttpCacheLookupManagerTest, ServerPushHitCache) {
 
   std::unique_ptr<MockServerPushHelper> push_helper =
       std::make_unique<MockServerPushHelper>(request_url);
+  if (!use_same_network_isolation_key) {
+    url::Origin origin = url::Origin::Create(GURL("http://www.abc.com"));
+    push_helper->set_network_isolation_key(
+        net::NetworkIsolationKey(origin, origin));
+  }
+
   MockServerPushHelper* push_helper_ptr = push_helper.get();
 
-  // Receive a server push and should cancel the push.
-  EXPECT_CALL(*push_helper_ptr, Cancel()).Times(1);
+  int expected_cancel_times =
+      use_same_network_isolation_key || !split_cache_enabled ? 1 : 0;
+  EXPECT_CALL(*push_helper_ptr, Cancel()).Times(expected_cancel_times);
   push_delegate.OnPush(std::move(push_helper), NetLogWithSource());
   base::RunLoop().RunUntilIdle();
 
   // Make sure no new net layer transaction is created.
   EXPECT_EQ(1, mock_cache.network_layer()->transaction_count());
-  EXPECT_EQ(1, mock_cache.disk_cache()->open_count());
+
+  int expected_open_count =
+      use_same_network_isolation_key || !split_cache_enabled ? 1 : 0;
+  EXPECT_EQ(expected_open_count, mock_cache.disk_cache()->open_count());
+
   EXPECT_EQ(1, mock_cache.disk_cache()->create_count());
 
   RemoveMockTransaction(mock_trans.get());
 }
 
+INSTANTIATE_TEST_SUITE_P(
+    /* no prefix */,
+    HttpCacheLookupManagerTest_NetworkIsolationKey,
+    ::testing::Bool());
+
 // Test when a server push is received while the HttpCacheLookupManager has a
 // pending lookup transaction for the same URL, the new server push will not
 // send a new lookup transaction and should not be canceled.
 TEST(HttpCacheLookupManagerTest, ServerPushPendingLookup) {
-  // Skip test if split cache is enabled, as it breaks push.
-  // crbug.com/1009619
-  if (base::FeatureList::IsEnabled(
-          net::features::kSplitCacheByNetworkIsolationKey)) {
-    return;
-  }
-
   base::test::TaskEnvironment task_environment;
   MockHttpCache mock_cache;
   HttpCacheLookupManager push_delegate(mock_cache.http_cache());
@@ -234,13 +261,6 @@ TEST(HttpCacheLookupManagerTest, ServerPushPendingLookup) {
 
 // Test the server push lookup is based on the full url.
 TEST(HttpCacheLookupManagerTest, ServerPushLookupOnUrl) {
-  // Skip test if split cache is enabled, as it breaks push.
-  // crbug.com/1009619
-  if (base::FeatureList::IsEnabled(
-          net::features::kSplitCacheByNetworkIsolationKey)) {
-    return;
-  }
-
   base::test::TaskEnvironment task_environment;
   MockHttpCache mock_cache;
   HttpCacheLookupManager push_delegate(mock_cache.http_cache());

net/quic/quic_chromium_client_session.cc

@@ -286,6 +286,13 @@ class QuicServerPushHelper : public ServerPushDelegate::ServerPushHelper {
 
   const GURL& GetURL() const override { return request_url_; }
 
+  NetworkIsolationKey GetNetworkIsolationKey() const override {
+    if (session_) {
+      return session_->quic_session_key().network_isolation_key();
+    }
+    return NetworkIsolationKey();
+  }
+
  private:
   base::WeakPtr<QuicChromiumClientSession> session_;
   const GURL request_url_;

net/spdy/server_push_delegate.h

@@ -27,6 +27,9 @@ class NET_EXPORT_PRIVATE ServerPushDelegate {
 
     // Gets the URL of the pushed request.
     virtual const GURL& GetURL() const = 0;
+
+    // Gets the network isolation key for the pushed request.
+    virtual NetworkIsolationKey GetNetworkIsolationKey() const = 0;
   };
 
   virtual ~ServerPushDelegate() {}

net/spdy/spdy_session.cc

@@ -471,6 +471,13 @@ class SpdyServerPushHelper : public ServerPushDelegate::ServerPushHelper {
 
   const GURL& GetURL() const override { return request_url_; }
 
+  NetworkIsolationKey GetNetworkIsolationKey() const override {
+    if (session_) {
+      return session_->spdy_session_key().network_isolation_key();
+    }
+    return NetworkIsolationKey();
+  }
+
  private:
   base::WeakPtr<SpdySession> session_;
   const GURL request_url_;

net/url_request/url_request_quic_unittest.cc

@@ -4,11 +4,13 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
+#include "base/feature_list.h"
 #include "base/files/file_path.h"
 #include "base/macros.h"
 #include "base/run_loop.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/stringprintf.h"
+#include "net/base/features.h"
 #include "net/base/load_timing_info.h"
 #include "net/base/network_delegate.h"
 #include "net/cert/mock_cert_verifier.h"
@@ -267,6 +269,19 @@ TEST_F(URLRequestQuicTest, TestGetRequest) {
 }
 
 TEST_F(URLRequestQuicTest, CancelPushIfCached_SomeCached) {
+  // Skip test if "split cache" is enabled while "partition connections" is
+  // disabled, as it breaks push.
+  if (base::FeatureList::IsEnabled(
+          net::features::kSplitCacheByNetworkIsolationKey) &&
+      !base::FeatureList::IsEnabled(
+          net::features::kPartitionConnectionsByNetworkIsolationKey)) {
+    return;
+  }
+
+  const url::Origin kOrigin1 =
+      url::Origin::Create(GURL("http://www.example.com"));
+  const NetworkIsolationKey kTestNetworkIsolationKey(kOrigin1, kOrigin1);
+
   Init();
 
   // Send a request to the pushed url: /kitten-1.jpg to pull the resource into
@@ -277,6 +292,7 @@ TEST_F(URLRequestQuicTest, CancelPushIfCached_SomeCached) {
   std::unique_ptr<URLRequest> request_0 =
       CreateRequest(GURL(url_0), DEFAULT_PRIORITY, &delegate_0);
 
+  request_0->set_network_isolation_key(kTestNetworkIsolationKey);
   request_0->Start();
   ASSERT_TRUE(request_0->is_pending());
 
@@ -295,6 +311,7 @@ TEST_F(URLRequestQuicTest, CancelPushIfCached_SomeCached) {
   std::unique_ptr<URLRequest> request =
       CreateRequest(GURL(url), DEFAULT_PRIORITY, &delegate);
 
+  request->set_network_isolation_key(kTestNetworkIsolationKey);
   request->Start();
   ASSERT_TRUE(request->is_pending());
 
@@ -340,6 +357,19 @@ TEST_F(URLRequestQuicTest, CancelPushIfCached_SomeCached) {
 }
 
 TEST_F(URLRequestQuicTest, CancelPushIfCached_AllCached) {
+  // Skip test if "split cache" is enabled while "partition connections" is
+  // disabled, as it breaks push.
+  if (base::FeatureList::IsEnabled(
+          net::features::kSplitCacheByNetworkIsolationKey) &&
+      !base::FeatureList::IsEnabled(
+          net::features::kPartitionConnectionsByNetworkIsolationKey)) {
+    return;
+  }
+
+  const url::Origin kOrigin1 =
+      url::Origin::Create(GURL("http://www.example.com"));
+  const NetworkIsolationKey kTestNetworkIsolationKey(kOrigin1, kOrigin1);
+
   Init();
 
   // Send a request to the pushed url: /kitten-1.jpg to pull the resource into
@@ -350,6 +380,7 @@ TEST_F(URLRequestQuicTest, CancelPushIfCached_AllCached) {
   std::unique_ptr<URLRequest> request_0 =
       CreateRequest(GURL(url_0), DEFAULT_PRIORITY, &delegate_0);
 
+  request_0->set_network_isolation_key(kTestNetworkIsolationKey);
   request_0->Start();
   ASSERT_TRUE(request_0->is_pending());
 
@@ -368,6 +399,7 @@ TEST_F(URLRequestQuicTest, CancelPushIfCached_AllCached) {
   std::unique_ptr<URLRequest> request_1 =
       CreateRequest(GURL(url_1), DEFAULT_PRIORITY, &delegate_1);
 
+  request_1->set_network_isolation_key(kTestNetworkIsolationKey);
   request_1->Start();
   ASSERT_TRUE(request_1->is_pending());
 
@@ -386,6 +418,7 @@ TEST_F(URLRequestQuicTest, CancelPushIfCached_AllCached) {
   std::unique_ptr<URLRequest> request =
       CreateRequest(GURL(url), DEFAULT_PRIORITY, &delegate);
 
+  request->set_network_isolation_key(kTestNetworkIsolationKey);
   request->Start();
   ASSERT_TRUE(request->is_pending());