Don't crash when two extension uninstall prompts are shown at the same time.

Don't crash when two extension uninstall prompts are shown at the same time.

This adds the same check ExtensionSettingsHandler has in its
ExtensionUninstallAccepted handler to the accepted handler for
chrome.management.uninstall.

BUG=371705
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=271225

[not at google - send to devlin, 2014-05-16 22:11:01]

lgtm

https://codereview.chromium.org/282133008/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/management/management_api.cc (right):

https://codereview.chromium.org/282133008/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/management/management_api.cc:584: // showing. Do
nothing in that case.
Do we have any tests which actually exercise the UI behaviour of the uninstall
dialogue? If so could you update it? If not I expect it will be a PITA so don't
worry.

https://codereview.chromium.org/282133008/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/management/management_api.cc:588: if (!extension)
{
emulate the extension not being there by using kNoExtensionError? (see line 540
for example).

https://codereview.chromium.org/282133008/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/management/management_api.cc:592:
service()->UninstallExtension(extension_id_,
what kind of behaviour do we have in ExtensionService::UninstallExtension if the
extension isn't there? can we make sure it's a CHECK failure rather than some
kind of unexpected failure.

speaking of which, what does the chrome://extensions page do when the extension
gets uninstalled underneath it?

[Marijn Kruisselbrink, 2014-05-16 22:19:12]

https://codereview.chromium.org/282133008/diff/1/chrome/browser/extensions/ap...
File chrome/browser/extensions/api/management/management_api.cc (right):

https://codereview.chromium.org/282133008/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/management/management_api.cc:584: // showing. Do
nothing in that case.
On 2014/05/16 22:11:00, kalman wrote:
> Do we have any tests which actually exercise the UI behaviour of the uninstall
> dialogue? If so could you update it? If not I expect it will be a PITA so
don't
> worry.
I think in tests we bypass the dialog, and behave as if either accept or cancel
was clicked synchronously. So exercising the actual UI in tests is going to be a
PITA.

https://codereview.chromium.org/282133008/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/management/management_api.cc:588: if (!extension)
{
On 2014/05/16 22:11:00, kalman wrote:
> emulate the extension not being there by using kNoExtensionError? (see line
540
> for example).

Done.

https://codereview.chromium.org/282133008/diff/1/chrome/browser/extensions/ap...
chrome/browser/extensions/api/management/management_api.cc:592:
service()->UninstallExtension(extension_id_,
On 2014/05/16 22:11:00, kalman wrote:
> what kind of behaviour do we have in ExtensionService::UninstallExtension if
the
> extension isn't there? can we make sure it's a CHECK failure rather than some
> kind of unexpected failure.
It is a CHECK failure already.

> speaking of which, what does the chrome://extensions page do when the
extension
> gets uninstalled underneath it?
chrome://extensions does exactly what I did here. If you try to uninstall the
same extension twice, the second time clicking accept will just silently do
nothing.

[Marijn Kruisselbrink, 2014-05-16 22:19:15]

The CQ bit was checked by mek@chromium.org

[commit-bot: I haz the power, 2014-05-16 23:34:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/mek@chromium.org/282133008/20001

[commit-bot: I haz the power, 2014-05-17 16:35:06]

Change committed as 271225