Cherry pick PR #1327: Refactor persistent settings for Cobalt cache (#…

…1642) Refer to the original PR: #1327 Move persistent settings module ownership to URLRequestContext, which also owns the cache and cache backends. This allows removing reverse depenendencies from /net code to /cobalt/persistent_settings, while leaving runtime functionality the same. Note: This intentionally just moves the code from `CobaltBackendImpl` to `URLRequestContext` with minimal changes. A future refactor could encapsulate the settings into its own class. b/296578318 Co-authored-by: Kaido Kert <[email protected]>
youtube · Sep 27, 2023 · 1f6f9a6 · 1f6f9a6
1 parent ead474d
commit 1f6f9a6
Show file tree

Hide file tree

Showing 8 changed files with 78 additions and 66 deletions.
diff --git a/cobalt/browser/browser_module.cc b/cobalt/browser/browser_module.cc
@@ -2234,11 +2234,7 @@ void BrowserModule::ValidateCacheBackendSettings() {
   auto url_request_context = network_module_->url_request_context();
   auto http_cache = url_request_context->http_transaction_factory()->GetCache();
   if (!http_cache) return;
-  auto cache_backend = static_cast<disk_cache::CobaltBackendImpl*>(
-      http_cache->GetCurrentBackend());
-  if (cache_backend) {
-    cache_backend->ValidatePersistentSettings();
-  }
+  network_module_->url_request_context()->ValidateCachePersistentSettings();
 }
 
 }  // namespace browser

