Gracefully handle a disconnected popup menu client on disposing open popup.
If the client of a PopupMenu disconnects itself before destructing a
currently open popup menu (and its page), handle the fact that the client
is not reachable for notifications.
R=keishi,haraken
BUG=457383
Review URL: https://codereview.chromium.org/920433002
git-svn-id: svn://svn.chromium.org/blink/trunk@189983 bbb929c8-8fbe-4397-9dbb-9b2b20218538
diff --git a/third_party/WebKit/LayoutTests/fast/forms/resources/popup-no-crash.html b/third_party/WebKit/LayoutTests/fast/forms/resources/popup-no-crash.html
new file mode 100644
index 0000000..9eb61bb
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/forms/resources/popup-no-crash.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<html>
+<script src="../../../resources/js-test.js"></script>
+<script>
+description("Check that first disconnecting then closing a page popup does not crash.");
+testPassed("no crash");
+
+testRunner.notifyDone();
+</script>
+</html>
diff --git a/third_party/WebKit/LayoutTests/fast/forms/select-popup-close-no-crash-expected.txt b/third_party/WebKit/LayoutTests/fast/forms/select-popup-close-no-crash-expected.txt
new file mode 100644
index 0000000..61d484c
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/forms/select-popup-close-no-crash-expected.txt
@@ -0,0 +1,10 @@
+Check that first disconnecting then closing a page popup does not crash.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS no crash
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/third_party/WebKit/LayoutTests/fast/forms/select-popup-close-no-crash.html b/third_party/WebKit/LayoutTests/fast/forms/select-popup-close-no-crash.html
new file mode 100644
index 0000000..87b7f2b
--- /dev/null
+++ b/third_party/WebKit/LayoutTests/fast/forms/select-popup-close-no-crash.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="resources/picker-common.js"></script>
+</head>
+<body>
+<select id="menu">
+<option>foo</option>
+<option>bar</option>
+</select>
+<form action="resources/popup-no-crash.html" id="form1"></form>
+<script>
+if (window.testRunner) {
+ testRunner.waitUntilDone();
+ testRunner.dumpAsText();
+}
+
+function openCallback() {
+ document.getElementById('form1').submit();
+}
+
+function errorCallback() {
+ window.location = "resources/popup-no-crash.html";
+}
+
+window.onload = function () {
+ openPicker(document.getElementById('menu'), openCallback, errorCallback);
+}
+</script>
+</body>
+</html>
diff --git a/third_party/WebKit/Source/core/dom/Node.cpp b/third_party/WebKit/Source/core/dom/Node.cpp
index 04356d0..4241f1b 100644
--- a/third_party/WebKit/Source/core/dom/Node.cpp
+++ b/third_party/WebKit/Source/core/dom/Node.cpp
@@ -919,7 +919,11 @@
DocumentLifecycle::DetachScope willDetach(document().lifecycle());
#if ENABLE(ASSERT)
- ASSERT(!detachingNode);
+ // The detaching might trigger destruction of a popup menu window,
+ // with ensuing detachment of its Nodes. In a separate document, so
+ // don't assert for these, but do set detachingNode to the most recent
+ // Node being detached.
+ ASSERT(!detachingNode || detachingNode->document() != document());
detachingNode = this;
#endif
diff --git a/third_party/WebKit/Source/web/PopupMenuImpl.cpp b/third_party/WebKit/Source/web/PopupMenuImpl.cpp
index a0bc7f3..40695b6 100644
--- a/third_party/WebKit/Source/web/PopupMenuImpl.cpp
+++ b/third_party/WebKit/Source/web/PopupMenuImpl.cpp
@@ -234,11 +234,12 @@
void PopupMenuImpl::didClosePopup()
{
- if (m_indexToSetOnClose >= 0)
+ if (m_client && m_indexToSetOnClose >= 0)
m_client->valueChanged(m_indexToSetOnClose);
m_indexToSetOnClose = -1;
m_popup = nullptr;
- m_client->popupDidHide();
+ if (m_client)
+ m_client->popupDidHide();
}
Element& PopupMenuImpl::ownerElement()
