Make synchronous methods on disk_cache::SimpleEntryImpl() reentrant.

net/disk_cache/simple/simple_entry_impl.h

 // Copyright (c) 2013 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 NET_DISK_CACHE_SIMPLE_SIMPLE_ENTRY_IMPL_H_
 #define NET_DISK_CACHE_SIMPLE_SIMPLE_ENTRY_IMPL_H_
 
 #include <string>
 
+#include "base/file_path.h"
 #include "base/memory/weak_ptr.h"
 #include "base/threading/thread_checker.h"
 #include "net/disk_cache/disk_cache.h"
+#include "net/disk_cache/simple/simple_disk_format.h"
 
 namespace net {
 class IOBuffer;
 }
 
 namespace disk_cache {
 
 class SimpleSynchronousEntry;
 
 // SimpleEntryImpl is the IO thread interface to an entry in the very simple
@@ skipping 60 matching lines @@
 
   virtual ~SimpleEntryImpl();
 
   // Clears the current |synchronous_entry_| and returns it. Used when posting
   // operations on the |synchronous_entry_| to the worker pool. This insures
   // accesses to |synchronous_entry_| are thread safe from the memory barriers
   // included in PostTask.
   SimpleSynchronousEntry* ReleaseSynchronousEntry();
 
   // Sets the current |synchronous_entry_|. Called on completion of worker pool
-  // IO to permit new operations to be launched.
+  // IO to permit new operations to be launched. Must be called on construction.
   void SetSynchronousEntry(SimpleSynchronousEntry* synchronous_entry);
 
   base::ThreadChecker thread_checker_;
   base::WeakPtrFactory<SimpleEntryImpl> weak_ptr_factory_;
-  std::string key_;
+
+  // |path_| and |key_| are copied from the synchronous entry on construction,
+  // and never updated as they are const.
+  const base::FilePath path_;
+  const std::string key_;
+
+  // |last_used_|, |last_modified_| and |data_size_| are copied from the
+  // synchronous entry at the completion of each item of asynchronous IO.
+  base::Time last_used_;

[felipeg, 2013/02/12 14:47:48]

I don't like the fact that we are keeping a copy of this data on both objects
(here and in the SynchrnounsEntry).
It will be very easy in the future to change something and forget to make a copy
from one side or the other. When we have to maintain a copy on the code, the
data will eventually get inconsistent as we change the code.

I would rather keep it only here and remove it completely from SynchronousEntry.

+  base::Time last_modified_;
+  int32 data_size_[kSimpleEntryFileCount];
 
   // The |synchronous_entry_| is the worker thread object that performs IO on
   // entries. It is set to NULL while operations are pending to prevent unsafe
   // access from multiple threads.
   SimpleSynchronousEntry* synchronous_entry_;
 
   bool has_been_doomed_;
 };
 
 }  // namespace disk_cache
 
 #endif  // NET_DISK_CACHE_SIMPLE_SIMPLE_ENTRY_IMPL_H_