ServiceWorker: Stop asynchronously creating response accessors to fix crash due to null context

content/browser/service_worker/service_worker_cache_writer.h

 // Copyright 2015 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 CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_CACHE_WRITER_H_
 #define CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_CACHE_WRITER_H_
 
 #include <stddef.h>
 
 #include <map>
@@ skipping 23 matching lines @@
 // Note that writes done by this class cannot be "short" - ie, if they succeed,
 // they always write all the supplied data back. Therefore completions are
 // signalled with net::Error without a count of bytes written.
 //
 // This class's behavior is modelled as a state machine; see the DoLoop function
 // for comments about this.
 class CONTENT_EXPORT ServiceWorkerCacheWriter {
  public:
   using OnWriteCompleteCallback = base::Callback<void(net::Error)>;
 
-  // The types for the factory functions passed into the constructor. These are
-  // responsible for creating readers from the existing cache entry and writers
-  // to the new cache entry when called. These are passed in as factories
-  // instead of passing readers and writers in directly to avoid creating
-  // writers to entries that won't be updated, and because this class may need
-  // multiple readers internally.

[nhiroki, 2016/02/18 04:21:53]

Creating unused readers and writers wouldn't cost a lot because their ctors just
set up fields and don't interact with the disk cache. Rather, we could reduce
code complexity by creating the readers and writers synchronously.

[falken, 2016/02/18 04:43:59]

Thanks for this comment, I was wondering why it was originally written this way
and why it's ok to change it (and thanks to the the writer of the original code
comment!).

-  using ResponseReaderCreator =
-      base::Callback<scoped_ptr<ServiceWorkerResponseReader>(void)>;
-  using ResponseWriterCreator =
-      base::Callback<scoped_ptr<ServiceWorkerResponseWriter>(void)>;
-
   // The existing reader may be null, in which case this instance will

[falken, 2016/02/18 04:43:59]

change "existing reader" to |compare_reader|

[nhiroki, 2016/02/18 07:09:48]

Done.

   // unconditionally write back data supplied to |MaybeWriteHeaders| and
   // |MaybeWriteData|.
-  ServiceWorkerCacheWriter(const ResponseReaderCreator& reader_creator,
-                           const ResponseWriterCreator& writer_creator);
+  ServiceWorkerCacheWriter(
+      scoped_ptr<ServiceWorkerResponseReader> compare_reader,
+      scoped_ptr<ServiceWorkerResponseReader> copy_reader,
+      scoped_ptr<ServiceWorkerResponseWriter> writer);
 
   ~ServiceWorkerCacheWriter();
 
   // Writes the supplied |headers| back to the cache. Returns ERR_IO_PENDING if
   // the write will complete asynchronously, in which case |callback| will be
   // called when it completes. Otherwise, returns a code other than
   // ERR_IO_PENDING and does not invoke |callback|. Note that this method will
   // not necessarily write data back to the cache if the incoming data is
   // equivalent to the existing cached data. See the source of this function for
   // details about how this function drives the state machine.
@@ skipping 142 matching lines @@
   size_t cached_length_;
 
   size_t bytes_compared_;
   size_t bytes_copied_;
   size_t bytes_written_;
 
   bool did_replace_;
 
   size_t compare_offset_;
 
-  ResponseReaderCreator reader_creator_;
-  ResponseWriterCreator writer_creator_;
   scoped_ptr<ServiceWorkerResponseReader> compare_reader_;
   scoped_ptr<ServiceWorkerResponseReader> copy_reader_;
   scoped_ptr<ServiceWorkerResponseWriter> writer_;
   base::WeakPtrFactory<ServiceWorkerCacheWriter> weak_factory_;
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_SERVICE_WORKER_SERVICE_WORKER_CACHE_WRITER_H_