WTF::ThreadSpecific should use base::ThreadLocalStorage.

WTF::ThreadSpecific should use base::ThreadLocalStorage.

Let's use the base abstraction to allocate the
TLS. It's way less code.

Committed: https://crrev.com/56641fdb889c6b5d27997465b622d7432c0ba0b8
Cr-Commit-Position: refs/heads/master@{#384650}

[esprehn, 2016-03-31 23:57:11]

The CQ bit was checked by esprehn@chromium.org to run a CQ dry run

[esprehn, 2016-03-31 23:57:16]

The CQ bit was unchecked by esprehn@chromium.org

[esprehn, 2016-03-31 23:57:27]

The CQ bit was checked by esprehn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-31 23:57:49]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849023002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849023002/1

[commit-bot: I haz the power, 2016-04-01 00:03:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-01 00:03:05]

Dry run: Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  android_chromium_gn_compile_rel on tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_chro...)
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  linux_blink_oilpan_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_blink_oil...)
  linux_chromium_clobber_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[esprehn, 2016-04-01 00:25:36]

The CQ bit was checked by esprehn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-01 00:26:06]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849023002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849023002/20001

[esprehn, 2016-04-01 01:50:11]

The CQ bit was unchecked by esprehn@chromium.org

[esprehn, 2016-04-01 01:50:16]

The CQ bit was checked by esprehn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-01 01:50:40]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849023002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849023002/20001

[esprehn, 2016-04-01 04:11:06]

Hmm, I'm not sure I understand this one. ThreadSpecific never destroys the
object on purpose... but the leak detector now finds it. Why didn't the leak
detector find it before?

@@@STEP_LOG_LINE@HeapTest.Threading@=========================================...
@@@STEP_LOG_LINE@HeapTest.Threading@==32514==ERROR: LeakSanitizer: detected
memory leaks@@@
@@@STEP_LOG_LINE@HeapTest.Threading@@@@
@@@STEP_LOG_LINE@HeapTest.Threading@Direct leak of 80 byte(s) in 10 object(s)
allocated from:@@@
@@@STEP_LOG_LINE@HeapTest.Threading@    #0 0x4da4bb in __interceptor_malloc
(/tmp/runCVVxKR/out/Release/blink_heap_unittests+0x4da4bb)@@@
@@@STEP_LOG_LINE@HeapTest.Threading@    #1 0x505f3c in
partitionAllocGenericFlags
third_party/WebKit/Source/wtf/PartitionAlloc.h:736:20@@@
@@@STEP_LOG_LINE@HeapTest.Threading@    #2 0x505f3c in partitionAllocGeneric
third_party/WebKit/Source/wtf/PartitionAlloc.h:763@@@
@@@STEP_LOG_LINE@HeapTest.Threading@    #3 0x505f3c in
WTF::Partitions::fastMalloc(unsigned long, char const*)
third_party/WebKit/Source/wtf/Partitions.h:122@@@
@@@STEP_LOG_LINE@HeapTest.Threading@    #4 0x505d4f in fastZeroedMalloc
third_party/WebKit/Source/wtf/Partitions.h:126:24@@@
@@@STEP_LOG_LINE@HeapTest.Threading@    #5 0x505d4f in
WTF::ThreadSpecific<blink::ThreadState*>::get()
third_party/WebKit/Source/wtf/ThreadSpecific.h:76@@@
@@@STEP_LOG_LINE@HeapTest.Threading@    #6 0x943e1b in operator*
third_party/WebKit/Source/wtf/ThreadSpecific.h:61:30@@@
@@@STEP_LOG_LINE@HeapTest.Threading@    #7 0x943e1b in
blink::ThreadState::ThreadState()
third_party/WebKit/Source/platform/heap/ThreadState.cpp:117@@@
@@@STEP_LOG_LINE@HeapTest.Threading@    #8 0x9471ce in
blink::ThreadState::attach()
third_party/WebKit/Source/platform/heap/ThreadState.cpp:265:30@@@
@@@STEP_LOG_LINE@HeapTest.Threading@    #9 0x63c8a1 in
blink::ThreadedHeapTester::runThread()
third_party/WebKit/Source/platform/heap/HeapTest.cpp:518:9@@@

[esprehn, 2016-04-01 04:31:58]

The CQ bit was checked by esprehn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-01 04:32:48]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849023002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849023002/40001

