Reporting API: Remove Persister.

We've decided to persist using SQLite, so there's no periodic "persist
everything" task that needs to happen, so the ReportingPersister no
longer needs to exist.

Bug: 
Change-Id: If229bc8cc5b946385c1f2934021b553cc5ae48fc
Reviewed-on: https://chromium-review.googlesource.com/797852
Commit-Queue: Julia Tuttle <juliatuttle@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521356}

net/BUILD.gn

@@ -1833,8 +1833,6 @@ component("net") {
         "reporting/reporting_network_change_observer.h",
         "reporting/reporting_observer.cc",
         "reporting/reporting_observer.h",
-        "reporting/reporting_persister.cc",
-        "reporting/reporting_persister.h",
         "reporting/reporting_policy.cc",
         "reporting/reporting_policy.h",
         "reporting/reporting_report.cc",
@@ -5372,7 +5370,6 @@ test("net_unittests") {
       "reporting/reporting_garbage_collector_unittest.cc",
       "reporting/reporting_header_parser_unittest.cc",
       "reporting/reporting_network_change_observer_unittest.cc",
-      "reporting/reporting_persister_unittest.cc",
       "reporting/reporting_service_unittest.cc",
       "reporting/reporting_test_util.cc",
       "reporting/reporting_test_util.h",

net/reporting/reporting_context.cc

@@ -20,7 +20,6 @@
 #include "net/reporting/reporting_garbage_collector.h"
 #include "net/reporting/reporting_network_change_observer.h"
 #include "net/reporting/reporting_observer.h"
-#include "net/reporting/reporting_persister.h"
 #include "net/reporting/reporting_policy.h"
 #include "net/reporting/reporting_uploader.h"
 
@@ -80,7 +79,6 @@ ReportingContext::ReportingContext(const ReportingPolicy& policy,
       cache_(ReportingCache::Create(this)),
       endpoint_manager_(ReportingEndpointManager::Create(this)),
       delivery_agent_(ReportingDeliveryAgent::Create(this)),
-      persister_(ReportingPersister::Create(this)),
       garbage_collector_(ReportingGarbageCollector::Create(this)),
       network_change_observer_(ReportingNetworkChangeObserver::Create(this)) {}
 

net/reporting/reporting_context.h

@@ -27,7 +27,6 @@ class ReportingEndpointManager;
 class ReportingGarbageCollector;
 class ReportingNetworkChangeObserver;
 class ReportingObserver;
-class ReportingPersister;
 class ReportingUploader;
 class URLRequestContext;
 
@@ -57,8 +56,6 @@ class NET_EXPORT ReportingContext {
     return garbage_collector_.get();
   }
 
-  ReportingPersister* persister() { return persister_.get(); }
-
   void AddObserver(ReportingObserver* observer);
   void RemoveObserver(ReportingObserver* observer);
 
@@ -91,9 +88,6 @@ class NET_EXPORT ReportingContext {
   // |cache_|, and |endpoint_manager_|.
   std::unique_ptr<ReportingDeliveryAgent> delivery_agent_;
 
-  // |persister_| must come after |clock_|, |tick_clock_|, and |cache_|.
-  std::unique_ptr<ReportingPersister> persister_;
-
   // |garbage_collector_| must come after |tick_clock_| and |cache_|.
   std::unique_ptr<ReportingGarbageCollector> garbage_collector_;
 

net/reporting/reporting_persister.cc

-// Copyright 2017 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.
-
-#include "net/reporting/reporting_persister.h"
-
-#include <utility>
-#include <vector>
-
-#include "base/strings/string_number_conversions.h"
-#include "base/time/clock.h"
-#include "base/time/tick_clock.h"
-#include "base/time/time.h"
-#include "base/timer/timer.h"
-#include "base/values.h"
-#include "net/reporting/reporting_cache.h"
-#include "net/reporting/reporting_client.h"
-#include "net/reporting/reporting_context.h"
-#include "net/reporting/reporting_observer.h"
-#include "net/reporting/reporting_policy.h"
-#include "net/reporting/reporting_report.h"
-
-namespace net {
-namespace {
-
-std::unique_ptr<base::Value> SerializeOrigin(const url::Origin& origin) {
-  auto serialized = std::make_unique<base::DictionaryValue>();
-
-  serialized->SetString("scheme", origin.scheme());
-  serialized->SetString("host", origin.host());
-  serialized->SetInteger("port", origin.port());
-  serialized->SetString("suborigin", origin.suborigin());
-
-  return std::move(serialized);
-}
-
-bool DeserializeOrigin(const base::DictionaryValue& serialized,
-                       url::Origin* origin_out) {
-  std::string scheme;
-  if (!serialized.GetString("scheme", &scheme))
-    return false;
-
-  std::string host;
-  if (!serialized.GetString("host", &host))
-    return false;
-
-  int port_int;
-  if (!serialized.GetInteger("port", &port_int))
-    return false;
-  uint16_t port = static_cast<uint16_t>(port_int);
-  if (port_int != port)
-    return false;
-
-  std::string suborigin;
-  if (!serialized.GetString("suborigin", &suborigin))
-    return false;
-
-  *origin_out = url::Origin::CreateFromNormalizedTupleWithSuborigin(
-      scheme, host, port, suborigin);
-  return true;
-}
-
-class ReportingPersisterImpl : public ReportingPersister {
- public:
-  ReportingPersisterImpl(ReportingContext* context) : context_(context) {}
-
-  // ReportingPersister implementation:
-
-  ~ReportingPersisterImpl() override = default;
-
- private:
-  std::string SerializeTicks(base::TimeTicks time_ticks) {
-    base::Time time = time_ticks - tick_clock()->NowTicks() + clock()->Now();
-    return base::Int64ToString(time.ToInternalValue());
-  }
-
-  bool DeserializeTicks(const std::string& serialized,
-                        base::TimeTicks* time_ticks_out) {
-    int64_t internal;
-    if (!base::StringToInt64(serialized, &internal))
-      return false;
-
-    base::Time time = base::Time::FromInternalValue(internal);
-    *time_ticks_out = time - clock()->Now() + tick_clock()->NowTicks();
-    return true;
-  }
-
-  std::unique_ptr<base::Value> SerializeReport(const ReportingReport& report) {
-    auto serialized = std::make_unique<base::DictionaryValue>();
-
-    serialized->SetString("url", report.url.spec());
-    serialized->SetString("group", report.group);
-    serialized->SetString("type", report.type);
-    serialized->Set("body", report.body->CreateDeepCopy());
-    serialized->SetString("queued", SerializeTicks(report.queued));
-    serialized->SetInteger("attempts", report.attempts);
-
-    return std::move(serialized);
-  }
-
-  bool DeserializeReport(const base::DictionaryValue& report) {
-    std::string url_string;
-    if (!report.GetString("url", &url_string))
-      return false;
-    GURL url(url_string);
-    if (!url.is_valid())
-      return false;
-
-    std::string group;
-    if (!report.GetString("group", &group))
-      return false;
-
-    std::string type;
-    if (!report.GetString("type", &type))
-      return false;
-
-    const base::Value* body_original;
-    if (!report.Get("body", &body_original))
-      return false;
-    std::unique_ptr<base::Value> body = body_original->CreateDeepCopy();
-
-    std::string queued_string;
-    if (!report.GetString("queued", &queued_string))
-      return false;
-    base::TimeTicks queued;
-    if (!DeserializeTicks(queued_string, &queued))
-      return false;
-
-    int attempts;
-    if (!report.GetInteger("attempts", &attempts))
-      return false;
-    if (attempts < 0)
-      return false;
-
-    cache()->AddReport(url, group, type, std::move(body), queued, attempts);
-    return true;
-  }
-
-  std::unique_ptr<base::Value> SerializeReports() {
-    std::vector<const ReportingReport*> reports;
-    cache()->GetReports(&reports);
-
-    auto serialized = std::make_unique<base::ListValue>();
-    for (const ReportingReport* report : reports)
-      serialized->Append(SerializeReport(*report));
-
-    return std::move(serialized);
-  }
-
-  bool DeserializeReports(const base::ListValue& reports) {
-    for (size_t i = 0; i < reports.GetSize(); ++i) {
-      const base::DictionaryValue* report;
-      if (!reports.GetDictionary(i, &report))
-        return false;
-      if (!DeserializeReport(*report))
-        return false;
-    }
-
-    return true;
-  }
-
-  std::unique_ptr<base::Value> SerializeClient(const ReportingClient& client) {
-    auto serialized = std::make_unique<base::DictionaryValue>();
-
-    serialized->Set("origin", SerializeOrigin(client.origin));
-    serialized->SetString("endpoint", client.endpoint.spec());
-    serialized->SetBoolean(
-        "subdomains",
-        client.subdomains == ReportingClient::Subdomains::INCLUDE);
-    serialized->SetString("group", client.group);
-    serialized->SetString("expires", SerializeTicks(client.expires));
-
-    return std::move(serialized);
-  }
-
-  bool DeserializeClient(const base::DictionaryValue& client) {
-    const base::DictionaryValue* origin_value;
-    if (!client.GetDictionary("origin", &origin_value))
-      return false;
-    url::Origin origin;
-    if (!DeserializeOrigin(*origin_value, &origin))
-      return false;
-
-    std::string endpoint_string;
-    if (!client.GetString("endpoint", &endpoint_string))
-      return false;
-    GURL endpoint(endpoint_string);
-    if (!endpoint.is_valid())
-      return false;
-
-    bool subdomains_bool;
-    if (!client.GetBoolean("subdomains", &subdomains_bool))
-      return false;
-    ReportingClient::Subdomains subdomains =
-        subdomains_bool ? ReportingClient::Subdomains::INCLUDE
-                        : ReportingClient::Subdomains::EXCLUDE;
-
-    std::string group;
-    if (!client.GetString("group", &group))
-      return false;
-
-    std::string expires_string;
-    if (!client.GetString("expires", &expires_string))
-      return false;
-    base::TimeTicks expires;
-    if (!DeserializeTicks(expires_string, &expires))
-      return false;
-
-    cache()->SetClient(origin, endpoint, subdomains, group, expires);
-    return true;
-  }
-
-  std::unique_ptr<base::Value> SerializeClients() {
-    std::vector<const ReportingClient*> clients;
-    cache()->GetClients(&clients);
-
-    auto serialized = std::make_unique<base::ListValue>();
-    for (const ReportingClient* client : clients)
-      serialized->Append(SerializeClient(*client));
-
-    return std::move(serialized);
-  }
-
-  bool DeserializeClients(const base::ListValue& clients) {
-    for (size_t i = 0; i < clients.GetSize(); ++i) {
-      const base::DictionaryValue* client;
-      if (!clients.GetDictionary(i, &client))
-        return false;
-      if (!DeserializeClient(*client))
-        return false;
-    }
-
-    return true;
-  }
-
-  static const int kSupportedVersion = 1;
-
-  std::unique_ptr<base::Value> Serialize() {
-    auto serialized = std::make_unique<base::DictionaryValue>();
-
-    serialized->SetInteger("reporting_serialized_cache_version",
-                           kSupportedVersion);
-
-    bool persist_reports = policy().persist_reports_across_restarts;
-    serialized->SetBoolean("includes_reports", persist_reports);
-    if (persist_reports)
-      serialized->Set("reports", SerializeReports());
-
-    bool persist_clients = policy().persist_clients_across_restarts;
-    serialized->SetBoolean("includes_clients", persist_clients);
-    if (persist_clients)
-      serialized->Set("clients", SerializeClients());
-
-    return std::move(serialized);
-  }
-
-  bool Deserialize(const base::Value& serialized_value) {
-    std::vector<const ReportingReport*> reports;
-    cache()->GetReports(&reports);
-    DCHECK(reports.empty());
-
-    std::vector<const ReportingClient*> clients;
-    cache()->GetClients(&clients);
-    DCHECK(clients.empty());
-
-    int version;
-
-    const base::DictionaryValue* serialized;
-    if (!serialized_value.GetAsDictionary(&serialized))
-      return false;
-
-    if (!serialized->GetInteger("reporting_serialized_cache_version", &version))
-      return false;
-    if (version != kSupportedVersion)
-      return false;
-
-    bool includes_reports;
-    bool includes_clients;
-    if (!serialized->GetBoolean("includes_reports", &includes_reports) ||
-        !serialized->GetBoolean("includes_clients", &includes_clients)) {
-      return false;
-    }
-
-    if (includes_reports) {
-      const base::ListValue* reports;
-      if (!serialized->GetList("reports", &reports))
-        return false;
-      if (!DeserializeReports(*reports))
-        return false;
-    }
-
-    if (includes_clients) {
-      const base::ListValue* clients;
-      if (!serialized->GetList("clients", &clients))
-        return false;
-      if (!DeserializeClients(*clients))
-        return false;
-    }
-
-    return true;
-  }
-
-  const ReportingPolicy& policy() { return context_->policy(); }
-  base::Clock* clock() { return context_->clock(); }
-  base::TickClock* tick_clock() { return context_->tick_clock(); }
-  ReportingCache* cache() { return context_->cache(); }
-
-  ReportingContext* context_;
-};
-
-}  // namespace
-
-// static
-std::unique_ptr<ReportingPersister> ReportingPersister::Create(
-    ReportingContext* context) {
-  return std::make_unique<ReportingPersisterImpl>(context);
-}
-
-ReportingPersister::~ReportingPersister() = default;
-
-}  // namespace net

net/reporting/reporting_persister.h

-// Copyright 2017 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.
-
-#ifndef NET_REPORTING_REPORTING_PERSISTER_H_
-#define NET_REPORTING_REPORTING_PERSISTER_H_
-
-#include <memory>
-
-#include "base/callback.h"
-#include "net/base/net_export.h"
-
-namespace net {
-
-class ReportingContext;
-
-// Will persist the state of the Reporting system to (reasonably) stable
-// storage using an as-yet-unwritten persistence mechanism within //net.
-class NET_EXPORT ReportingPersister {
- public:
-  // Creates a ReportingPersister. |context| must outlive the persister.
-  static std::unique_ptr<ReportingPersister> Create(ReportingContext* context);
-
-  virtual ~ReportingPersister();
-};
-
-}  // namespace net
-
-#endif  // NET_REPORTING_REPORTING_PERSISTER_H_

net/reporting/reporting_persister_unittest.cc

-// Copyright 2017 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.
-
-#include "net/reporting/reporting_persister.h"
-
-#include "base/json/json_writer.h"
-#include "base/test/simple_test_clock.h"
-#include "base/test/simple_test_tick_clock.h"
-#include "base/time/time.h"
-#include "base/timer/mock_timer.h"
-#include "base/values.h"
-#include "net/base/test_completion_callback.h"
-#include "net/reporting/reporting_cache.h"
-#include "net/reporting/reporting_client.h"
-#include "net/reporting/reporting_policy.h"
-#include "net/reporting/reporting_report.h"
-#include "net/reporting/reporting_test_util.h"
-#include "testing/gtest/include/gtest/gtest.h"
-
-namespace net {
-namespace {
-
-class ReportingPersisterTest : public ReportingTestBase {
- protected:
-  const GURL kUrl_ = GURL("https://origin/path");
-  const url::Origin kOrigin_ = url::Origin::Create(kUrl_);
-  const GURL kEndpoint_ = GURL("https://endpoint/");
-  const std::string kGroup_ = "group";
-  const std::string kType_ = "default";
-};
-
-// Disabled because the Persister has no persistence layer to use yet.
-TEST_F(ReportingPersisterTest, DISABLED_Test) {
-  ReportingPolicy policy;
-  policy.persist_reports_across_restarts = true;
-  policy.persist_clients_across_restarts = true;
-  // Make sure reports don't expire on our simulated restart.
-  policy.max_report_age = base::TimeDelta::FromDays(30);
-  UsePolicy(policy);
-
-  static const int kAttempts = 3;
-
-  base::DictionaryValue body;
-  body.SetString("key", "value");
-
-  cache()->AddReport(kUrl_, kGroup_, kType_, body.CreateDeepCopy(),
-                     tick_clock()->NowTicks(), kAttempts);
-  cache()->SetClient(kOrigin_, kEndpoint_, ReportingClient::Subdomains::EXCLUDE,
-                     kGroup_,
-                     tick_clock()->NowTicks() + base::TimeDelta::FromDays(1));
-
-  // TODO: Actually save data, once it's possible.
-
-  SimulateRestart(/* delta= */ base::TimeDelta::FromHours(1),
-                  /* delta_ticks= */ base::TimeDelta::FromHours(-3));
-
-  // TODO: Actually load data, once it's possible.
-
-  std::vector<const ReportingReport*> reports;
-  cache()->GetReports(&reports);
-  ASSERT_EQ(1u, reports.size());
-  EXPECT_EQ(kUrl_, reports[0]->url);
-  EXPECT_EQ(kGroup_, reports[0]->group);
-  EXPECT_EQ(kType_, reports[0]->type);
-  EXPECT_EQ(body, *reports[0]->body);
-  EXPECT_EQ(tick_clock()->NowTicks() - base::TimeDelta::FromHours(1),
-            reports[0]->queued);
-  EXPECT_EQ(kAttempts, reports[0]->attempts);
-
-  const ReportingClient* client =
-      FindClientInCache(cache(), kOrigin_, kEndpoint_);
-  ASSERT_TRUE(client);
-  EXPECT_EQ(ReportingClient::Subdomains::EXCLUDE, client->subdomains);
-  EXPECT_EQ(kGroup_, client->group);
-  EXPECT_EQ(tick_clock()->NowTicks() + base::TimeDelta::FromDays(1) -
-                base::TimeDelta::FromHours(1),
-            client->expires);
-}
-
-// TODO(juliatuttle): Test asynchronous behavior.
-
-}  // namespace
-}  // namespace net

net/reporting/reporting_service.cc

@@ -16,7 +16,6 @@
 #include "net/reporting/reporting_context.h"
 #include "net/reporting/reporting_delegate.h"
 #include "net/reporting/reporting_header_parser.h"
-#include "net/reporting/reporting_persister.h"
 #include "url/gurl.h"
 
 namespace net {

net/reporting/reporting_test_util.cc

@@ -20,7 +20,6 @@
 #include "net/reporting/reporting_delegate.h"
 #include "net/reporting/reporting_delivery_agent.h"
 #include "net/reporting/reporting_garbage_collector.h"
-#include "net/reporting/reporting_persister.h"
 #include "net/reporting/reporting_policy.h"
 #include "net/reporting/reporting_uploader.h"
 #include "testing/gtest/include/gtest/gtest.h"

net/reporting/reporting_test_util.h

@@ -123,8 +123,8 @@ class TestReportingContext : public ReportingContext {
   }
 
  private:
-  // Owned by the Persister and GarbageCollector, respectively, but referenced
-  // here to preserve type:
+  // Owned by the DeliveryAgent and GarbageCollector, respectively, but
+  // referenced here to preserve type:
 
   base::MockTimer* delivery_timer_;
   base::MockTimer* garbage_collection_timer_;
@@ -172,8 +172,6 @@ class ReportingTestBase : public ::testing::Test {
     return context_->garbage_collector();
   }
 
-  ReportingPersister* persister() { return context_->persister(); }
-
   base::TimeTicks yesterday();
   base::TimeTicks now();
   base::TimeTicks tomorrow();