Force chooser bubbles to close synchronously.

Force chooser bubbles to close synchronously.

This fixes a potential use-after-free because UsbChooserController holds
a raw pointer to the RenderFrameHost and so must be destroyed as part of
RenderFrameHost destruction.

BUG=697486
Review-Url: https://codereview.chromium.org/2724903008
Cr-Commit-Position: refs/heads/master@{#454989}
Committed: https://chromium.googlesource.com/chromium/src/+/692db28a688889a7946b04debfd890673ae2269f

[Reilly Grant (use Gerrit), 2017-03-03 22:59:49]

The CQ bit was checked by reillyg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-03 23:00:06]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Reilly Grant (use Gerrit), 2017-03-03 23:01:37]

reillyg@chromium.org changed reviewers:
+ msw@chromium.org

[Reilly Grant (use Gerrit), 2017-03-03 23:01:38]

Please take a look.

[msw, 2017-03-03 23:06:33]

lgtm

[commit-bot: I haz the power, 2017-03-03 23:40:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-03 23:40:54]

Dry run: This issue passed the CQ dry run.

[Reilly Grant (use Gerrit), 2017-03-04 02:52:18]

Make ChooserBubbleUiView a WidgetObserver.

[Reilly Grant (use Gerrit), 2017-03-04 02:52:52]

Please take another look. To close the widget the caller should be a
WidgetObserver so that it doesn't close the widget after it has already been
destroyed.

[Reilly Grant (use Gerrit), 2017-03-04 02:53:02]

The CQ bit was checked by reillyg@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-04 02:53:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[msw, 2017-03-04 03:22:59]

lgtm with nits

https://codereview.chromium.org/2724903008/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc (right):

https://codereview.chromium.org/2724903008/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc:212: if
(!widget->IsClosed()) {
optional nit: If you know that GetWidget will never return null here, then
consider combining this with the conditional above:
if (bubble_view_ && ! bubble_view_->GetWidget()->IsClosed) {
otherwise, check for non-null here.

https://codereview.chromium.org/2724903008/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.h (right):

https://codereview.chromium.org/2724903008/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.h:17: class
Widget;
nit: not necessary

[commit-bot: I haz the power, 2017-03-04 03:43:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-04 03:43:08]

Dry run: This issue passed the CQ dry run.

[Reilly Grant (use Gerrit), 2017-03-06 19:09:59]

https://codereview.chromium.org/2724903008/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc (right):

https://codereview.chromium.org/2724903008/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.cc:212: if
(!widget->IsClosed()) {
On 2017/03/04 03:22:59, msw wrote:
> optional nit: If you know that GetWidget will never return null here, then
> consider combining this with the conditional above:
> if (bubble_view_ && ! bubble_view_->GetWidget()->IsClosed) {
> otherwise, check for non-null here.

Done.

https://codereview.chromium.org/2724903008/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.h (right):

https://codereview.chromium.org/2724903008/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/website_settings/chooser_bubble_ui_view.h:17: class
Widget;
On 2017/03/04 03:22:59, msw wrote:
> nit: not necessary

Done.

[Reilly Grant (use Gerrit), 2017-03-06 19:10:02]

Address msw@'s nits.

[Reilly Grant (use Gerrit), 2017-03-06 19:10:38]

The CQ bit was checked by reillyg@chromium.org

[Reilly Grant (use Gerrit), 2017-03-06 19:10:38]

The patchset sent to the CQ was uploaded after l-g-t-m from msw@chromium.org
Link to the patchset: https://codereview.chromium.org/2724903008/#ps40001
(title: "Address msw@'s nits.")

[commit-bot: I haz the power, 2017-03-06 19:11:18]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-06 20:23:59]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-06 20:24:00]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Reilly Grant (use Gerrit), 2017-03-06 22:53:56]

The CQ bit was checked by reillyg@chromium.org

[commit-bot: I haz the power, 2017-03-06 22:54:34]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-06 23:00:47]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488840836765870,
"parent_rev": "2fd0d88d674246f33b92619525c101630dd4f77e", "commit_rev":
"692db28a688889a7946b04debfd890673ae2269f"}

[commit-bot: I haz the power, 2017-03-06 23:01:31]

Description was changed from

==========
Force chooser bubbles to close synchronously.

This fixes a potential use-after-free because UsbChooserController holds
a raw pointer to the RenderFrameHost and so must be destroyed as part of
RenderFrameHost destruction.

BUG=697486
==========

to

==========
Force chooser bubbles to close synchronously.

This fixes a potential use-after-free because UsbChooserController holds
a raw pointer to the RenderFrameHost and so must be destroyed as part of
RenderFrameHost destruction.

BUG=697486

Review-Url: https://codereview.chromium.org/2724903008
Cr-Commit-Position: refs/heads/master@{#454989}
Committed:
https://chromium.googlesource.com/chromium/src/+/692db28a688889a7946b04debfd8...
==========

[commit-bot: I haz the power, 2017-03-06 23:01:32]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/692db28a688889a7946b04debfd8...

[Reilly Grant (use Gerrit), 2017-03-07 15:50:01]

A revert of this CL (patchset #3 id:40001) has been created in
https://codereview.chromium.org/2736793003/ by reillyg@chromium.org.

The reason for reverting is: Reverting this patchset because it was
insufficiently tested and introduced a new bug..