Installing extensions (drag/drop, download crx file) will now be...

Installing extensions (drag/drop, download crx file) will now be
handled by the Chrome instance w/o shell execute. This will install
the extension in the proper profile using the running bits.


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

[Paul Godavari, 2009-03-31 01:38:54]

http://codereview.chromium.org/55046/diff/1/6
File chrome/browser/download/download_file.cc (right):

http://codereview.chromium.org/55046/diff/1/6#newcode554
Line 554: DownloadManager* manager) {
You can lookup this parameter. From the else clause you added:
DownloadManagerMap::iterator dmit = managers_.find(download->id());

http://codereview.chromium.org/55046/diff/1/3
File chrome/browser/download/download_manager.cc (right):

http://codereview.chromium.org/55046/diff/1/3#newcode831
Line 831: return;
Don't need the return statement, and then you can remove the braces.

http://codereview.chromium.org/55046/diff/1/4
File chrome/browser/download/download_manager.h (right):

http://codereview.chromium.org/55046/diff/1/4#newcode338
Line 338: // Called when the download is renamed to it's final name.
nit: its

http://codereview.chromium.org/55046/diff/1/4#newcode375
Line 375: // Opens downloaded crx file.
Add a comment indicating what a crx file is (in case the extension changes).

http://codereview.chromium.org/55046/diff/1/4#newcode376
Line 376: void OpenChromeExtension(const FilePath& full_path);
Can this method and "OpenDownloadInShell" be made private now, to avoid
confusion as to which API external callers can use?

[George Djabarov, 2009-03-31 20:01:38]

http://codereview.chromium.org/55046/diff/1/6
File chrome/browser/download/download_file.cc (right):

http://codereview.chromium.org/55046/diff/1/6#newcode554
Line 554: DownloadManager* manager) {
Unfortunately no. When the download is ready for renaming the item is already
gone from managers_. Should I try to do use the function arg in the else clause
instead?

http://codereview.chromium.org/55046/diff/1/3
File chrome/browser/download/download_manager.cc (right):

http://codereview.chromium.org/55046/diff/1/3#newcode831
Line 831: return;
On 2009/03/31 01:38:54, Paul Godavari wrote:
> Don't need the return statement, and then you can remove the braces.

Done.

http://codereview.chromium.org/55046/diff/1/4
File chrome/browser/download/download_manager.h (right):

http://codereview.chromium.org/55046/diff/1/4#newcode338
Line 338: // Called when the download is renamed to it's final name.
On 2009/03/31 01:38:54, Paul Godavari wrote:
> nit: its

Done.

http://codereview.chromium.org/55046/diff/1/4#newcode375
Line 375: // Opens downloaded crx file.
On 2009/03/31 01:38:54, Paul Godavari wrote:
> Add a comment indicating what a crx file is (in case the extension changes).

Done.

http://codereview.chromium.org/55046/diff/1/4#newcode376
Line 376: void OpenChromeExtension(const FilePath& full_path);
On 2009/03/31 01:38:54, Paul Godavari wrote:
> Can this method and "OpenDownloadInShell" be made private now, to avoid
> confusion as to which API external callers can use?

Done.

[Paul Godavari, 2009-04-01 17:18:56]

LGTM, but:

One problem you may run into is if someone wants to download an extension so
that they can unpack it and look at the contents. There'll be no way to do that
with Chrome because it will automatically try to install any file with a crx
extension.