Update page action manifest parsing to match the new format.

Update page action manifest parsing to match the new format.

Preserves backwards compatability with the old manifest format.

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

[Aaron Boodman, 2009-10-13 21:17:07]

Yeah, it's messy. Thanks for putting up with it.

http://codereview.chromium.org/275007/diff/1/10
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/275007/diff/1/10#newcode304
Line 304: // TODO(EXTENSIONS_DEPRECATED): icons list is obsolete.
No need to change this, but just want to verify my understanding of the code...
Technically this is not obsolete yet, right? We haven't implement setIcon(string
path) yet.

http://codereview.chromium.org/275007/diff/1/10#newcode335
Line 335: if (page_action->GetString(keys::kPageActionIcons, &default_icon) ||
I think you mean keys::kPageActionDefaultIcon here.

http://codereview.chromium.org/275007/diff/1/10#newcode340
Line 340: // TODO(EXTENSIONS_DEPRECATED): one icon.
I don't get why this is marked EXTENSIONS_DEPRECATED.

http://codereview.chromium.org/275007/diff/1/10#newcode344
Line 344: // Read the page action |default_text|.
default_title

[Matt Perry, 2009-10-13 21:23:16]

http://codereview.chromium.org/275007/diff/1/10
File chrome/common/extensions/extension.cc (right):

http://codereview.chromium.org/275007/diff/1/10#newcode304
Line 304: // TODO(EXTENSIONS_DEPRECATED): icons list is obsolete.
On 2009/10/13 21:17:07, Aaron Boodman wrote:
> No need to change this, but just want to verify my understanding of the
code...
> Technically this is not obsolete yet, right? We haven't implement
setIcon(string
> path) yet.

I meant that >1 icon is obsolete as far as the new API spec is concerned. But
yeah, we'll probably need to implement setIcon(path) before it becomes
unnecessary.

http://codereview.chromium.org/275007/diff/1/10#newcode335
Line 335: if (page_action->GetString(keys::kPageActionIcons, &default_icon) ||
On 2009/10/13 21:17:07, Aaron Boodman wrote:
> I think you mean keys::kPageActionDefaultIcon here.

Done.

http://codereview.chromium.org/275007/diff/1/10#newcode340
Line 340: // TODO(EXTENSIONS_DEPRECATED): one icon.
On 2009/10/13 21:17:07, Aaron Boodman wrote:
> I don't get why this is marked EXTENSIONS_DEPRECATED.

Extension actions should only have 1 icon, not a list.

http://codereview.chromium.org/275007/diff/1/10#newcode344
Line 344: // Read the page action |default_text|.
On 2009/10/13 21:17:07, Aaron Boodman wrote:
> default_title

Done.

[Aaron Boodman, 2009-10-13 21:48:23]

lgtm