Update extensions with Drag-and-Drop in extension settings page.

Update extensions with Drag-and-Drop in extension settings page.

While dropping a CRX on the extension page, if there is a previous version installed, then do the following.

- If the extension requires no new permission, then update silently.
- If the extension requires additional permission, install the update silently which causes the extension to get disabled. And show the re-enable prompt.
      * if user cancels the prompt, extension will continue to be disabled.
      * if user confirms re-enable, extension will be enabled back.

Screenshot of the re-enable prompt is uploaded at http://crbug.com/169521#c11

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

[Joe Thomas, 2013-02-27 01:13:12]

Please take a look. Thanks!

[Matt Perry, 2013-02-27 01:21:46]

https://codereview.chromium.org/12319131/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/extension_install_prompt.cc (right):

https://codereview.chromium.org/12319131/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extension_install_prompt.cc:61:
IDS_EXTENSION_UPDATE_PROMPT_TITLE,
Let's not add a new prompt type. We should do whatever the current
re-enable-after-update prompt shows.

[Joe Thomas, 2013-02-27 20:55:38]

Please take another look. I updated the CL description with the current
behavior. Please let me know if that is fine.

https://codereview.chromium.org/12319131/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/extension_install_prompt.cc (right):

https://codereview.chromium.org/12319131/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extension_install_prompt.cc:61:
IDS_EXTENSION_UPDATE_PROMPT_TITLE,
On 2013/02/27 01:21:46, Matt Perry wrote:
> Let's not add a new prompt type. We should do whatever the current
> re-enable-after-update prompt shows.

Done.

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
File chrome/browser/extensions/crx_installer.cc (right):

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/crx_installer.cc:44: #include
"chrome/common/extensions/permissions/permission_set.h"
This is not required.

[Matt Perry, 2013-02-28 01:13:27]

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
File chrome/browser/extensions/crx_installer.cc (left):

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/crx_installer.cc:679: extension_ = NULL;
Don't delete this code. You should still NULL-out extension_ if we don't end up
calling client_->ConfirmReEnable

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
File chrome/browser/extensions/crx_installer.cc (right):

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/crx_installer.cc:481: if (update_from_settings_page_)
{
could you add a comment here that explains we're getting this in response to
ConfirmReEnable? I was confused at first.

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/crx_installer.cc:640: record_oauth2_grant_ =
client_->record_oauth2_grant();
indent

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/crx_installer.cc:759: AddRef();  // Balanced in
Proceed() and Abort().
nit: balanced in InstallUIProceed* ...

[Joe Thomas, 2013-02-28 02:01:16]

Thanks for the review. Update the patch. Please take a look.

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
File chrome/browser/extensions/crx_installer.cc (left):

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/crx_installer.cc:679: extension_ = NULL;
On 2013/02/28 01:13:27, Matt Perry wrote:
> Don't delete this code. You should still NULL-out extension_ if we don't end
up
> calling client_->ConfirmReEnable

This code is not required. If we dont call client_->ConfirmReEnable,
CrxInstaller object's lifetime ends and it gets destroyed. Here extension_ is
nothing but scoped_refptr<Extension>. And scoped_refptr's destructor does the
same as assigning the variable to NULL (it calls Release() on the object it
hold).

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
File chrome/browser/extensions/crx_installer.cc (right):

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/crx_installer.cc:481: if (update_from_settings_page_)
{
On 2013/02/28 01:13:27, Matt Perry wrote:
> could you add a comment here that explains we're getting this in response to
> ConfirmReEnable? I was confused at first.

Done.

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/crx_installer.cc:640: record_oauth2_grant_ =
client_->record_oauth2_grant();
On 2013/02/28 01:13:27, Matt Perry wrote:
> indent

Done.

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/crx_installer.cc:759: AddRef();  // Balanced in
Proceed() and Abort().
On 2013/02/28 01:13:27, Matt Perry wrote:
> nit: balanced in InstallUIProceed* ...

Done.

[Matt Perry, 2013-02-28 20:02:07]

lgtm

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
File chrome/browser/extensions/crx_installer.cc (left):

https://codereview.chromium.org/12319131/diff/4001/chrome/browser/extensions/...
chrome/browser/extensions/crx_installer.cc:679: extension_ = NULL;
On 2013/02/28 02:01:16, Joe Thomas wrote:
> On 2013/02/28 01:13:27, Matt Perry wrote:
> > Don't delete this code. You should still NULL-out extension_ if we don't end
> up
> > calling client_->ConfirmReEnable
> 
> This code is not required. If we dont call client_->ConfirmReEnable,
> CrxInstaller object's lifetime ends and it gets destroyed. Here extension_ is
> nothing but scoped_refptr<Extension>. And scoped_refptr's destructor does the
> same as assigning the variable to NULL (it calls Release() on the object it
> hold).

OK

[Joe Thomas, 2013-02-28 20:51:54]

On 2013/02/28 20:02:07, Matt Perry wrote:
> lgtm
> 

Thanks !

[commit-bot: I haz the power, 2013-02-28 20:52:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/MHX348@motorola.com/12319131/7001

[commit-bot: I haz the power, 2013-02-28 22:23:55]

Retried try job too often on win_rel for step(s) browser_tests,
content_browsertests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-02-28 23:22:45]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/MHX348@motorola.com/12319131/7001

[commit-bot: I haz the power, 2013-03-01 01:58:30]

Change committed as 185401