Make Supplement and Supplementable live and die together

third_party/WebKit/Source/platform/Supplementable.h

 /*
  * Copyright (C) 2012 Google, Inc. All Rights Reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 75 matching lines @@
 //
 // Note that reattachThread() does nothing if assertion is not enabled.
 //
 
 template<typename T>
 class Supplementable;
 
 template<typename T>
 class Supplement : public GarbageCollectedMixin {
 public:
+    // TODO(haraken): Remove this default constructor.

[haraken, 2016/07/08 06:37:29]

I'll fix the TODO before committing. But I want to confirm if you're okay with
this change before spending time updating all Supplement constructors.

[sof, 2016/07/08 07:07:02]

Before Oilpan, Supplementable provided OwnPtr<>-ownership of its Supplements
along with on-demand/lazy creation of the supplements.

With Oilpan, we clearly want the on-demand part, but we need to decide how
ownership should be handled. What you're providing here will make them _all_ die
at the same time, but a reference to one Supplement<> will keep everything alive
-- how do you plan to address that?

I would assume the codebase is pretty much well-behaved in its current state wrt
not holding onto supplements, but it seems like something that could easily
slide & regress, without it being noticed (by the leak detector.)

An abrupt Supplementable detach step might help here?

[haraken, 2016/07/08 07:17:37]

Thanks for the details.

I feel that it would make sense to make _all_ Supplements and the Supplementable
live and die together because they're conceptually the same object. So if
someone keeps a strong reference to the Supplement and causes leaks, I'd say
that it's a bug to be detected by the leak detector.

But I don't have a strong opinion here. If you worry about such leaks, I'm fine
with making the reference a WeakMember<T>.

+    Supplement() : m_host(nullptr) { }
+    explicit Supplement(T* host) : m_host(host) { }
+    T* host() const { return m_host; }
+
     static void provideTo(Supplementable<T>& host, const char* key, Supplement<T>* supplement)
     {
         host.provideSupplement(key, supplement);
     }
 
     static Supplement<T>* from(Supplementable<T>& host, const char* key)
     {
         return host.requireSupplement(key);
     }
 
     static Supplement<T>* from(Supplementable<T>* host, const char* key)
     {
         return host ? host->requireSupplement(key) : 0;
     }
+
+    DEFINE_INLINE_VIRTUAL_TRACE()
+    {
+        visitor->trace(m_host);
+    }
+
+private:
+    Member<T> m_host;
 };
 
 // Supplementable<T> inherits from GarbageCollectedMixin virtually
 // to allow ExecutionContext to derive from two GC mixin classes.
 template<typename T>
 class Supplementable : public virtual GarbageCollectedMixin {
     WTF_MAKE_NONCOPYABLE(Supplementable);
 public:
     void provideSupplement(const char* key, Supplement<T>* supplement)
     {
@@ skipping 49 matching lines @@
 };
 
 template<typename T>
 struct ThreadingTrait<Supplementable<T>> {
     static const ThreadAffinity Affinity = ThreadingTrait<T>::Affinity;
 };
 
 } // namespace blink
 
 #endif // Supplementable_h