Revert 210423 "base: Make SequencedWorkerPool issue globally uni..."

Speculative revert for failing PageCyclerTest.FailProvisionalLoads on
linux_clang.

> base: Make SequencedWorkerPool issue globally unique SequenceTokens.
> 
> SequencedWorkerPool currently issues SequenceTokens out of an internal member counter. This means that two different SequencedWorkerPool instances can issue identical SequenceTokens, which mucks up any attempt to distinguish sequences using only SequenceTokens.
> 
> This change makes the SequenceTokens issued from an StaticAtomicSequenceNumber, which is globally shared amongst all SequencedWorkerPools.
> 
> This change also makes the SequencedWorkerPool included in the nacl_untrusted builds, as it is needed for SequenceChecker and WeakPtr to work correctly. It previously was excluded because it used base/metrics. I've #ifdefed the base/metrics usage out for nacl.
> 
> This issue is a spinoff and pre-requisite of issue 18501008: Make WeakPtr use SequenceChecker instead of ThreadChecker.
> 
> R=akalin,darin
> BUG=165590
> 
> Review URL: https://chromiumcodereview.appspot.com/18650006

TBR=tommycli@chromium.org

Review URL: https://codereview.chromium.org/18271011

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

base/base.gypi

@@ -701,6 +701,7 @@
                'scoped_native_library.cc',
                'files/scoped_temp_dir.cc',
                'sys_info_posix.cc',
