New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 682060 link

Starred by 3 users
Status: Fixed
Owner:
a...@chromium.org
Closed: Jan 2017
Cc:
a...@chromium.org
kkaluri@chromium.org
█ rpop@chromium.org
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocking:
issue 629964



Sign in to add a comment

Closing a dialog from a popup hangs

Project Member Reported by a...@chromium.org, Jan 18 2017 
-----test1.html-----
<script>
function doClick() {
  window.open("test2.html");
}
</script>
<button onclick="doClick()">yo</button>
-----test1.html-----

-----test2.html-----
<script>
window.alert("hi");
</script>
-----test2.html-----

Open test1.html. Click the button. A new tab appears, and the new tab shows a dialog.

Close the new tab without dismissing the dialog first.

test1 hangs.
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 19 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2de41893d8301d98cfc64a3691672d2197e9a4f3

commit 2de41893d8301d98cfc64a3691672d2197e9a4f3
Author: avi <avi@chromium.org>
Date: Thu Jan 19 22:57:09 2017

Make the callback from JavaScript dialogs even when closing a WebContents.

Otherwise, other WebContentses sharing that render process will hang.

The reason the code didn't do this before was because I had been afraid that doing the callback would crash. Therefore, this CL adds a test JavaScriptDialogTest.ClosingPageWithSubframeAlertingDoesntCrash to make sure the crash doesn't happen. (It doesn't; my fears were unfounded.) The other new test, JavaScriptDialogTest.ClosingPageSharingRendererDoesntHang, tests for the bug, and passes with this fix.

BUG= 682060 
TEST=as noted

Review-Url: https://codereview.chromium.org/2641173004
Cr-Commit-Position: refs/heads/master@{#444874}

[modify] https://crrev.com/2de41893d8301d98cfc64a3691672d2197e9a4f3/chrome/browser/ui/javascript_dialogs/javascript_dialog_browsertest.cc
[modify] https://crrev.com/2de41893d8301d98cfc64a3691672d2197e9a4f3/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
[modify] https://crrev.com/2de41893d8301d98cfc64a3691672d2197e9a4f3/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h
Project Member

Comment 2 by bugdroid1@chromium.org, Jan 21 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6879fcfed049c28357eaa85c6d9a0126db54c0d7

commit 6879fcfed049c28357eaa85c6d9a0126db54c0d7
Author: avi <avi@chromium.org>
Date: Sat Jan 21 05:27:53 2017

Clean up unneeded JavaScript dialog parameter.

Removes a parameter unneeded after https://codereview.chromium.org/2641173004.

BUG= 682060 
TEST=none

Review-Url: https://codereview.chromium.org/2639893002
Cr-Commit-Position: refs/heads/master@{#445273}

[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/android_webview/browser/aw_javascript_dialog_manager.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/android_webview/browser/aw_javascript_dialog_manager.h
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/chrome/browser/extensions/content_script_apitest.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/chrome/browser/ui/javascript_dialogs/javascript_dialog_browsertest.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/chrome/browser/ui/javascript_dialogs/javascript_dialog_tab_helper.h
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/components/app_modal/app_modal_dialog.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/components/app_modal/app_modal_dialog.h
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/components/app_modal/javascript_app_modal_dialog.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/components/app_modal/javascript_app_modal_dialog.h
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/components/app_modal/javascript_dialog_manager.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/components/app_modal/javascript_dialog_manager.h
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/content/browser/devtools/protocol/devtools_protocol_browsertest.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/content/browser/web_contents/web_contents_impl_browsertest.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/content/browser/web_contents/web_contents_impl_unittest.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/content/public/browser/javascript_dialog_manager.h
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/content/shell/browser/shell_javascript_dialog_manager.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/content/shell/browser/shell_javascript_dialog_manager.h
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/extensions/browser/guest_view/web_view/javascript_dialog_helper.cc
[modify] https://crrev.com/6879fcfed049c28357eaa85c6d9a0126db54c0d7/extensions/browser/guest_view/web_view/javascript_dialog_helper.h

Comment 3 by a...@chromium.org, Jan 31 2017

Status: Fixed (was: Assigned)

Comment 4 by a...@chromium.org, Feb 3 2017

Cc: kkaluri@chromium.org a...@chromium.org
 Issue 680699  has been merged into this issue.
►

Sign in to add a comment