[Cronet] Purge storage directory if version does not match

components/cronet/android/cronet_url_request_context_adapter.cc

 // Copyright 2014 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 "components/cronet/android/cronet_url_request_context_adapter.h"
 
 #include <limits.h>
 #include <stddef.h>
 #include <map>
 #include <utility>
@@ skipping 37 matching lines @@
 #include "net/url_request/url_request_context_builder.h"
 #include "net/url_request/url_request_interceptor.h"
 
 #if defined(DATA_REDUCTION_PROXY_SUPPORT)
 #include "components/cronet/android/cronet_data_reduction_proxy.h"
 #endif
 
 namespace {
 
 const char kHttpServerProperties[] = "net.http_server_properties";
+// Current version of disk cache.
+const int32_t kDiskCacheVersion = 1;
+
+struct VersionHeader {
+  VersionHeader() { std::memset(this, 0, sizeof(*this)); };

[mmenke, 2016/03/31 15:39:07]

Think we should just just direct initialize version.  I'd add a
kDiskCacheVersionUnknown = 0 or something, and use that, just to be paranoid.

[xunjieli, 2016/03/31 18:45:58]

Done.

+  uint32_t version;

[mmenke, 2016/03/31 15:39:07]

include <stdint.h>

[xunjieli, 2016/03/31 18:45:58]

Done.

+};
 
 // Connects the HttpServerPropertiesManager's storage to the prefs.
 class PrefServiceAdapter
     : public net::HttpServerPropertiesManager::PrefDelegate {
  public:
   explicit PrefServiceAdapter(PrefService* pref_service)
       : pref_service_(pref_service), path_(kHttpServerProperties) {
     pref_change_registrar_.Init(pref_service_);
   }
 
@@ skipping 188 matching lines @@
   }
 
   bool OnCanAccessFile(const net::URLRequest& request,
                        const base::FilePath& path) const override {
     return false;
   }
 
   DISALLOW_COPY_AND_ASSIGN(BasicNetworkDelegate);
 };
 
