PlatformThreadHandle: remove public id() interface

base/threading/thread.h

Index: base/threading/thread.h
diff --git a/base/threading/thread.h b/base/threading/thread.h
index 5126491b3861cdb60f23eec1b40b11466d27a33a..a0607d9076cf693ccd3786c063e0057e25b676b1 100644
--- a/base/threading/thread.h
+++ b/base/threading/thread.h
@@ -170,7 +170,7 @@ class BASE_EXPORT Thread : PlatformThread::Delegate {
   // The native thread handle.
   PlatformThreadHandle thread_handle() { return thread_; }
 
-  // The thread ID.
+  // The thread ID. Only valid after this Thread was started.
   PlatformThreadId thread_id() const;
 
   // Returns true if the thread has been started, and not yet stopped.
@@ -212,15 +212,21 @@ class BASE_EXPORT Thread : PlatformThread::Delegate {
 
   // If true, we're in the middle of stopping, and shouldn't access
   // |message_loop_|. It may non-NULL and invalid.
+  // Should be written on the thread that creates this thread. Also read data
+  // could be wrong on other threads.
   bool stopping_;
 
   // True while inside of Run().
   bool running_;
-  mutable base::Lock running_lock_;  // Protects running_.
 
   // The thread's handle.
   PlatformThreadHandle thread_;
-  mutable base::Lock thread_lock_;  // Protects thread_.
+
+  // The thread's id once it was started.
+  PlatformThreadId id_;
+
+  // Protects |running_|, |thread_|, and |id_|.

[kinuko, 2015/07/16 00:50:37]

nit: This no longer protects thread_?

nit: Unless the protected fields have close relationships or we have very severe
memory limit I prefer using separate locks for each field.

[Takashi Toyoshima, 2015/07/16 05:07:31]

Ah, good catch. It's true from the last patch set.

I changed to share one lock because gab@ suggested in the previous review, but
I'd follow kinuko's suggestion in the next patch set unless base/ owner object
it. Since kinuko is a content/ owner that is one of the most authoritative part
in chrome.

[Takashi Toyoshima, 2015/07/16 08:12:38]

So one reason to use a separate lock for id_ is that thread_id() is called from
many places for DCHECK and so on. So, sharing lock can easily introduce
recursive lock on changing this class.

+  mutable base::Lock lock_;
 
   // The thread's message loop.  Valid only while the thread is alive.  Set
   // by the created thread.