Dismiss DevicePermissionsPrompt when the widgets are destroyed.

Dismiss DevicePermissionsPrompt when the widgets are destroyed.

Destruction of the widget hierarchy (which happens if the application
window is closed) does not result in a call to either Accept or Cancel.
To catch this DevicePermissionsDialogView should be ready to dismiss
the prompt if the delegate is destroyed. Otherwise the extension
function call is never completed and the prompt object is leaked.

Committed: https://crrev.com/571132c474ef060b1f7db932eea3f552f6d8fcca
Cr-Commit-Position: refs/heads/master@{#325953}

[Reilly Grant (use Gerrit), 2015-04-20 19:32:05]

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

[Reilly Grant (use Gerrit), 2015-04-20 19:32:40]

Peter, please take a look. Let me know if there's a better way to fix this.

[Peter Kasting, 2015-04-20 21:33:33]

LGTM

https://codereview.chromium.org/1070273003/diff/1/chrome/browser/ui/views/ext...
File chrome/browser/ui/views/extensions/device_permissions_dialog_view.cc
(right):

https://codereview.chromium.org/1070273003/diff/1/chrome/browser/ui/views/ext...
chrome/browser/ui/views/extensions/device_permissions_dialog_view.cc:125: // be
closed including the unexpected closure of its parent.
Nit: How about:

Calling prompt_->Dismissed() here ensures it will be called regardless of how
the view is closed, including shutdown of the entire view hierarchy.

https://codereview.chromium.org/1070273003/diff/1/chrome/browser/ui/views/ext...
File chrome/browser/ui/views/extensions/device_permissions_dialog_view.h
(right):

https://codereview.chromium.org/1070273003/diff/1/chrome/browser/ui/views/ext...
chrome/browser/ui/views/extensions/device_permissions_dialog_view.h:25: //
Overriding views::DialogDelegateView.
Nit: While here:

Since this class only subclasses views::DialogDelegateView, there should be just
one override section, headed "// views::DialogDelegateView:", and the methods in
it should be listed in the order in which they were declared within their base
classes, in the order in which the base classes themselves were declared.  In
this case, that would be this order:

GetDialogButtonLabel
GetModalType
GetWindowTitle
DeleteDelegate
Accept
GetPreferredSize

[Reilly Grant (use Gerrit), 2015-04-20 23:14:49]

https://codereview.chromium.org/1070273003/diff/1/chrome/browser/ui/views/ext...
File chrome/browser/ui/views/extensions/device_permissions_dialog_view.cc
(right):

https://codereview.chromium.org/1070273003/diff/1/chrome/browser/ui/views/ext...
chrome/browser/ui/views/extensions/device_permissions_dialog_view.cc:125: // be
closed including the unexpected closure of its parent.
On 2015/04/20 21:33:33, Peter Kasting wrote:
> Nit: How about:
> 
> Calling prompt_->Dismissed() here ensures it will be called regardless of how
> the view is closed, including shutdown of the entire view hierarchy.

Done.

https://codereview.chromium.org/1070273003/diff/1/chrome/browser/ui/views/ext...
File chrome/browser/ui/views/extensions/device_permissions_dialog_view.h
(right):

https://codereview.chromium.org/1070273003/diff/1/chrome/browser/ui/views/ext...
chrome/browser/ui/views/extensions/device_permissions_dialog_view.h:25: //
Overriding views::DialogDelegateView.
On 2015/04/20 21:33:33, Peter Kasting wrote:
> Nit: While here:
> 
> Since this class only subclasses views::DialogDelegateView, there should be
just
> one override section, headed "// views::DialogDelegateView:", and the methods
in
> it should be listed in the order in which they were declared within their base
> classes, in the order in which the base classes themselves were declared.  In
> this case, that would be this order:
> 
> GetDialogButtonLabel
> GetModalType
> GetWindowTitle
> DeleteDelegate
> Accept
> GetPreferredSize

Done.

[Reilly Grant (use Gerrit), 2015-04-20 23:14:54]

The CQ bit was checked by reillyg@chromium.org

[Reilly Grant (use Gerrit), 2015-04-20 23:14:54]

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

[commit-bot: I haz the power, 2015-04-20 23:16:51]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1070273003/20001

[commit-bot: I haz the power, 2015-04-21 00:32:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-04-21 00:32:55]

Patchset 2 (id:??) landed as
https://crrev.com/571132c474ef060b1f7db932eea3f552f6d8fcca
Cr-Commit-Position: refs/heads/master@{#325953}