Base: Don't overwrite the thread id when ThreadMain exits. There's

Base: Don't overwrite the thread id when ThreadMain exits. There's
no need to do that, and it makes debugging easier to have the value.

Change the logic of IsRunning() to use a dedicated member variable 
instead of overloading the thread id. 

BUG=127931
TEST=none
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=138119

[willchan no longer on Chromium, 2012-05-21 01:33:17]

LGTM, although your changelist description is wrong and should be fixed
prior to committing.

On Fri, May 18, 2012 at 8:08 PM, <rvargas@chromium.org> wrote:

> Reviewers: willchan,
>
> Description:
> Base: Don't overwrite the thread id when ThreadMain exits. There's
> no need to do that, and it makes debugging easier to have the value.
>
> BUG=127931
> TEST=none
>
> Please review this at
https://chromiumcodereview.**appspot.com/10399097/<https://chromiumcodereview...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src/<http://svn.chromium.org/chrome/trunk/src/>
>
> Affected files:
>  M     base/threading/thread.h
>  M     base/threading/thread.cc
>
>
> Index: base/threading/thread.cc
> ==============================**==============================**=======
> --- base/threading/thread.cc    (revision 138018)
> +++ base/threading/thread.cc    (working copy)
> @@ -47,6 +47,7 @@
>  Thread::Thread(const char* name)
>     : started_(false),
>       stopping_(false),
> +      running_(false),
>       startup_data_(NULL),
>       thread_(0),
>       message_loop_(NULL),
> @@ -124,6 +125,10 @@
>   message_loop_->PostTask(FROM_**HERE, base::Bind(&ThreadQuitHelper))**;
>  }
>
> +bool Thread::IsRunning() const {
> +  return running_;
> +}
> +
>  void Thread::Run(MessageLoop* message_loop) {
>   message_loop->Run();
>  }
> @@ -160,7 +165,9 @@
>     // startup_data_ can't be touched anymore since the starting thread is
> now
>     // unlocked.
>
> +    running_ = true;
>     Run(message_loop_);
> +    running_ = false;
>
>     // Let the thread do extra cleanup.
>     CleanUp();
> @@ -171,7 +178,6 @@
>     // We can't receive messages anymore.
>     message_loop_ = NULL;
>   }
> -  thread_id_ = kInvalidThreadId;
>  }
>
>  }  // namespace base
> Index: base/threading/thread.h
> ==============================**==============================**=======
> --- base/threading/thread.h     (revision 138018)
> +++ base/threading/thread.h     (working copy)
> @@ -131,8 +131,7 @@
>   PlatformThreadId thread_id() const { return thread_id_; }
>
>   // Returns true if the thread has been started, and not yet stopped.
> -  // When a thread is running, |thread_id_| is a valid id.
> -  bool IsRunning() const { return thread_id_ != kInvalidThreadId; }
> +  bool IsRunning() const;
>
>  protected:
>   // Called just prior to starting the message loop
> @@ -164,6 +163,9 @@
>   // |message_loop_|. It may non-NULL and invalid.
>   bool stopping_;
>
> +  // True while inside of Run().
> +  bool running_;
> +
>   // Used to pass data to ThreadMain.
>   struct StartupData;
>   StartupData* startup_data_;
>
>
>

[commit-bot: I haz the power, 2012-05-21 04:34:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rvargas@chromium.org/10399097/1

[commit-bot: I haz the power, 2012-05-21 06:18:30]

Try job failure for 10399097-1 (retry) (retry) on linux_rel for step
"browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...