Update WebstoreInstaller to be ref counted.

Update WebstoreInstaller to be ref counted.

This simplifies the WebstoreInstaller by making it live until the installation has succeeded or failed. This also fixes a problem with installing extensions from incognito windows by having the WebstoreInstaller explicitly pass in the NavigationController used to download the extension.

BUG=100788, 98687
TEST=*WebstorePrivate*


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=107104

[jstritar, 2011-10-24 21:57:25]

This also includes the change to WebstoreInstallHelper to pass the extension id
to the delegate... they were easy to pull off the bundle CL together.

[Mihai Parparita -not on Chrome, 2011-10-24 22:36:18]

LGTM, just some nits.

http://codereview.chromium.org/8375034/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_webstore_private_api.cc (right):

http://codereview.chromium.org/8375034/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_webstore_private_api.cc:348: 
Extra newline.

http://codereview.chromium.org/8375034/diff/1/chrome/browser/extensions/webst...
File chrome/browser/extensions/webstore_installer.cc (right):

http://codereview.chromium.org/8375034/diff/1/chrome/browser/extensions/webst...
chrome/browser/extensions/webstore_installer.cc:82: // |download_url|. We will
navigate the current tab to this url which will
This should be |download_url_|

http://codereview.chromium.org/8375034/diff/1/chrome/browser/extensions/webst...
chrome/browser/extensions/webstore_installer.cc:137: delegate_ = NULL;
Out of curiosity, why do you need to have delegate_ get nulled out? Is it to
prevent multiple success/failure calls to it? It seems like that shouldn't
happen, so having CHECKs for that is more appropriate (especially since the
installer will delete itself with the Release() calls).

[jstritar, 2011-10-25 02:45:04]

http://codereview.chromium.org/8375034/diff/1/chrome/browser/extensions/exten...
File chrome/browser/extensions/extension_webstore_private_api.cc (right):

http://codereview.chromium.org/8375034/diff/1/chrome/browser/extensions/exten...
chrome/browser/extensions/extension_webstore_private_api.cc:348: 
On 2011/10/24 22:36:18, Mihai Parparita wrote:
> Extra newline.

Done.

http://codereview.chromium.org/8375034/diff/1/chrome/browser/extensions/webst...
File chrome/browser/extensions/webstore_installer.cc (right):

http://codereview.chromium.org/8375034/diff/1/chrome/browser/extensions/webst...
chrome/browser/extensions/webstore_installer.cc:82: // |download_url|. We will
navigate the current tab to this url which will
On 2011/10/24 22:36:18, Mihai Parparita wrote:
> This should be |download_url_|

Done, and updated the comment to make a bit more sense.

http://codereview.chromium.org/8375034/diff/1/chrome/browser/extensions/webst...
chrome/browser/extensions/webstore_installer.cc:137: delegate_ = NULL;
On 2011/10/24 22:36:18, Mihai Parparita wrote:
> Out of curiosity, why do you need to have delegate_ get nulled out? Is it to
> prevent multiple success/failure calls to it? It seems like that shouldn't
> happen, so having CHECKs for that is more appropriate (especially since the
> installer will delete itself with the Release() calls).

This was to make sure it doesn't get called twice. We shouldn't get multiple
notifications for the same extension install, and the installer should delete
itself... so I'll just get rid of it.

[commit-bot: I haz the power, 2011-10-25 02:45:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jstritar@chromium.org/8375034/3001

[commit-bot: I haz the power, 2011-10-25 04:36:15]

Try job failure for 8375034-3001 (retry) on mac_rel for step "base_unittests".
It's a second try, previously, step "base_unittests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2011-10-25 11:46:35]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jstritar@chromium.org/8375034/3001

[commit-bot: I haz the power, 2011-10-25 13:00:18]

Change committed as 107104