Delete persisted plugin when detaching without reattach.

A plugin kept through a reattachment operation isn't processed and hooked up immediately (synchronously) when attaching the layout object, so it's possible that it might still be around if the element is detached again. If it gets detached without being reattached, we'd never get around to deleting the persisted plugin. The test added is not a WPT, because it uses an internal application/x-blink-test-plugin thing. Bug: 962790 Change-Id: If33126bdd54b613fc4df82a738f77b3b312cb318 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1614197 Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#660338}

Delete persisted plugin when detaching without reattach.
A plugin kept through a reattachment operation isn't processed and hooked up immediately (synchronously) when attaching the layout object, so it's possible that it might still be around if the element is detached again. If it gets detached without being reattached, we'd never get around to deleting the persisted plugin. The test added is not a WPT, because it uses an internal application/x-blink-test-plugin thing. Bug: 962790 Change-Id: If33126bdd54b613fc4df82a738f77b3b312cb318 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1614197 Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/master@{#660338}
4292bebb · Morten Stenshorne · Commit Bot · 48a23d74 · 4292bebb · 4292bebb
Commit 4292bebb authored May 16, 2019 by Morten Stenshorne Committed by Commit Bot May 16, 2019
2 changed files
--- a/third_party/blink/renderer/core/html/html_plugin_element.cc
+++ b/third_party/blink/renderer/core/html/html_plugin_element.cc
@@ -346,13 +346,23 @@ void HTMLPlugInElement::DetachLayoutTree(const AttachContext& context) {
    GetDocument().DecrementLoadEventDelayCount();
  }

+  bool keep_plugin = context.performing_reattach && !dispose_view_;
+
  // Only try to persist a plugin we actually own.
  WebPluginContainerImpl* plugin = OwnedPlugin();
-  if (plugin && context.performing_reattach && !dispose_view_) {
+  if (plugin && keep_plugin) {
    SetPersistedPlugin(ToWebPluginContainerImpl(ReleaseEmbeddedContentView()));
  } else {
+    // A persisted plugin isn't processed and hooked up immediately
+    // (synchronously) when attaching the layout object, so it's possible that
+    // it's still around. That's fine if we're allowed to keep it. Otherwise,
+    // get rid of it now.
+    if (persisted_plugin_ && !keep_plugin)
+      SetPersistedPlugin(nullptr);
+
    // Clear the plugin; will trigger disposal of it with Oilpan.
-    SetEmbeddedContentView(nullptr);
+    if (!persisted_plugin_)
+      SetEmbeddedContentView(nullptr);
  }

  // We should attempt to use the same view afterwards, so that we don't lose

--- a/third_party/blink/web_tests/plugins/remove-reattached-plugin-crash.html
+++ b/third_party/blink/web_tests/plugins/remove-reattached-plugin-crash.html
+<!DOCTYPE html>
+<object id="plugin" type="application/x-blink-test-plugin"></object>
+<script src="../resources/testharness.js"></script>
+<script src="../resources/testharnessreport.js"></script>
+<script>
+  test(()=> {
+    document.body.offsetTop;
+    plugin.style.display = "block";
+    document.body.offsetTop;
+    plugin.parentNode.removeChild(plugin);
+  }, "no crash or DCHECK failure");
+</script>