[esprehn, 2016-04-01 04:42:46]

Description was changed from

==========
WTF::ThreadSpecific should use base::ThreadLocalPointer.
==========

to

==========
WTF::ThreadSpecific should use base::ThreadLocalStorage.

Let's use the base abstraction to allocate the
TLS. It's way less code.
==========

[commit-bot: I haz the power, 2016-04-01 05:31:10]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-01 05:31:11]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[esprehn, 2016-04-01 07:16:51]

The CQ bit was checked by esprehn@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-04-01 07:17:19]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849023002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849023002/60001

[commit-bot: I haz the power, 2016-04-01 08:48:56]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-04-01 08:48:57]

Dry run: This issue passed the CQ dry run.

[esprehn, 2016-04-01 11:53:41]

esprehn@chromium.org changed reviewers:
+ danakj@chromium.org, haraken@chromium.org, yutak@chromium.org

[haraken, 2016-04-01 12:28:35]

LGTM

https://codereview.chromium.org/1849023002/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/ThreadSpecific.h (right):

https://codereview.chromium.org/1849023002/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/ThreadSpecific.h:88: if (isShutdown())

I'm not sure if this check is needed though.

[esprehn, 2016-04-01 15:44:06]

On 2016/04/01 at 12:28:35, haraken wrote:
> LGTM
> 
>
https://codereview.chromium.org/1849023002/diff/60001/third_party/WebKit/Sour...
> File third_party/WebKit/Source/wtf/ThreadSpecific.h (right):
> 
>
https://codereview.chromium.org/1849023002/diff/60001/third_party/WebKit/Sour...
> third_party/WebKit/Source/wtf/ThreadSpecific.h:88: if (isShutdown())
> 
> I'm not sure if this check is needed though.

It is. See patch #3 where every bot was red because they hit an assert in
fastFree about the partition being initialized.

ASSERT(root->initialized)

[esprehn, 2016-04-01 16:35:34]

On 2016/04/01 at 15:44:06, esprehn wrote:
> On 2016/04/01 at 12:28:35, haraken wrote:
> > LGTM
> > 
> >
https://codereview.chromium.org/1849023002/diff/60001/third_party/WebKit/Sour...
> > File third_party/WebKit/Source/wtf/ThreadSpecific.h (right):
> > 
> >
https://codereview.chromium.org/1849023002/diff/60001/third_party/WebKit/Sour...
> > third_party/WebKit/Source/wtf/ThreadSpecific.h:88: if (isShutdown())
> > 
> > I'm not sure if this check is needed though.
> 
> It is. See patch #3 where every bot was red because they hit an assert in
fastFree about the partition being initialized.
> 
> ASSERT(root->initialized)

ex.
https://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel...

I think we need to check isShutdown() because the main thread will call shutdown
before the native thread is destroyed. So the destroy() method on the
ThreadSpecific ends up after the shutdown. And it's not safe to call into PA
after shutdown.

[danakj, 2016-04-01 18:52:53]

DEPS LGTM

[esprehn, 2016-04-01 18:52:59]

The CQ bit was checked by esprehn@chromium.org

[commit-bot: I haz the power, 2016-04-01 18:53:23]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1849023002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1849023002/60001

[commit-bot: I haz the power, 2016-04-01 18:59:19]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-04-01 19:01:42]

Description was changed from

==========
WTF::ThreadSpecific should use base::ThreadLocalStorage.

Let's use the base abstraction to allocate the
TLS. It's way less code.
==========

to

==========
WTF::ThreadSpecific should use base::ThreadLocalStorage.

Let's use the base abstraction to allocate the
TLS. It's way less code.

Committed: https://crrev.com/56641fdb889c6b5d27997465b622d7432c0ba0b8
Cr-Commit-Position: refs/heads/master@{#384650}
==========

[commit-bot: I haz the power, 2016-04-01 19:01:43]

Patchset 4 (id:??) landed as
https://crrev.com/56641fdb889c6b5d27997465b622d7432c0ba0b8
Cr-Commit-Position: refs/heads/master@{#384650}

[esprehn, 2016-04-04 15:19:03]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1856793002/ by esprehn@chromium.org.

The reason for reverting is: May have caused a bunch of perf regressions: 
https://bugs.chromium.org/p/chromium/issues/detail?id=600271.