HTML Pack Extension Dialog / Linux & Mac Packaging Support

HTML Pack Extension Dialog. This removes the views implementation of the ExtensionPackDialog, and implements the dialog in html in the extensions_ui DOMUI.

Additionally, support is added for packaging extensions via ---pack-extension on linux and mac

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

[rafaelw, 2009-09-24 01:08:27]

Note: I plan to land the rsa_privatekey_mac and signature_creator_mac before
this patch.

When that happens, I'll modify this patch to include mac support.

[rafaelw, 2009-10-03 01:20:09]

Here's the packing CL. It's blocked behind:
http://codereview.chromium.org/259026, so no particular rush.

[Aaron Boodman, 2009-10-06 17:31:19]

http://codereview.chromium.org/207062/diff/4001/5003
File chrome/browser/extensions/extensions_ui.cc (right):

http://codereview.chromium.org/207062/diff/4001/5003#newcode211
Line 211: ListValue results;
"results" is an odd name for this variable. "arguments" maybe?

http://codereview.chromium.org/207062/diff/4001/5004
File chrome/browser/extensions/extensions_ui.h (right):

http://codereview.chromium.org/207062/diff/4001/5004#newcode108
Line 108: void WindowAlert(const std::string& message);
Nit: methods should be verbs. ShowAlert()?

http://codereview.chromium.org/207062/diff/4001/5004#newcode138
Line 138: scoped_refptr<PackExtensionJob> pack_job_;
You need to call ClearClient() on this guy when the tab is closed to prevent it
from calling you back if the tab closes before the pack completes.

http://codereview.chromium.org/207062/diff/4001/5005
File chrome/browser/extensions/pack_extension_job.h (right):

http://codereview.chromium.org/207062/diff/4001/5005#newcode5
Line 5: #ifndef CHROME_BROWSER_EXTENSIONS_PACK_EXTENSION_JOB_UI_H_
Whoops. Thanks.

http://codereview.chromium.org/207062/diff/4001/5007
File chrome/browser/resources/extensions_ui.html (right):

http://codereview.chromium.org/207062/diff/4001/5007#newcode351
Line 351: window.handleFilePathSelected = function(filePath) {
It's a little odd to see 'window.' here. While technically correct, it seems to
indicate this value is a property of window, but it's really a global variable.

I think it would be more clear without the 'window.'.

http://codereview.chromium.org/207062/diff/4001/5007#newcode356
Line 356: chrome.send('selectFilePath', ['folder', 'load']);
The pattern of setting the handleFilePathSelected handler, sending
'selectFilePath' message, then clearing the handler when the call responds
happens several times. Consider abstracting this into a showFileDialog()
function that takes a callback.

[rafaelw, 2009-10-07 00:10:20]

http://codereview.chromium.org/207062/diff/4001/5003
File chrome/browser/extensions/extensions_ui.cc (right):

http://codereview.chromium.org/207062/diff/4001/5003#newcode211
Line 211: ListValue results;
On 2009/10/06 17:31:19, Aaron Boodman wrote:
> "results" is an odd name for this variable. "arguments" maybe?

Done.

http://codereview.chromium.org/207062/diff/4001/5004
File chrome/browser/extensions/extensions_ui.h (right):

http://codereview.chromium.org/207062/diff/4001/5004#newcode108
Line 108: void WindowAlert(const std::string& message);
On 2009/10/06 17:31:19, Aaron Boodman wrote:
> Nit: methods should be verbs. ShowAlert()?

Done.

http://codereview.chromium.org/207062/diff/4001/5004#newcode138
Line 138: scoped_refptr<PackExtensionJob> pack_job_;
On 2009/10/06 17:31:19, Aaron Boodman wrote:
> You need to call ClearClient() on this guy when the tab is closed to prevent
it
> from calling you back if the tab closes before the pack completes.

Done.

http://codereview.chromium.org/207062/diff/4001/5005
File chrome/browser/extensions/pack_extension_job.h (right):

http://codereview.chromium.org/207062/diff/4001/5005#newcode5
Line 5: #ifndef CHROME_BROWSER_EXTENSIONS_PACK_EXTENSION_JOB_UI_H_
On 2009/10/06 17:31:19, Aaron Boodman wrote:
> Whoops. Thanks.

np

http://codereview.chromium.org/207062/diff/4001/5007
File chrome/browser/resources/extensions_ui.html (right):

http://codereview.chromium.org/207062/diff/4001/5007#newcode351
Line 351: window.handleFilePathSelected = function(filePath) {
On 2009/10/06 17:31:19, Aaron Boodman wrote:
> It's a little odd to see 'window.' here. While technically correct, it seems
to
> indicate this value is a property of window, but it's really a global
variable.
> 
> I think it would be more clear without the 'window.'.

Done.

http://codereview.chromium.org/207062/diff/4001/5007#newcode356
Line 356: chrome.send('selectFilePath', ['folder', 'load']);
On 2009/10/06 17:31:19, Aaron Boodman wrote:
> The pattern of setting the handleFilePathSelected handler, sending
> 'selectFilePath' message, then clearing the handler when the call responds
> happens several times. Consider abstracting this into a showFileDialog()
> function that takes a callback.

Done.

[Aaron Boodman, 2009-10-07 00:12:27]

lgtm