+               'threading/sequenced_worker_pool.cc',
                'third_party/dynamic_annotations/dynamic_annotations.c',
             ],
             'sources/': [

base/threading/sequenced_worker_pool.cc

@@ -10,22 +10,21 @@
 #include <utility>
 #include <vector>
 
-#include "base/atomic_sequence_num.h"
+#include "base/atomicops.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/critical_closure.h"
 #include "base/debug/trace_event.h"
-#include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/linked_ptr.h"
 #include "base/message_loop/message_loop_proxy.h"
+#include "base/metrics/histogram.h"
 #include "base/stl_util.h"
 #include "base/strings/stringprintf.h"
 #include "base/synchronization/condition_variable.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/platform_thread.h"
 #include "base/threading/simple_thread.h"
-#include "base/threading/thread_local.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time/time.h"
 #include "base/tracked_objects.h"
@@ -34,10 +33,6 @@
 #include "base/mac/scoped_nsautorelease_pool.h"
 #endif
 
-#if !defined(OS_NACL)
-#include "base/metrics/histogram.h"
-#endif
-
 namespace base {
 
 namespace {
@@ -218,10 +213,6 @@ uint64 GetTaskTraceID(const SequencedTask& task,
          static_cast<uint64>(reinterpret_cast<intptr_t>(pool));
 }
 
-base::LazyInstance<base::ThreadLocalPointer<
-    SequencedWorkerPool::SequenceToken> > g_lazy_tls_ptr =
-        LAZY_INSTANCE_INITIALIZER;
-
 }  // namespace
 
 // Worker ---------------------------------------------------------------------
@@ -391,9 +382,8 @@ class SequencedWorkerPool::Inner {
 
   // The last sequence number used. Managed by GetSequenceToken, since this
   // only does threadsafe increment operations, you do not need to hold the
-  // lock. This is class-static to make SequenceTokens issued by
-  // GetSequenceToken unique across SequencedWorkerPool instances.
-  static base::StaticAtomicSequenceNumber g_last_sequence_number_;
+  // lock.
+  volatile subtle::Atomic32 last_sequence_number_;
 
   // This lock protects |everything in this class|. Do not read or modify
   // anything without holding this lock. Do not block while holding this
@@ -489,10 +479,6 @@ SequencedWorkerPool::Worker::~Worker() {
 }
 
 void SequencedWorkerPool::Worker::Run() {
-  // Store a pointer to the running sequence in thread local storage for
-  // static function access.
-  g_lazy_tls_ptr.Get().Set(&running_sequence_);
-
   // Just jump back to the Inner object to run the thread, since it has all the
   // tracking information and queues. It might be more natural to implement
   // using DelegateSimpleThread and have Inner implement the Delegate to avoid
@@ -511,6 +497,7 @@ SequencedWorkerPool::Inner::Inner(
     const std::string& thread_name_prefix,
     TestingObserver* observer)
     : worker_pool_(worker_pool),
+      last_sequence_number_(0),
       lock_(),
       has_work_cv_(&lock_),
       can_shutdown_cv_(&lock_),
@@ -545,9 +532,9 @@ SequencedWorkerPool::Inner::~Inner() {
 
 SequencedWorkerPool::SequenceToken
 SequencedWorkerPool::Inner::GetSequenceToken() {
-  // Need to add one because StaticAtomicSequenceNumber starts at zero, which
-  // is used as a sentinel value in SequenceTokens.
-  return SequenceToken(g_last_sequence_number_.GetNext() + 1);
+  subtle::Atomic32 result =
+      subtle::NoBarrier_AtomicIncrement(&last_sequence_number_, 1);
+  return SequenceToken(static_cast<int>(result));
 }
 
 SequencedWorkerPool::SequenceToken
@@ -628,7 +615,7 @@ bool SequencedWorkerPool::Inner::IsRunningSequenceOnCurrentThread(
   ThreadMap::const_iterator found = threads_.find(PlatformThread::CurrentId());
   if (found == threads_.end())
     return false;
-  return sequence_token.Equals(found->second->running_sequence());
+  return found->second->running_sequence().Equals(sequence_token);
 }
 
 // See https://code.google.com/p/chromium/issues/detail?id=168415
@@ -687,10 +674,8 @@ void SequencedWorkerPool::Inner::Shutdown(
     while (!CanShutdown())
       can_shutdown_cv_.Wait();
   }
-#if !defined(OS_NACL)
   UMA_HISTOGRAM_TIMES("SequencedWorkerPool.ShutdownDelayTime",
                       TimeTicks::Now() - shutdown_wait_begin);
-#endif
 }
 
 void SequencedWorkerPool::Inner::ThreadLoop(Worker* this_worker) {
@@ -887,10 +872,8 @@ SequencedWorkerPool::Inner::GetWorkStatus SequencedWorkerPool::Inner::GetWork(
     std::vector<Closure>* delete_these_outside_lock) {
   lock_.AssertAcquired();
 
-#if !defined(OS_NACL)
   UMA_HISTOGRAM_COUNTS_100("SequencedWorkerPool.TaskCount",
                            static_cast<int>(pending_tasks_.size()));
-#endif
 
   // Find the next task with a sequence token that's not currently in use.
   // If the token is in use, that means another thread is running something
@@ -975,10 +958,8 @@ SequencedWorkerPool::Inner::GetWorkStatus SequencedWorkerPool::Inner::GetWork(
   // Track the number of tasks we had to skip over to see if we should be
   // making this more efficient. If this number ever becomes large or is
   // frequently "some", we should consider the optimization above.
-#if !defined(OS_NACL)
   UMA_HISTOGRAM_COUNTS_100("SequencedWorkerPool.UnrunnableTaskCount",
                            unrunnable_tasks);
-#endif
   return status;
 }
 
@@ -1107,20 +1088,8 @@ bool SequencedWorkerPool::Inner::CanShutdown() const {
          blocking_shutdown_pending_task_count_ == 0;
 }
 
-base::StaticAtomicSequenceNumber
-SequencedWorkerPool::Inner::g_last_sequence_number_;
-
 // SequencedWorkerPool --------------------------------------------------------
 
-// static
-SequencedWorkerPool::SequenceToken
-SequencedWorkerPool::GetSequenceTokenForCurrentThread() {
-  SequencedWorkerPool::SequenceToken* token = g_lazy_tls_ptr.Get().Get();
-  if (!token)
-    return SequenceToken();
-  return *token;
-}
-
 SequencedWorkerPool::SequencedWorkerPool(
     size_t max_threads,
     const std::string& thread_name_prefix)

base/threading/sequenced_worker_pool.h

@@ -126,11 +126,6 @@ class BASE_EXPORT SequencedWorkerPool : public TaskRunner {
       return id_ == other.id_;
     }
 
-    // Returns false if current thread is executing an unsequenced task.
-    bool IsValid() const {
-      return id_ != 0;
-    }
-
    private:
     friend class SequencedWorkerPool;
 
@@ -148,11 +143,6 @@ class BASE_EXPORT SequencedWorkerPool : public TaskRunner {
     virtual void OnDestruct() = 0;
   };
 
-  // Gets the SequencedToken of the current thread.
-  // If current thread is not a SequencedWorkerPool worker thread or is running
-  // an unsequenced task, returns an invalid SequenceToken.
-  static SequenceToken GetSequenceTokenForCurrentThread();
-
   // When constructing a SequencedWorkerPool, there must be a
   // MessageLoop on the current thread unless you plan to deliberately
   // leak it.
@@ -169,7 +159,7 @@ class BASE_EXPORT SequencedWorkerPool : public TaskRunner {
                       TestingObserver* observer);
 
   // Returns a unique token that can be used to sequence tasks posted to
-  // PostSequencedWorkerTask(). Valid tokens are always nonzero.
+  // PostSequencedWorkerTask(). Valid tokens are alwys nonzero.
   SequenceToken GetSequenceToken();
 
   // Returns the sequence token associated with the given name. Calling this

base/threading/sequenced_worker_pool_unittest.cc

@@ -728,6 +728,8 @@ TEST_F(SequencedWorkerPoolTest, IsRunningOnCurrentThread) {
 
   scoped_refptr<SequencedWorkerPool> unused_pool =
       new SequencedWorkerPool(2, "unused_pool");
+  EXPECT_TRUE(token1.Equals(unused_pool->GetSequenceToken()));
+  EXPECT_TRUE(token2.Equals(unused_pool->GetSequenceToken()));
 
   EXPECT_FALSE(pool()->RunsTasksOnCurrentThread());
   EXPECT_FALSE(pool()->IsRunningSequenceOnCurrentThread(token1));