Fix a bug where extensions created with locale info from the command line is broken.

Fix a bug where extensions created with locale info from the command line is broken.

Register the ManifestHandler for default_locale

BUG=176187
TEST=1. Unzip the test.zip attachment from issue 176187.
2. Use chrome to pack it as an extension (chrome --pack-extension=test).
test.crx should be created successfully.

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

[Joe Thomas, 2013-02-14 18:12:27]

Please take a look. Thanks

[Joe Thomas, 2013-02-14 18:14:41]

https://codereview.chromium.org/12256041/diff/1/chrome/browser/extensions/pac...
File chrome/browser/extensions/pack_extension_job.cc (right):

https://codereview.chromium.org/12256041/diff/1/chrome/browser/extensions/pac...
chrome/browser/extensions/pack_extension_job.cc:32: make_linked_ptr(new
DefaultLocaleHandler));
We can also move this registration to StartupHelper::PackExtension to make it
specific to the failing test case. But for me having it here looks better.

[Yoyo Zhou, 2013-02-14 18:41:02]

https://codereview.chromium.org/12256041/diff/1/chrome/browser/extensions/pac...
File chrome/browser/extensions/pack_extension_job.cc (right):

https://codereview.chromium.org/12256041/diff/1/chrome/browser/extensions/pac...
chrome/browser/extensions/pack_extension_job.cc:32: make_linked_ptr(new
DefaultLocaleHandler));
On 2013/02/14 18:14:42, Joe Thomas wrote:
> We can also move this registration to StartupHelper::PackExtension to make it
> specific to the failing test case. But for me having it here looks better.

I think it belongs in StartupHelper, probably the constructor.

PackExtensionJob can also be invoked from the chrome UI, in which case the
handler should already be registered.

[Joe Thomas, 2013-02-14 18:48:47]

https://codereview.chromium.org/12256041/diff/1/chrome/browser/extensions/pac...
File chrome/browser/extensions/pack_extension_job.cc (right):

https://codereview.chromium.org/12256041/diff/1/chrome/browser/extensions/pac...
chrome/browser/extensions/pack_extension_job.cc:32: make_linked_ptr(new
DefaultLocaleHandler));
On 2013/02/14 18:41:02, Yoyo Zhou wrote:
> On 2013/02/14 18:14:42, Joe Thomas wrote:
> > We can also move this registration to StartupHelper::PackExtension to make
it
> > specific to the failing test case. But for me having it here looks better.
> 
> I think it belongs in StartupHelper, probably the constructor.
> 
> PackExtensionJob can also be invoked from the chrome UI, in which case the
> handler should already be registered.


I thought about that. Then I found that we just destroy the old ManifestHandler
and replace it with new one. So I add the code in PackExtensionJob constructor.

Anyways I will move it to  StartupHelper c'tor.

[Joe Thomas, 2013-02-14 19:16:28]

https://codereview.chromium.org/12256041/diff/1/chrome/browser/extensions/pac...
File chrome/browser/extensions/pack_extension_job.cc (right):

https://codereview.chromium.org/12256041/diff/1/chrome/browser/extensions/pac...
chrome/browser/extensions/pack_extension_job.cc:32: make_linked_ptr(new
DefaultLocaleHandler));
On 2013/02/14 18:48:47, Joe Thomas wrote:
> On 2013/02/14 18:41:02, Yoyo Zhou wrote:
> > On 2013/02/14 18:14:42, Joe Thomas wrote:
> > > We can also move this registration to StartupHelper::PackExtension to make
> it
> > > specific to the failing test case. But for me having it here looks better.
> > 
> > I think it belongs in StartupHelper, probably the constructor.
> > 
> > PackExtensionJob can also be invoked from the chrome UI, in which case the
> > handler should already be registered.
> 
> 
> I thought about that. Then I found that we just destroy the old
ManifestHandler
> and replace it with new one. So I add the code in PackExtensionJob
constructor.
> 
> Anyways I will move it to  StartupHelper c'tor.

Done.

[Yoyo Zhou, 2013-02-14 19:55:25]

Please add a TEST= line to the CL description to give instructions that QA can
use to manually test this.

[Joe Thomas, 2013-02-14 20:09:02]

On 2013/02/14 19:55:25, Yoyo Zhou wrote:
> Please add a TEST= line to the CL description to give instructions that QA can
> use to manually test this.

Done.

[Yoyo Zhou, 2013-02-14 20:10:12]

On 2013/02/14 20:09:02, Joe Thomas wrote:
> On 2013/02/14 19:55:25, Yoyo Zhou wrote:
> > Please add a TEST= line to the CL description to give instructions that QA
can
> > use to manually test this.
> 
> Done.

LGTM

[commit-bot: I haz the power, 2013-02-14 20:12:27]

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

[commit-bot: I haz the power, 2013-02-14 22:28:07]

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

[commit-bot: I haz the power, 2013-02-15 04:33:37]

Change committed as 182615