Move the Widget hierarchy to the Oilpan heap.

Source/web/WebHelperPluginImpl.cpp

 /*
  * 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:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 37 matching lines @@
     if (!toWebHelperPluginImpl(plugin.get())->initialize(pluginType, toWebLocalFrameImpl(frame)))
         return 0;
     return plugin.leakPtr();
 }
 
 WebHelperPluginImpl::WebHelperPluginImpl()
     : m_destructionTimer(this, &WebHelperPluginImpl::reallyDestroy)
 {
 }
 
 WebHelperPluginImpl::~WebHelperPluginImpl()

[haraken, 2014/10/11 17:33:03]

Just to confirm: Isn't it possible that ~WebHelperPluginImpl() is called during
an Oilpan's GC? (WebHelperPluginImpl is off-heap, but it might be possible that
the last reference to the WebHelperPluginImpl is lost when finalizing some
on-heap object.)

[sof, 2014/10/12 08:16:22]

WebHelperPlugin is a wrapping layer that the embedder holds onto and explicitly
deletes. I think we have to assume that what you describe can somehow happen, in
the general case. (I can't make out how via the EME player code that uses it
atm.)

So, if we end up in ~WebHelperPluginImpl and call dispose() on the Persistent<>
plugin container reference _and_ the LocalFrame of the plugin container will
have been finalized already, then things would break.

Is that really possible, i.e., that the two Persistent<>s to the plugin
container and the object/plugin element won't keep the LocalFrame alive until
they're released (by ~WebHelperPluginImpl)? And would only be sweepable during
the next GC. 

I'm assuming not; if you think that's not sound somehow, we could shore it up
adding a Persistent<> over the LocalFrame they're created in.

[haraken, 2014/10/12 12:56:04]

Thanks, understood.

Can we add an ASSERT to verify that something unexpected doesn't actually
happen? Probably we can add ASSERT(!ThreadState::SweepInProgress()).

[sof, 2014/10/12 18:35:36]

I'm not super comfortable with having such an assert around longer term, but I
think we need to understand the case better if it really does arise, so added
it.

 {
+#if ENABLE(OILPAN)
+    if (m_pluginContainer)
+        m_pluginContainer->dispose();
+#endif
 }
 
 bool WebHelperPluginImpl::initialize(const String& pluginType, WebLocalFrameImpl* frame)
 {
     ASSERT(!m_objectElement && !m_pluginContainer);
     if (!frame->frame()->loader().client())
         return false;
 
     m_objectElement = HTMLObjectElement::create(*frame->frame()->document(), 0, false);
     Vector<String> attributeNames;
     Vector<String> attributeValues;
     ASSERT(frame->frame()->document()->url().isValid());
-    m_pluginContainer = adoptRef(toWebPluginContainerImpl(frame->frame()->loader().client()->createPlugin(
+    m_pluginContainer = adoptRefWillBeNoop(toWebPluginContainerImpl(frame->frame()->loader().client()->createPlugin(
         m_objectElement.get(),
         frame->frame()->document()->url(),
         attributeNames,
         attributeValues,
         pluginType,
         false,
         FrameLoaderClient::AllowDetachedPlugin).leakRef()));
 
     if (!m_pluginContainer)
         return false;
@@ skipping 18 matching lines @@
 }
 
 WebPlugin* WebHelperPluginImpl::getPlugin()
 {
     ASSERT(m_pluginContainer);
     ASSERT(m_pluginContainer->plugin());
     return m_pluginContainer->plugin();
 }
 
 } // namespace blink