AppCache: Add nullptr check where AppCacheStorageImpl::disk_cache() is called

content/browser/appcache/appcache_storage_impl.cc

Index: content/browser/appcache/appcache_storage_impl.cc
diff --git a/content/browser/appcache/appcache_storage_impl.cc b/content/browser/appcache/appcache_storage_impl.cc
index 96e5d1be2968a6f8ded15bac0d8454a5b3d7eaed..04b5dd75daed729d244ec9f75626eef81d4c3882 100644
--- a/content/browser/appcache/appcache_storage_impl.cc
+++ b/content/browser/appcache/appcache_storage_impl.cc
@@ -1735,19 +1735,20 @@ void AppCacheStorageImpl::StoreEvictionTimes(AppCacheGroup* group) {
 AppCacheResponseReader* AppCacheStorageImpl::CreateResponseReader(
     const GURL& manifest_url,
     int64_t response_id) {
-  return new AppCacheResponseReader(response_id, disk_cache()->GetWeakPtr());
+  return new AppCacheResponseReader(
+      response_id, is_disabled_ ? nullptr : disk_cache()->GetWeakPtr());
 }
 
 AppCacheResponseWriter* AppCacheStorageImpl::CreateResponseWriter(
     const GURL& manifest_url) {
-  return new AppCacheResponseWriter(NewResponseId(),
-                                    disk_cache()->GetWeakPtr());
+  return new AppCacheResponseWriter(
+      NewResponseId(), is_disabled_ ? nullptr : disk_cache()->GetWeakPtr());
 }
 
 AppCacheResponseMetadataWriter*
 AppCacheStorageImpl::CreateResponseMetadataWriter(int64_t response_id) {
-  return new AppCacheResponseMetadataWriter(response_id,
-                                            disk_cache()->GetWeakPtr());
+  return new AppCacheResponseMetadataWriter(
+      response_id, is_disabled_ ? nullptr : disk_cache()->GetWeakPtr());
 }
 
 void AppCacheStorageImpl::DoomResponses(
@@ -1812,8 +1813,7 @@ void AppCacheStorageImpl::DeleteOneResponse() {
   DCHECK(is_response_deletion_scheduled_);
   DCHECK(!deletable_response_ids_.empty());
 
-  if (!disk_cache()) {
-    DCHECK(is_disabled_);
+  if (is_disabled_) {

[kinuko, 2016/06/06 04:06:17]

nit: !disk_cache() and is_disabled_ seems to have slight difference, did you
make sure this change exactly does what's expected?

[nhiroki, 2016/06/06 04:41:35]

disk_cache() returns nullptr only if is_disabled_ is true. When a diskcache
instance hasn't been created yet, disk_cache() creates a new one and returns it.
So, I think this is completely equal to the previous.

[kinuko, 2016/06/06 04:56:21]

It seems to mean disk_cache() has side-effect, is directly accessing disk_cache_
below on line 1825 always safe in new code? (just checking)

[nhiroki, 2016/06/06 06:04:17]

Ohh, good point! I'm not 100% sure that disk_cache_ has been ensured before line
1825. Maybe we can call disk_cache() on line 1825 instead of directly accessing
disk_cache. Changed.

     deletable_response_ids_.clear();
     deleted_response_ids_.clear();
     is_response_deletion_scheduled_ = false;
@@ -1901,9 +1901,7 @@ void AppCacheStorageImpl::RunOnePendingSimpleTask() {
 
 AppCacheDiskCache* AppCacheStorageImpl::disk_cache() {
   DCHECK(IsInitTaskComplete());
-
-  if (is_disabled_)
-    return NULL;
+  DCHECK(!is_disabled_);
 
   if (!disk_cache_) {
     int rv = net::OK;