Pass NetworkIsolationKey to ReportingCache from ReportingHeaderParser

ReportingCache is now told what the NIK of each parsed header is.
This will eventually be passed to the ReportingHeaderParser by the
caller and will correspond to the NIK of the response sending the
header.

The NIK fields are still empty for now, except in tests.

Bug: 993805
Change-Id: I93c30932559338c6ff2173d7ecbedf3541a0d35e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2135829
Commit-Queue: Lily Chen <chlily@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756882}

net/reporting/reporting_cache.h

@@ -128,8 +128,8 @@ class NET_EXPORT ReportingCache {
   // Adds a new client to the cache for |origin|, or updates the existing one
   // to match the new header. All values are assumed to be valid as they have
   // passed through the ReportingHeaderParser.
-  // TODO(chlily): Take NetworkIsolationKey.
   virtual void OnParsedHeader(
+      const NetworkIsolationKey& network_isolation_key,
       const url::Origin& origin,
       std::vector<ReportingEndpointGroup> parsed_header) = 0;
 

net/reporting/reporting_cache_impl.cc

@@ -215,12 +215,11 @@ bool ReportingCacheImpl::IsReportDoomedForTesting(
 }
 
 void ReportingCacheImpl::OnParsedHeader(
+    const NetworkIsolationKey& network_isolation_key,
     const url::Origin& origin,
     std::vector<ReportingEndpointGroup> parsed_header) {
   SanityCheckClients();
 
-  // TODO(chlily): Respect NetworkIsolationKey.
-  NetworkIsolationKey network_isolation_key = NetworkIsolationKey::Todo();
   Client new_client(network_isolation_key, origin);
   base::Time now = clock().Now();
   new_client.last_used = now;
@@ -234,11 +233,8 @@ void ReportingCacheImpl::OnParsedHeader(
     // Creates an endpoint group and sets its |last_used| to |now|.
     CachedReportingEndpointGroup new_group(parsed_endpoint_group, now);
 
-    // TODO(chlily): This DCHECK passes right now because the groups have their
-    // NIK set to an empty NIK by the header parser, and we also set the
-    // client's NIK to an empty NIK above. Eventually it should pass because the
-    // header parser should provide the NIK it used for the groups so that the
-    // client can be created using the same NIK.
+    // Consistency check: the new client should have the same NIK and origin as
+    // all groups parsed from this header.
     DCHECK_EQ(new_group.group_key.network_isolation_key,
               new_client.network_isolation_key);
     DCHECK_EQ(new_group.group_key.origin, new_client.origin);

net/reporting/reporting_cache_impl.h

@@ -65,6 +65,7 @@ class ReportingCacheImpl : public ReportingCache {
   bool IsReportPendingForTesting(const ReportingReport* report) const override;
   bool IsReportDoomedForTesting(const ReportingReport* report) const override;
   void OnParsedHeader(
+      const NetworkIsolationKey& network_isolation_key,
       const url::Origin& origin,
       std::vector<ReportingEndpointGroup> parsed_header) override;
   std::set<url::Origin> GetAllOrigins() const override;

net/reporting/reporting_endpoint_manager_unittest.cc

@@ -103,6 +103,7 @@ class TestReportingCache : public ReportingCache {
     return false;
   }
   void OnParsedHeader(
+      const NetworkIsolationKey& network_isolation_key,
       const url::Origin& origin,
       std::vector<ReportingEndpointGroup> parsed_header) override {
     NOTREACHED();

net/reporting/reporting_header_parser.cc

@@ -241,7 +241,9 @@ void ReportingHeaderParser::RecordHeaderDiscardedForJsonTooBig() {
 }
 
 // static
-void ReportingHeaderParser::ParseHeader(ReportingContext* context,
+void ReportingHeaderParser::ParseHeader(
+    ReportingContext* context,
+    const NetworkIsolationKey& network_isolation_key,
     const GURL& url,
     std::unique_ptr<base::Value> value) {
   DCHECK(url.SchemeIsCryptographic());
@@ -254,7 +256,6 @@ void ReportingHeaderParser::ParseHeader(ReportingContext* context,
   ReportingCache* cache = context->cache();
 
   url::Origin origin = url::Origin::Create(url);
-  NetworkIsolationKey network_isolation_key = NetworkIsolationKey::Todo();
 
   std::vector<ReportingEndpointGroup> parsed_header;
 
@@ -279,8 +280,8 @@ void ReportingHeaderParser::ParseHeader(ReportingContext* context,
     return;
   }
 
-  // TODO(chlily): Pass NIK to cache.
-  cache->OnParsedHeader(origin, std::move(parsed_header));
+  cache->OnParsedHeader(network_isolation_key, origin,
+                        std::move(parsed_header));
   RecordHeaderOutcome(HeaderOutcome::PARSED);
 }
 

net/reporting/reporting_header_parser.h

@@ -17,6 +17,7 @@ class Value;
 
 namespace net {
 
+class NetworkIsolationKey;
 class ReportingContext;
 
 class NET_EXPORT ReportingHeaderParser {
@@ -79,8 +80,8 @@ class NET_EXPORT ReportingHeaderParser {
   static void RecordHeaderDiscardedForJsonInvalid();
   static void RecordHeaderDiscardedForJsonTooBig();
 
-  // TODO(chlily): Pass in the NetworkIsolationKey.
   static void ParseHeader(ReportingContext* context,
+                          const NetworkIsolationKey& network_isolation_key,
                           const GURL& url,
                           std::unique_ptr<base::Value> value);
 

net/reporting/reporting_header_parser_fuzzer.cc

@@ -10,6 +10,7 @@
 #include "base/time/default_tick_clock.h"
 #include "base/time/time.h"
 #include "base/values.h"
+#include "net/base/network_isolation_key.h"
 #include "net/reporting/reporting_cache.h"
 #include "net/reporting/reporting_header_parser.h"
 #include "net/reporting/reporting_policy.pb.h"
@@ -38,7 +39,8 @@ void FuzzReportingHeaderParser(const std::string& data_json,
 
   // TODO: consider including proto definition for URL after moving that to
   // testing/libfuzzer/proto and creating a separate converter.
-  net::ReportingHeaderParser::ParseHeader(&context, GURL("https://origin/path"),
+  net::ReportingHeaderParser::ParseHeader(&context, net::NetworkIsolationKey(),
+                                          GURL("https://origin/path"),
                                           std::move(data_value));
   if (context.cache()->GetEndpointCount() == 0) {
     return;

net/reporting/reporting_service.cc

@@ -93,9 +93,10 @@ class ReportingServiceImpl : public ReportingService {
     }
 
     DVLOG(1) << "Received Reporting policy for " << url.GetOrigin();
-    DoOrBacklogTask(base::BindOnce(&ReportingServiceImpl::DoProcessHeader,
-                                   base::Unretained(this), url,
-                                   std::move(header_value)));
+    // TODO(chlily): Get the proper NetworkIsolationKey from the caller.
+    DoOrBacklogTask(base::BindOnce(
+        &ReportingServiceImpl::DoProcessHeader, base::Unretained(this),
+        NetworkIsolationKey::Todo(), url, std::move(header_value)));
   }
 
   void RemoveBrowsingData(int data_type_mask,
@@ -161,11 +162,12 @@ class ReportingServiceImpl : public ReportingService {
                                  0 /* attempts */);
   }
 
-  void DoProcessHeader(const GURL& url,
+  void DoProcessHeader(const NetworkIsolationKey& network_isolation_key,
+                       const GURL& url,
                        std::unique_ptr<base::Value> header_value) {
     DCHECK(initialized_);
-    ReportingHeaderParser::ParseHeader(context_.get(), url,
-                                       std::move(header_value));
+    ReportingHeaderParser::ParseHeader(context_.get(), network_isolation_key,
+                                       url, std::move(header_value));
   }
 
   void DoRemoveBrowsingData(