Dispose a persisted plugin widget if the content frame is detached.

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 191 matching lines @@
   if (m_isDelayingLoadEvent) {
     m_isDelayingLoadEvent = false;
     document().decrementLoadEventDelayCount();
   }
 }
 
 void HTMLPlugInElement::removedFrom(ContainerNode* insertionPoint) {
   // If we've persisted the plugin and we're removed from the tree then
   // make sure we cleanup the persistance pointer.
   if (m_persistedPluginWidget) {
+    // TODO(dcheng): This UpdateSuspendScope doesn't seem to provide much;

[sof, 2016/12/17 15:01:53]

Given that the persisted-widget detach happens slightly earlier now, is this
override still needed?

[dcheng, 2016/12/17 20:20:02]

For now, yes. If I understand this correctly, PluginViews won't be detached by
disconnectContentFrame(). My long-term plan is to move  widget disposal into
ChildFrameDisconnector, which will allow this to be removed. I also plan on
making plugin loads respect SubframeLoadingDisabler.

The eventual goal is to remove deferred widget updates altogether, and change
UpdateSuspendScope to AssertNoWidgetUpdatesScope. However, I'm still trying to
understand FrameView::updateWidgets, which (usually) run in a timer but can
occasionally run synchronously when post layout tasks are flushed. I'm hoping
that in all cases where we run updateWidgets, we don't need to suspend widget
updates... but I'm not actually sure how this will work out in practice...

[sof, 2016/12/17 21:47:28]

WebPluginContainerImpl won't separately call removedFrom(), as far as i know.

[dcheng, 2016/12/18 06:40:38]

WebPluginContainerImpl won't, but things like ContainerNode::removeChild will
detach the layout tree (which may persist a widget), and then notify the Node
that it was removed (which is when we clear any persisted widget). This path is
not covered by frame disconnection.

+    // investigate removing it.
     HTMLFrameOwnerElement::UpdateSuspendScope suspendWidgetHierarchyUpdates;
     setPersistedPluginWidget(nullptr);
   }
   HTMLFrameOwnerElement::removedFrom(insertionPoint);
 }
 
 void HTMLPlugInElement::requestPluginCreationWithoutLayoutObjectIfPossible() {
   if (m_serviceType.isEmpty())
     return;
 
@@ skipping 193 matching lines @@
 }
 
 bool HTMLPlugInElement::hasCustomFocusLogic() const {
   return !useFallbackContent();
 }
 
 bool HTMLPlugInElement::isPluginElement() const {
   return true;
 }
 
+void HTMLPlugInElement::disconnectContentFrame() {
+  HTMLFrameOwnerElement::disconnectContentFrame();
+  setPersistedPluginWidget(nullptr);

[sof, 2016/12/17 15:01:53]

If there is a reason for this (unexpected) ordering (to avoid an
UpdateSuspendScope..?), could you document it?

[dcheng, 2016/12/17 20:20:02]

I /think/ that removedFrom() isn't allowed to run script, while
disconnectContentFrame() is. Given that, it seems safer to clear the persisted
widget afterwards. I'll update this comment when I move widget disposal into
ChildFrameDisconnector.

+}
+
 bool HTMLPlugInElement::layoutObjectIsFocusable() const {
   if (HTMLFrameOwnerElement::supportsFocus() &&
       HTMLFrameOwnerElement::layoutObjectIsFocusable())
     return true;
 
   if (useFallbackContent() || !HTMLFrameOwnerElement::layoutObjectIsFocusable())
     return false;
   return m_pluginIsAvailable;
 }
 
@@ skipping 206 matching lines @@
 
 void HTMLPlugInElement::lazyReattachIfNeeded() {
   if (!useFallbackContent() && needsWidgetUpdate() && layoutObject() &&
       !isImageType()) {
     lazyReattachIfAttached();
     setPersistedPluginWidget(nullptr);
   }
 }
 
 }  // namespace blink