diff --git a/cobalt/h5vcc/h5vcc_storage.cc b/cobalt/h5vcc/h5vcc_storage.cc
@@ -314,7 +314,10 @@ H5vccStorageSetQuotaResponse H5vccStorage::SetQuota(
 void H5vccStorage::SetAndSaveQuotaForBackend(disk_cache::ResourceType type,
                                              uint32_t bytes) {
   if (cache_backend_) {
-    cache_backend_->UpdateSizes(type, bytes);
+    if (cache_backend_->UpdateSizes(type, bytes)) {
+      auto url_request_context = network_module_->url_request_context();
+      url_request_context->UpdateCacheSizeSetting(type, bytes);
+    }
 
     if (bytes == 0) {
       network_module_->task_runner()->PostTask(

diff --git a/cobalt/network/url_request_context.cc b/cobalt/network/url_request_context.cc
@@ -14,6 +14,7 @@
 
 #include "cobalt/network/url_request_context.h"
 
+#include <algorithm>
 #include <memory>
 #include <string>
 #include <utility>
@@ -48,6 +49,45 @@
 #include "net/url_request/data_protocol_handler.h"
 #include "net/url_request/url_request_job_factory_impl.h"
 
+namespace {
+
+const char kPersistentSettingsJson[] = "cache_settings.json";
+
+
+void ReadDiskCacheSize(cobalt::persistent_storage::PersistentSettings* settings,
+                       int64_t max_bytes) {
+  auto total_size = 0;
+  disk_cache::ResourceTypeMetadata kTypeMetadataNew[disk_cache::kTypeCount];
+
+  for (int i = 0; i < disk_cache::kTypeCount; i++) {
+    auto metadata = disk_cache::kTypeMetadata[i];
+    uint32_t bucket_size =
+        static_cast<uint32_t>(settings->GetPersistentSettingAsDouble(
+            metadata.directory, metadata.max_size_bytes));
+    kTypeMetadataNew[i] = {metadata.directory, bucket_size};
+
+    total_size += bucket_size;
+  }
+
+  // Check if PersistentSettings values are valid and can replace the
+  // disk_cache::kTypeMetadata.
+  if (total_size <= max_bytes) {
+    std::copy(std::begin(kTypeMetadataNew), std::end(kTypeMetadataNew),
+              std::begin(disk_cache::kTypeMetadata));
+    return;
+  }
+
+  // PersistentSettings values are invalid and will be replaced by the default
+  // values in disk_cache::kTypeMetadata.
+  for (int i = 0; i < disk_cache::kTypeCount; i++) {
+    auto metadata = disk_cache::kTypeMetadata[i];
+    settings->SetPersistentSetting(
+        metadata.directory, std::make_unique<base::Value>(
+                                static_cast<double>(metadata.max_size_bytes)));
+  }
+}
+}  // namespace
+
 namespace cobalt {
 namespace network {
 namespace {
@@ -189,10 +229,16 @@ URLRequestContext::URLRequestContext(
     // is less than 1 mb and subtract this from the max_cache_bytes.
     max_cache_bytes -= (1 << 20);
 
+    // Initialize and read caching persistent settings
+    cache_persistent_settings_ =
+        std::make_unique<cobalt::persistent_storage::PersistentSettings>(
+            kPersistentSettingsJson);
+    ReadDiskCacheSize(cache_persistent_settings_.get(), max_cache_bytes);
+
     auto http_cache = std::make_unique<net::HttpCache>(
         storage_.http_network_session(),
         std::make_unique<net::HttpCache::DefaultBackend>(
-            net::DISK_CACHE, net::CACHE_BACKEND_COBALT,
+            net::DISK_CACHE, net::CACHE_BACKEND_DEFAULT,
             base::FilePath(std::string(path.data())),
             /* max_bytes */ max_cache_bytes),
         true);
@@ -243,5 +289,17 @@ void URLRequestContext::OnQuicToggle(const std::string& message) {
 }
 #endif  // defined(ENABLE_DEBUGGER)
 
+void URLRequestContext::UpdateCacheSizeSetting(disk_cache::ResourceType type,
+                                               uint32_t bytes) {
+  CHECK(cache_persistent_settings_);
+  cache_persistent_settings_->SetPersistentSetting(
+      disk_cache::kTypeMetadata[type].directory,
+      std::make_unique<base::Value>(static_cast<double>(bytes)));
+}
+
+void URLRequestContext::ValidateCachePersistentSettings() {
+  cache_persistent_settings_->ValidatePersistentSettings();
+}
+
 }  // namespace network
 }  // namespace cobalt
diff --git a/cobalt/network/url_request_context.h b/cobalt/network/url_request_context.h
@@ -15,13 +15,15 @@
 #ifndef COBALT_NETWORK_URL_REQUEST_CONTEXT_H_
 #define COBALT_NETWORK_URL_REQUEST_CONTEXT_H_
 
+#include <memory>
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/macros.h"
 #include "base/sequence_checker.h"
 #include "cobalt/persistent_storage/persistent_settings.h"
 #include "net/cookies/cookie_monster.h"
+#include "net/disk_cache/cobalt/resource_type.h"
 #include "net/log/net_log.h"
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_context_getter.h"
@@ -53,6 +55,9 @@ class URLRequestContext : public net::URLRequestContext {
 
   bool using_http_cache();
 
+  void UpdateCacheSizeSetting(disk_cache::ResourceType type, uint32_t bytes);
+  void ValidateCachePersistentSettings();
+
  private:
   SEQUENCE_CHECKER(sequence_checker_);
   net::URLRequestContextStorage storage_;
@@ -70,6 +75,10 @@ class URLRequestContext : public net::URLRequestContext {
   void OnQuicToggle(const std::string&);
 #endif  // defined(ENABLE_DEBUGGER)
 
+  // Persistent settings module for Cobalt disk cache quotas
+  std::unique_ptr<cobalt::persistent_storage::PersistentSettings>
+      cache_persistent_settings_;
+
   DISALLOW_COPY_AND_ASSIGN(URLRequestContext);
 };
 

diff --git a/net/BUILD.gn b/net/BUILD.gn
@@ -4217,7 +4217,6 @@ target(gtest_target_type, "net_unittests") {
     "//base:i18n",
     "//base/test:test_support",
     "//base/third_party/dynamic_annotations",
-    "//cobalt/persistent_storage:persistent_settings",
     "//crypto",
     "//testing/gmock",
     "//testing/gtest",

diff --git a/net/base/cache_type.h b/net/base/cache_type.h
@@ -24,9 +24,6 @@ enum BackendType {
   CACHE_BACKEND_DEFAULT,
   CACHE_BACKEND_BLOCKFILE,  // The |BackendImpl|.
   CACHE_BACKEND_SIMPLE,     // The |SimpleBackendImpl|.
-#if defined(STARBOARD)
-  CACHE_BACKEND_COBALT      // The |CobaltBackendImpl|,
-#endif
 };
 
 }  // namespace net

diff --git a/net/disk_cache/cobalt/cobalt_backend_impl.cc b/net/disk_cache/cobalt/cobalt_backend_impl.cc
@@ -30,8 +30,6 @@ namespace disk_cache {
 
 namespace {
 
-const char kPersistentSettingsJson[] = "cache_settings.json";
-
 void CompletionOnceCallbackHandler(
     scoped_refptr<CobaltBackendImpl::RefCountedRunner> runner,
     int result) {
@@ -53,38 +51,6 @@ ResourceType GetType(const std::string& key) {
   return kOther;
 }
 
-void ReadDiskCacheSize(
-    cobalt::persistent_storage::PersistentSettings* settings,
-    int64_t max_bytes) {
-  auto total_size = 0;
-  disk_cache::ResourceTypeMetadata kTypeMetadataNew[disk_cache::kTypeCount];
-
-  for (int i = 0; i < disk_cache::kTypeCount; i++) {
-    auto metadata = disk_cache::kTypeMetadata[i];
-    uint32_t bucket_size =
-        static_cast<uint32_t>(settings->GetPersistentSettingAsDouble(
-            metadata.directory, metadata.max_size_bytes));
-    kTypeMetadataNew[i] = {metadata.directory, bucket_size};
-
-    total_size += bucket_size;
-  }
-
-  // Check if PersistentSettings values are valid and can replace the disk_cache::kTypeMetadata.
-  if (total_size <= max_bytes) {
-    std::copy(std::begin(kTypeMetadataNew), std::end(kTypeMetadataNew), std::begin(disk_cache::kTypeMetadata));
-    return;
-  }
-
-  // PersistentSettings values are invalid and will be replaced by the default values in
-  // disk_cache::kTypeMetadata.
-  for (int i = 0; i < disk_cache::kTypeCount; i++) {
-    auto metadata = disk_cache::kTypeMetadata[i];
-    settings->SetPersistentSetting(
-            metadata.directory,
-            std::make_unique<base::Value>(static_cast<double>(metadata.max_size_bytes)));
-  }
-}
-
 }  // namespace
 
 CobaltBackendImpl::CobaltBackendImpl(
@@ -94,12 +60,9 @@ CobaltBackendImpl::CobaltBackendImpl(
     net::CacheType cache_type,
     net::NetLog* net_log)
     : weak_factory_(this) {
-  persistent_settings_ =
-      std::make_unique<cobalt::persistent_storage::PersistentSettings>(
-          kPersistentSettingsJson);
-  ReadDiskCacheSize(persistent_settings_.get(), max_bytes);
 
   // Initialize disk backend for each resource type.
+  // Note: kTypeMetadata is refreshed from settings before this constructor runs
   int64_t total_size = 0;
   for (int i = 0; i < kTypeCount; i++) {
     auto metadata = kTypeMetadata[i];
@@ -123,28 +86,20 @@ CobaltBackendImpl::~CobaltBackendImpl() {
   simple_backend_map_.clear();
 }
 
-void CobaltBackendImpl::UpdateSizes(ResourceType type, uint32_t bytes) {
+bool CobaltBackendImpl::UpdateSizes(ResourceType type, uint32_t bytes) {
   if (bytes == disk_cache::kTypeMetadata[type].max_size_bytes)
-    return;
-
-  // Static cast value to double since base::Value cannot be a long.
-  persistent_settings_->SetPersistentSetting(
-      disk_cache::kTypeMetadata[type].directory,
-      std::make_unique<base::Value>(static_cast<double>(bytes)));
+    return false;
 
   disk_cache::kTypeMetadata[type].max_size_bytes = bytes;
   SimpleBackendImpl* simple_backend = simple_backend_map_[type];
   simple_backend->SetMaxSize(bytes);
+  return true;
 }
 
 uint32_t CobaltBackendImpl::GetQuota(ResourceType type) {
   return disk_cache::kTypeMetadata[type].max_size_bytes;
 }
 
-void CobaltBackendImpl::ValidatePersistentSettings() {
-  persistent_settings_->ValidatePersistentSettings();
-}
-
 net::Error CobaltBackendImpl::Init(CompletionOnceCallback completion_callback) {
   auto closure_runner =
       base::MakeRefCounted<RefCountedRunner>(std::move(completion_callback));

diff --git a/net/disk_cache/cobalt/cobalt_backend_impl.h b/net/disk_cache/cobalt/cobalt_backend_impl.h
@@ -23,7 +23,6 @@
 #include <utility>
 
 #include "base/callback_helpers.h"
-#include "cobalt/persistent_storage/persistent_settings.h"
 #include "net/base/completion_once_callback.h"
 #include "net/disk_cache/cobalt/resource_type.h"
 #include "net/disk_cache/disk_cache.h"
@@ -49,9 +48,8 @@ class NET_EXPORT_PRIVATE CobaltBackendImpl final : public Backend {
   ~CobaltBackendImpl() override;
 
   net::Error Init(CompletionOnceCallback completion_callback);
-  void UpdateSizes(ResourceType type, uint32_t bytes);
+  bool UpdateSizes(ResourceType type, uint32_t bytes);
   uint32_t GetQuota(ResourceType type);
-  void ValidatePersistentSettings();
 
   // Backend interface.
   net::CacheType GetCacheType() const override;
@@ -118,9 +116,6 @@ class NET_EXPORT_PRIVATE CobaltBackendImpl final : public Backend {
 
   std::map<ResourceType, SimpleBackendImpl*> simple_backend_map_;
 
-  // Json PrefStore used for persistent settings.
-  std::unique_ptr<cobalt::persistent_storage::PersistentSettings>
-      persistent_settings_;
 };
 
 }  // namespace disk_cache