+bool IsCurrentVersion(const base::FilePath& version_filepath) {
+  if (!base::PathExists(version_filepath))
+    return false;
+  base::File version_file(version_filepath,
+                          base::File::FLAG_OPEN | base::File::FLAG_READ);
+  VersionHeader version;
+  int bytes_read =
+      version_file.Read(0, reinterpret_cast<char*>(&version), sizeof(version));
+  if (bytes_read != sizeof(version)) {
+    DLOG(WARNING) << "Cannot read from version file.";
+    return false;
+  }
+  return version.version == kDiskCacheVersion;
+}
+
+// TODO(xunjieli): Handle failures.
+void InitializeCacheDiskCacheDirectory(const base::FilePath& dir) {
+  // Checks version file and remove old versions.
+  base::FilePath version_filepath = dir.Append("version");
+  if (IsCurrentVersion(version_filepath)) {
+    // The version is up-to-date, so there is nothing to do.
+    return;
+  }
+  if (!(base::DeleteFile(dir, true) && base::CreateDirectory(dir))) {

[mmenke, 2016/03/31 15:39:07]

Think it's worth a comment that DeleteFile returns true if the directory does
not exist, since we're strongly depending on that unexpected behavior here.

[xunjieli, 2016/03/31 18:45:58]

Done.

+    DLOG(WARNING) << "Cannot purge directory.";
+    return;
+  }
+  base::FilePath new_version = dir.Append("version");

[mmenke, 2016/03/31 15:39:07]

This is the same as version_filepath.  Just using that seems less regression
prone.

[xunjieli, 2016/03/31 18:45:58]

Done.

+  base::File new_version_file(
+      new_version, base::File::FLAG_CREATE_ALWAYS | base::File::FLAG_WRITE);
+
+  if (!new_version_file.IsValid()) {
+    DLOG(WARNING) << "Cannot create a version file.";
+    return;
+  }
+
+  DCHECK(new_version_file.created());
+  VersionHeader version;
+  version.version = kDiskCacheVersion;
+  int bytes_written = new_version_file.Write(
+      0, reinterpret_cast<char*>(&version), sizeof(version));
+  if (bytes_written != sizeof(version)) {
+    DLOG(WARNING) << "Cannot write to version file.";
+    return;
+  }
+  base::FilePath prefs_dir = dir.Append(FILE_PATH_LITERAL("prefs"));
+  if (!base::CreateDirectory(prefs_dir)) {
+    DLOG(WARNING) << "Cannot create prefs directory";
+    return;
+  }
+}
+
 }  // namespace
 
 namespace cronet {
 
 // Explicitly register static JNI functions.
 bool CronetUrlRequestContextAdapterRegisterJni(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }
 
 CronetURLRequestContextAdapter::CronetURLRequestContextAdapter(
@@ skipping 151 matching lines @@
 #endif  // defined(DATA_REDUCTION_PROXY_SUPPORT)
   context_builder.set_network_delegate(std::move(network_delegate));
   context_builder.set_net_log(net_log_.get());
 
   // Android provides a local HTTP proxy server that handles proxying when a PAC
   // URL is present. Create a proxy service without a resolver and rely on this
   // local HTTP proxy. See: crbug.com/432539.
   context_builder.set_proxy_service(
       net::ProxyService::CreateWithoutProxyResolver(
           std::move(proxy_config_service_), net_log_.get()));
+
   config->ConfigureURLRequestContextBuilder(&context_builder, net_log_.get(),
                                             GetFileThread()->task_runner());
 
   // Set up pref file if storage path is specified.
   if (!config->storage_path.empty()) {
-    base::FilePath filepath(config->storage_path);
-    filepath = filepath.Append(FILE_PATH_LITERAL("local_prefs.json"));
+    base::FilePath storage_path(config->storage_path);
+    // Make sure disk cache directory has correct version.
+    InitializeCacheDiskCacheDirectory(storage_path);

[mmenke, 2016/03/31 15:39:07]

We're doing file IO on the network thread.  This doesn't trigger any DCHECKs? 
We don't actually get anything at all from not doing on the network thread,
since we're blocked on waiting for this to complete anyways, just wondering
about the checks.

[xunjieli, 2016/03/31 18:45:58]

No checks are triggered when I run the tests locally with a debug build. I
looked through the code. It seems that File I/O is permitted on thread started
with base::MessageLoop::TYPE_IO option. The network thread has this option.

in base/files/file_posix.h:
ThreadRestrictions::AssertIOAllowed
https://code.google.com/p/chromium/codesearch#chromium/src/base/files/file_po...

+    base::FilePath filepath =
+        storage_path.Append(FILE_PATH_LITERAL("prefs"))
+            .Append(FILE_PATH_LITERAL("local_prefs.json"));
     json_pref_store_ = new JsonPrefStore(
         filepath, GetFileThread()->task_runner(), scoped_ptr<PrefFilter>());
     context_builder.SetFileTaskRunner(GetFileThread()->task_runner());
 
     // Set up HttpServerPropertiesManager.
     PrefServiceFactory factory;
     factory.set_user_prefs(json_pref_store_);
     scoped_refptr<PrefRegistrySimple> registry(new PrefRegistrySimple());
     registry->RegisterDictionaryPref(kHttpServerProperties,
                                      new base::DictionaryValue());
@@ skipping 346 matching lines @@
     JNIEnv* env,
     const JavaParamRef<jclass>& jcaller) {
   base::StatisticsRecorder::Initialize();
   std::vector<uint8_t> data;
   if (!HistogramManager::GetInstance()->GetDeltas(&data))
     return ScopedJavaLocalRef<jbyteArray>();
   return base::android::ToJavaByteArray(env, &data[0], data.size());
 }
 
 }  // namespace cronet