Fix a couple of data races on booleans

BUG=21468,22520
For the explanation why these are data races,
see http://code.google.com/p/data-race-test/wiki/PopularDataRaces
Review URL: http://codereview.chromium.org/251027

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@33825 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 19 insertions, 18 deletions

diff --git a/chrome/browser/cancelable_request.h b/chrome/browser/cancelable_request.h
index e83dd0c..f771d02 100644
--- a/chrome/browser/cancelable_request.h
+++ b/chrome/browser/cancelable_request.h
@@ -87,6 +87,7 @@
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/cancellation_flag.h"
 #include "base/lock.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
@@ -365,8 +366,7 @@ class CancelableRequestBase
   CancelableRequestBase()
       : provider_(NULL),
         consumer_(NULL),
-        handle_(0),
-        canceled_(false) {
+        handle_(0) {
     callback_thread_ = MessageLoop::current();
   }
 
@@ -380,11 +380,12 @@ class CancelableRequestBase
 
   // The canceled flag indicates that the request should not be executed.
   // A request can never be uncanceled, so only a setter for true is provided.
+  // This can be called multiple times, but only from one thread.
   void set_canceled() {
-    canceled_ = true;
+    canceled_.Set();
   }
   bool canceled() {
-    return canceled_;
+    return canceled_.IsSet();
   }
 
  protected:
@@ -417,7 +418,7 @@ class CancelableRequestBase
   CancelableRequestProvider* provider_;
 
   // Notified after we execute that the request is complete.  This should only
-  // be accessed if !canceled_, otherwise the pointer is invalid.
+  // be accessed if !canceled_.IsSet(), otherwise the pointer is invalid.
   CancelableRequestConsumerBase* consumer_;
 
   // The handle to this request inside the provider. This will be initialized
@@ -427,7 +428,7 @@ class CancelableRequestBase
 
   // Set if the caller cancels this request. No callbacks should be made when
   // this is set.
-  bool canceled_;
+  base::CancellationFlag canceled_;
 
  private:
   DISALLOW_EVIL_CONSTRUCTORS(CancelableRequestBase);
@@ -512,7 +513,7 @@ class CancelableRequest : public CancelableRequestBase {
   // Executes the callback and notifies the provider and the consumer that this
   // request has been completed. This must be called on the callback_thread_.
   void ExecuteCallback(const TupleType& param) {
-    if (!canceled_) {
+    if (!canceled_.IsSet()) {
       // Execute the callback.
       callback_->RunWithParams(param);
 
@@ -522,8 +523,8 @@ class CancelableRequest : public CancelableRequestBase {
     }
   }
 
-  // This should only be executed if !canceled_, otherwise the pointers may be
-  // invalid.
+  // This should only be executed if !canceled_.IsSet(),
+  // otherwise the pointers may be invalid.
   scoped_ptr<CallbackType> callback_;
 };
 
diff --git a/net/base/directory_lister.cc b/net/base/directory_lister.cc
index 8204fd8..35b30cd 100644
--- a/net/base/directory_lister.cc
+++ b/net/base/directory_lister.cc
@@ -55,8 +55,7 @@ DirectoryLister::DirectoryLister(const FilePath& dir,
     : dir_(dir),
       delegate_(delegate),
       message_loop_(NULL),
-      thread_(kNullThreadHandle),
-      canceled_(false) {
+      thread_(kNullThreadHandle) {
   DCHECK(!dir.value().empty());
 }
 
@@ -84,7 +83,7 @@ bool DirectoryLister::Start() {
 }
 
 void DirectoryLister::Cancel() {
-  canceled_ = true;
+  canceled_.Set();
 
   if (thread_) {
     PlatformThread::Join(thread_);
@@ -108,7 +107,7 @@ void DirectoryLister::ThreadMain() {
           file_util::FileEnumerator::DIRECTORIES |
           file_util::FileEnumerator::INCLUDE_DOT_DOT));
 
-  while (!canceled_ && !(file_enum.Next().value().empty())) {
+  while (!canceled_.IsSet() && !(file_enum.Next().value().empty())) {
     e->data.push_back(file_util::FileEnumerator::FindInfo());
     file_enum.GetFindInfo(&e->data[e->data.size() - 1]);
 
@@ -143,14 +142,14 @@ void DirectoryLister::OnReceivedData(
   // need to null check it during each iteration of the loop.  Similarly, it is
   // necessary to check the canceled_ flag to avoid sending data to a delegate
   // who doesn't want anymore.
-  for (int i = 0; !canceled_ && delegate_ && i < count; ++i)
+  for (int i = 0; !canceled_.IsSet() && delegate_ && i < count; ++i)
     delegate_->OnListFile(data[i]);
 }
 
 void DirectoryLister::OnDone(int error) {
-  // If canceled, we need to report some kind of error, but don't overwrite the
-  // error condition if it is already set.
-  if (!error && canceled_)
+  // If canceled is set, we need to report some kind of error,
+  // but don't overwrite the error condition if it is already set.
+  if (!error && canceled_.IsSet())
     error = net::ERR_ABORTED;
 
   if (delegate_)
diff --git a/net/base/directory_lister.h b/net/base/directory_lister.h
index 83dc1ef..609a087 100644
--- a/net/base/directory_lister.h
+++ b/net/base/directory_lister.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/cancellation_flag.h"
 #include "base/file_path.h"
 #include "base/file_util.h"
 #include "base/platform_thread.h"
@@ -65,7 +66,7 @@ class DirectoryLister : public base::RefCountedThreadSafe<DirectoryLister>,
   DirectoryListerDelegate* delegate_;
   MessageLoop* message_loop_;
   PlatformThreadHandle thread_;
-  bool canceled_;
+  base::CancellationFlag canceled_;
 };
 
 }  // namespace net