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

content/browser/appcache/appcache_storage_impl.cc

 // Copyright (c) 2012 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 "content/browser/appcache/appcache_storage_impl.h"
 
 #include <stddef.h>
 
 #include <algorithm>
 #include <functional>
@@ skipping 1717 matching lines @@
 
 void AppCacheStorageImpl::StoreEvictionTimes(AppCacheGroup* group) {
   scoped_refptr<UpdateEvictionTimesTask> task(
       new UpdateEvictionTimesTask(this, group));
   task->Schedule();
 }
 
 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(
     const GURL& manifest_url,
     const std::vector<int64_t>& response_ids) {
   if (response_ids.empty())
     return;
 
   // Start deleting them from the disk cache lazily.
   StartDeletingResponses(response_ids);
@@ skipping 44 matching lines @@
       FROM_HERE, base::Bind(&AppCacheStorageImpl::DeleteOneResponse,
                             weak_factory_.GetWeakPtr()),
       kBriefDelay);
   is_response_deletion_scheduled_ = true;
 }
 
 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;
     return;
   }
 
   // TODO(michaeln): add group_id to DoomEntry args
   int64_t id = deletable_response_ids_.front();
   int rv = disk_cache_->DoomEntry(
       id, base::Bind(&AppCacheStorageImpl::OnDeletedOneResponse,
@@ skipping 67 matching lines @@
 
 void AppCacheStorageImpl::RunOnePendingSimpleTask() {
   DCHECK(!pending_simple_tasks_.empty());
   base::Closure task = pending_simple_tasks_.front();
   pending_simple_tasks_.pop_front();
   task.Run();
 }
 
 AppCacheDiskCache* AppCacheStorageImpl::disk_cache() {
   DCHECK(IsInitTaskComplete());
-
-  if (is_disabled_)
-    return NULL;
+  DCHECK(!is_disabled_);
 
   if (!disk_cache_) {
     int rv = net::OK;
     disk_cache_.reset(new AppCacheDiskCache);
     if (is_incognito_) {
       rv = disk_cache_->InitWithMemBackend(
           kMaxMemDiskCacheSize,
           base::Bind(&AppCacheStorageImpl::OnDiskCacheInitialized,
                      base::Unretained(this)));
     } else {
@@ skipping 66 matching lines @@
 
 void AppCacheStorageImpl::OnLazyCommitTimer() {
   lazy_commit_timer_.Stop();
   if (is_disabled())
     return;
   scoped_refptr<DatabaseTask> task(new CommitLastAccessTimesTask(this));
   task->Schedule();
 }
 
 }  // namespace content