Move plugins to be stored in HTMLPlugInElement.

third_party/WebKit/Source/core/html/HTMLPlugInElement.cpp

 /**
  * Copyright (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  *           (C) 2000 Stefan Schimanski (1Stein@gmx.de)
  * Copyright (C) 2004, 2005, 2006 Apple Computer, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
@@ skipping 74 matching lines @@
       m_shouldPreferPlugInsForImages(preferPlugInsForImagesOption ==
                                      ShouldPreferPlugInsForImages) {}
 
 HTMLPlugInElement::~HTMLPlugInElement() {
   DCHECK(!m_pluginWrapper);  // cleared in detachLayoutTree()
   DCHECK(!m_isDelayingLoadEvent);
 }
 
 DEFINE_TRACE(HTMLPlugInElement) {
   visitor->trace(m_imageLoader);
+  visitor->trace(m_plugin);
   visitor->trace(m_persistedPlugin);
   HTMLFrameOwnerElement::trace(visitor);
 }
 
-// TODO(joelhockey): Move implementation of HTMLFrameOwnerElement
-// setWidget/releaseWidget/ownedWidget that relates to plugins to here and
-// remove inheritance from PluginView to FrameViewBase.
-void HTMLPlugInElement::setPlugin(PluginView* pluginView) {
-  setWidget(pluginView);
+void HTMLPlugInElement::setPlugin(PluginView* plugin) {
+  if (plugin == m_plugin)
+    return;
+
+  // Remove and dispose the old plugin if we had one.
+  if (m_plugin) {

[dcheng, 2017/03/27 22:30:42]

I haven't looked at this part in detail, but I looked over the rest of the CL.

Would we eventually be able to clean up more of
HTMLFrameOwnerElement::setWidget()? Is that blocked on something else atm?

[joelhockey, 2017/03/28 22:56:23]

There is definitely a cleaner way that I can handle some of that function. 
Rather than 2 calls to moveToParentSoon with some funny inputs like nullptr, I
can make calls to methods like add/remove.

But from what I can tell, the code in HTMLFrameOwnerElement::setWidget is
required.

+    document().view()->removePlugin(m_plugin);

[haraken, 2017/03/28 07:39:41]

Don't you need to do something like moveWidgetToParentSoon? i.e., take care of
s_updateSuspendCount.

[joelhockey, 2017/03/28 22:56:23]

You might be right, but I thought that it is only the call on dispose that must
be careful to wait until after any suspends since it may trigger calls to JS.

I think changing the frame/plugin parent/child references is fine to happen at
any time since the purpose of this parent/child relationship is for
FrameView::updateStyleAndLayoutIfNeededRecursiveInternal to call
WebPluginContainerImpl::updateAllLifecyclePhases for all children plugins.  My
guess (and it is not very well informed, I admit) was that a failure to call
updateAllLifecyclePhases on a plugin which is just about to be disposed will not
be a problem.  And likewise, calling it on a newly created plugin is not an
issue.  Please do correct me if this is wrong, or if you want me to delay the
calls to document.view()-.[add|remove]Plugin to be outside the suspend, I can
make that change.

[haraken, 2017/03/29 11:04:46]

I'll defer this part to dcheng@, who should be more familiar with the subtlety.

[dcheng, 2017/03/30 04:43:07]

I'm willing to try this and see what happens--if it doesn't break anything, I
agree that it would probably be simpler overall.

[joelhockey, 2017/03/30 06:40:56]

Ack

[haraken, 2017/03/30 14:29:12]

If this breaks something, what will break?

Actually I don't fully understand what the moveWidgetToParentSoon is doing.
i.e., why we had to introduce s_updateSuspendCount.

[joelhockey, 2017/03/31 01:03:55]

I assume this question is for dcheng.

I see some description from dcheng at https://codereview.chromium.org/1444183003

It looks like this is where the code was first added.
https://chromium.googlesource.com/chromium/src/+/102e6f3cf3e6e849f2e7ef6ebecf...

I'm going to check if any tests fail if I remove the suspend.

+    disposeWidgetSoon(m_plugin);
+  }
+  m_plugin = plugin;
+
+  // TODO(joelhockey): I copied the rest of this method from
+  // HTMLFrameOwnerElement.  I don't really know what it does.
+  // Are the layoutPartItem.isNull check and DCHECKs required?
+  LayoutPart* layoutPart = toLayoutPart(layoutObject());
+  LayoutPartItem layoutPartItem = LayoutPartItem(layoutPart);
+  if (layoutPartItem.isNull())
+    return;
+
+  // Update layout and frame with new plugin.
+  if (m_plugin) {
+    layoutPartItem.updateOnWidgetChange();
+
+    DCHECK_EQ(document().view(), layoutPartItem.frameView());
+    DCHECK(layoutPartItem.frameView());
+    plugin->setParent(layoutPartItem.frameView());
+    document().view()->addPlugin(plugin);
+  }
+
+  // Apparently accessibility objects might have been modified if plugin
+  // was removed.
+  if (AXObjectCache* cache = document().existingAXObjectCache())
+    cache->childrenChanged(layoutPart);
 }
 
 PluginView* HTMLPlugInElement::releasePlugin() {
-  FrameViewBase* plugin = releaseWidget();
-  return plugin && plugin->isPluginView() ? toPluginView(plugin) : nullptr;
-}
-
-PluginView* HTMLPlugInElement::ownedPlugin() const {
-  FrameViewBase* plugin = ownedWidget();
-  return plugin && plugin->isPluginView() ? toPluginView(plugin) : nullptr;
+  if (!m_plugin)
+    return nullptr;
+  document().view()->removePlugin(m_plugin);

[haraken, 2017/03/28 07:39:41]

Don't we need to do something like temporarilyRemoveWidgetFromParentSoon? i.e.,
take care of s_updateSuspendCount.

[joelhockey, 2017/03/28 22:56:23]

same as above

+  LayoutPart* layoutPart = toLayoutPart(layoutObject());
+  if (layoutPart) {
+    if (AXObjectCache* cache = document().existingAXObjectCache())
+      cache->childrenChanged(layoutPart);
+  }
+  return m_plugin.release();
 }
 
 void HTMLPlugInElement::setPersistedPlugin(PluginView* plugin) {
   if (m_persistedPlugin == plugin)
     return;
   if (m_persistedPlugin) {
     m_persistedPlugin->hide();
     disposeWidgetSoon(m_persistedPlugin.release());
   }
   m_persistedPlugin = plugin;
@@ skipping 22 matching lines @@
     // new frame and set it as the LayoutPart's FrameViewBase, causing what was
     // previously in the FrameViewBase to be torn down.
     return loadOrRedirectSubframe(completedURL, getNameAttribute(), true);
   }
 
   return loadPlugin(completedURL, mimeType, paramNames, paramValues,
                     useFallback, true);
 }
 
 bool HTMLPlugInElement::canProcessDrag() const {
-  return pluginWidget() && pluginWidget()->isPluginView() &&
-         toPluginView(pluginWidget())->canProcessDrag();
+  return pluginWidget() && pluginWidget()->canProcessDrag();
 }
 
 bool HTMLPlugInElement::canStartSelection() const {
   return useFallbackContent() && Node::canStartSelection();
 }
 
 bool HTMLPlugInElement::willRespondToMouseClickEvents() {
   if (isDisabledFormControl())
     return false;
   LayoutObject* r = layoutObject();
   return r && (r->isEmbeddedObject() || r->isLayoutPart());
 }
 
 void HTMLPlugInElement::removeAllEventListeners() {
   HTMLFrameOwnerElement::removeAllEventListeners();
-  if (LayoutPart* layoutObject = existingLayoutPart()) {
-    if (FrameViewBase* frameViewBase = layoutObject->frameViewBase())
-      frameViewBase->eventListenersRemoved();
+  if (m_plugin) {
+    m_plugin->eventListenersRemoved();
   }
 }
 
 void HTMLPlugInElement::didMoveToNewDocument(Document& oldDocument) {
   if (m_imageLoader)
     m_imageLoader->elementDidMoveToNewDocument();
   HTMLFrameOwnerElement::didMoveToNewDocument(oldDocument);
 }
 
 void HTMLPlugInElement::attachLayoutTree(const AttachContext& context) {
@@ skipping 71 matching lines @@
   Vector<String> paramValues;
 
   paramNames.push_back("type");
   paramValues.push_back(m_serviceType);
 
   bool useFallback = false;
   loadPlugin(url, m_serviceType, paramNames, paramValues, useFallback, false);
 }
 
 bool HTMLPlugInElement::shouldAccelerate() const {
-  return ownedPlugin() && ownedPlugin()->platformLayer();
+  return m_plugin && m_plugin->platformLayer();
 }
 
 void HTMLPlugInElement::detachLayoutTree(const AttachContext& context) {
   // Update the FrameViewBase the next time we attach (detaching destroys the
   // plugin).
   // FIXME: None of this "needsPluginUpdate" related code looks right.
   if (layoutObject() && !useFallbackContent())
     setNeedsPluginUpdate(true);
 
   if (m_isDelayingLoadEvent) {
     m_isDelayingLoadEvent = false;
     document().decrementLoadEventDelayCount();
   }
 
   // Only try to persist a plugin we actually own.
-  PluginView* plugin = ownedPlugin();
-  if (plugin && context.performingReattach) {
+  if (m_plugin && context.performingReattach) {
     setPersistedPlugin(releasePlugin());
   } else {
     // Clear the plugin; will trigger disposal of it with Oilpan.
     setPlugin(nullptr);
   }
 
   resetInstance();
 
   HTMLFrameOwnerElement::detachLayoutTree(context);
 }
@@ skipping 45 matching lines @@
       plugin = m_persistedPlugin.get();
     else
       plugin = pluginWidget();
 
     if (plugin)
       m_pluginWrapper = frame->script().createPluginWrapper(plugin);
   }
   return m_pluginWrapper.get();
 }
 
-FrameViewBase* HTMLPlugInElement::pluginWidget() const {
+// TODO(joelhockey): Remove this and use m_plugin.
+// Maybe just pluginWrapper function should check if m_plugin is null,
+// and call document().updateStyleAndLayoutIgnorePendingStylesheets as per
+// layoutPartForJSBindings.
+PluginView* HTMLPlugInElement::pluginWidget() const {
   if (LayoutPart* layoutPart = layoutPartForJSBindings())
-    return layoutPart->frameViewBase();
+    return layoutPart->plugin();
   return nullptr;
 }
 
+PluginView* HTMLPlugInElement::plugin() const {
+  return m_plugin.get();
+}
+
 bool HTMLPlugInElement::isPresentationAttribute(
     const QualifiedName& name) const {
   if (name == widthAttr || name == heightAttr || name == vspaceAttr ||
       name == hspaceAttr || name == alignAttr)
     return true;
   return HTMLFrameOwnerElement::isPresentationAttribute(name);
 }
 
 void HTMLPlugInElement::collectStyleForPresentationAttribute(
     const QualifiedName& name,
@@ skipping 29 matching lines @@
   // code in EventHandler; these code paths should be united.
 
   LayoutObject* r = layoutObject();
   if (!r || !r->isLayoutPart())
     return;
   if (r->isEmbeddedObject()) {
     if (LayoutEmbeddedItem(toLayoutEmbeddedObject(r))
             .showsUnavailablePluginIndicator())
       return;
   }
-  FrameViewBase* frameViewBase = toLayoutPart(r)->frameViewBase();
-  if (!frameViewBase)
+  if (!m_plugin)
     return;
-  frameViewBase->handleEvent(event);
+  m_plugin->handleEvent(event);
   if (event->defaultHandled())
     return;
   HTMLFrameOwnerElement::defaultEventHandler(event);
 }
 
 LayoutPart* HTMLPlugInElement::layoutPartForJSBindings() const {
   // Needs to load the plugin immediatedly because this function is called
   // when JavaScript code accesses the plugin.
   // FIXME: Check if dispatching events here is safe.
   document().updateStyleAndLayoutIgnorePendingStylesheets(
       Document::RunPostLayoutTasksSynchronously);
   return existingLayoutPart();
 }
 
 bool HTMLPlugInElement::isKeyboardFocusable() const {
   if (HTMLFrameOwnerElement::isKeyboardFocusable())
     return true;
   return document().isActive() && pluginWidget() &&
-         pluginWidget()->isPluginView() &&
-         toPluginView(pluginWidget())->supportsKeyboardFocus();
+         pluginWidget()->supportsKeyboardFocus();
 }
 
 bool HTMLPlugInElement::hasCustomFocusLogic() const {
   return !useFallbackContent();
 }
 
 bool HTMLPlugInElement::isPluginElement() const {
   return true;
 }
 
@@ skipping 219 matching lines @@
 
 void HTMLPlugInElement::lazyReattachIfNeeded() {
   if (!useFallbackContent() && needsPluginUpdate() && layoutObject() &&
       !isImageType()) {
     lazyReattachIfAttached();
     setPersistedPlugin(nullptr);
   }
 }
 
 }  // namespace blink