Oilpan: Add ThreadState cleanup callbacks for static thread-specific persistent.

third_party/WebKit/Source/platform/heap/HeapThreadSpecific.h

+// Copyright 2016 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef HeapThreadSpecific_h
+#define HeapThreadSpecific_h
+
+#include "platform/heap/Handle.h"
+#include "platform/heap/ThreadState.h"
+#include "wtf/Functional.h"
+
+namespace blink {
+
+// Requests that the thread state clear a persistent handle when the thread
+// shuts down. This is intended for use with ThreadSpecific<Persistent<T>>.
+// It's important that the Persistent<T> exist until then, because this takes a

[haraken, 2016/04/14 02:36:23]

exist => exists

I'm okay with putting the assumption (i.e., the Persistent<T> exists until
shutdown) at this point, but this is a TODO.

A better fix would be:

1) Introduce ThreadSpecificnessPersistentConfiguration to Handle.h.

2) Modify PersistentBase::trace like this:

  void PersistentBase::trace(Visitor* visitor) {
    if (threadSpecificConfiguration == ThreadSpecificPersistentConfiguration &&
visitor->isTerminationGC()) {
      // Don't trace the persistent handle.
    }
    ...;
  }

[xlai (Olivia), 2016/04/14 16:04:46]

Pardon me for popping a question here:

The reason why https://codereview.chromium.org/1870503002/ doesn't work is that
the destroy of ThreadSpecific instances happens after ThreadState::cleanup(), in
which it checks and finds that there're some uncleaned Persistent handles (e.g.
a static instance ThreadSpecific<Persistent<CSSValuePool>>). Jeremy's patch
tries to do clean-up on this special-case Persistent handles. But what you
suggested here, in my understanding, is not to trace these special-case
Persistent handles at all. Would that be too inconsistent with other Persistent
handles? 

[jbroman, 2016/04/14 19:15:28]

I believe "exist" is correct here, due to the subjunctive mood (e.g.
https://en.wikipedia.org/wiki/English_subjunctive has examples of the form "it's
important that...").
I did consider this (and actually tried it, now that you've mentioned it).

Some reasons for the approach in this patchset:
1. it's less intrusive in the PersistentNode code:
  a) to actually fix the assertion in ThreadState, thread-linked persistent like
this have to be excluded from the count of remaining persistent handles, which
means that the number of these must be known to the PersistentRegion, and
  b) there is another assertion in ~PersistentNode (which happens while the
thread is shutting down, but before the system destroys thread-locals) that
fails if any slot is still used at that time
2) it doesn't allow the dangling Persistent to exist between the GC and thread
termination

+// raw pointer to that handle.
+//
+// Example:
+//   Foo& sharedFoo()
+//   {
+//        DEFINE_THREAD_SAFE_STATIC_LOCAL(
+//            ThreadSpecific<Persistent<Foo>>, threadSpecificFoo,
+//            new ThreadSpecific<Persistent<Foo>>);
+//        Persistent<Foo>& fooHandle = *threadSpecificFoo;
+//        if (!fooHandle) {
+//            fooHandle = new Foo;
+//            registerStaticThreadSpecificPersistent(fooHandle);
+//        }

[haraken, 2016/04/14 02:36:23]

I might prefer introducing Persistent<>::clearOnThreadShutdown so that we can
write the code as:

  fooHandle->clearOnThreadShutdown();

[jbroman, 2016/04/14 19:15:28]

OK, done.

+//        return *fooHandle;
+//   }
+template <typename T>
+void registerStaticThreadSpecificPersistent(Persistent<T>& handle)
+{
+    void (*closure)(Persistent<T>*) = [](Persistent<T>* handle)
+    {
+        *handle = nullptr;
+    };
+    ThreadState::current()->registerCleanupHook(WTF::bind(closure, &handle));
+}
+
+} // namespace blink
+
+#endif // HeapThreadSpecific_h