Move the parsing of 'update_url' & 'options_page' URLs out of Extension.

Move the parsing of 'update_url' & 'options_page' URLs out of Extension.

This CL follows https://codereview.chromium.org/11624036/

Patch from Joe Thomas <mhx348@motorola.com>;.

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

[Joe Thomas, 2012-12-30 23:20:32]

Please review.

[Yoyo Zhou, 2013-01-03 23:16:00]

https://codereview.chromium.org/11726002/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/extension_process_manager.cc (right):

https://codereview.chromium.org/11726002/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extension_process_manager.cc:312: void
ExtensionProcessManager::OpenOptionsPage(const Extension* extension,
While you're here, can you add a TODO in the .h file:
TODO(yoz): Move this function to a more appropriate location. crbug.com/157279

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/mani...
File
chrome/common/extensions/manifest_tests/extension_manifests_initvalue_unittest.cc
(right):

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/mani...
chrome/common/extensions/manifest_tests/extension_manifests_initvalue_unittest.cc:21:
namespace keys = extension_manifest_keys;
no need for this if it's only used once.

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/mani...
chrome/common/extensions/manifest_tests/extension_manifests_initvalue_unittest.cc:24:
namespace {
I don't think the prevailing style is to put these in an anonymous namespace.

(Same comments in the other test files.)

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/mani...
File chrome/common/extensions/manifest_url_info.cc (right):

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/mani...
chrome/common/extensions/manifest_url_info.cc:33: const GURL&
ManifestURLInfo::GetUpdateURL(const Extension* extension) {
Since these 2 are very similar to GetDevToolsPage, you should be able to define
a common helper function.

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/opti...
File chrome/common/extensions/options_page_handler.h (right):

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/opti...
chrome/common/extensions/options_page_handler.h:5: #ifndef
CHROME_COMMON_EXTENSIONS_OPTIONS_PAGE_HANDLER_H_
You can rebase this onto your other change that now has a single file for
manifest URL handling.

[Joe Thomas, 2013-01-04 01:36:27]

https://codereview.chromium.org/11726002/diff/1/chrome/browser/extensions/ext...
File chrome/browser/extensions/extension_process_manager.cc (right):

https://codereview.chromium.org/11726002/diff/1/chrome/browser/extensions/ext...
chrome/browser/extensions/extension_process_manager.cc:312: void
ExtensionProcessManager::OpenOptionsPage(const Extension* extension,
On 2013/01/03 23:16:00, Yoyo Zhou wrote:
> While you're here, can you add a TODO in the .h file:
> TODO(yoz): Move this function to a more appropriate location. crbug.com/157279

Done.

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/mani...
File
chrome/common/extensions/manifest_tests/extension_manifests_initvalue_unittest.cc
(right):

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/mani...
chrome/common/extensions/manifest_tests/extension_manifests_initvalue_unittest.cc:21:
namespace keys = extension_manifest_keys;
On 2013/01/03 23:16:00, Yoyo Zhou wrote:
> no need for this if it's only used once.

Done.

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/mani...
chrome/common/extensions/manifest_tests/extension_manifests_initvalue_unittest.cc:24:
namespace {
On 2013/01/03 23:16:00, Yoyo Zhou wrote:
> I don't think the prevailing style is to put these in an anonymous namespace.
> 
> (Same comments in the other test files.)

Done.

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/mani...
File chrome/common/extensions/manifest_url_info.cc (right):

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/mani...
chrome/common/extensions/manifest_url_info.cc:33: const GURL&
ManifestURLInfo::GetUpdateURL(const Extension* extension) {
On 2013/01/03 23:16:00, Yoyo Zhou wrote:
> Since these 2 are very similar to GetDevToolsPage, you should be able to
define
> a common helper function.

Done.

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/opti...
File chrome/common/extensions/options_page_handler.h (right):

https://codereview.chromium.org/11726002/diff/1/chrome/common/extensions/opti...
chrome/common/extensions/options_page_handler.h:5: #ifndef
CHROME_COMMON_EXTENSIONS_OPTIONS_PAGE_HANDLER_H_
On 2013/01/03 23:16:00, Yoyo Zhou wrote:
> You can rebase this onto your other change that now has a single file for
> manifest URL handling.

Done.

[Yoyo Zhou, 2013-01-05 01:11:58]

LGTM

[Joe Thomas, 2013-01-05 01:22:24]

+ben for chrome/browser/automation/
+pkotwicz for chrome/browser/themes/

[Joe Thomas, 2013-01-05 01:27:58]

+ben for chrome/browser/automation/
+pkotwicz for chrome/browser/themes/
+xiyuan for chrome/browser/ui/app_list/
+sail for chrome/browser/ui/cocoa/
+benwells for chrome/browser/ui/extensions/
+arv for chrome/browser/ui/webui/

Please review.  Thanks.

[sail, 2013-01-05 05:23:52]

cocoa/* LGTM

[xiyuan, 2013-01-07 17:11:21]

ui/app_list LGTM

[Ben Goodger (Google), 2013-01-07 18:06:27]

automation lgtm

[benwells, 2013-01-08 02:09:51]

c/b/ui/extensions lgtm

[pkotwicz, 2013-01-08 15:55:34]

chrome/browser/themes lgtm

[Joe Thomas, 2013-01-10 15:00:22]

@arv, estade,

Looking for OWNERS approval in chrome/browser/ui/webui/. Thanks!

[Joe Thomas, 2013-01-14 21:03:06]

On 2013/01/10 15:00:22, Joe Thomas wrote:
> @arv, estade,
> 
> Looking for OWNERS approval in chrome/browser/ui/webui/. Thanks!

@estade, arv,

Friendly Ping.

[Evan Stade, 2013-01-14 21:04:24]

lgtm

[commit-bot: I haz the power, 2013-01-14 23:41:42]

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

[Joe Thomas, 2013-01-15 00:50:01]

@Yoyo,

Could you please review
chrome/browser/extensions/updater/extension_updater_unittest.cc?  Fixed 2
failing unittests.

Thanks

[Yoyo Zhou, 2013-01-15 00:51:40]

LGTM. You don't need another LGTM for something trivial like this, just an FYI
message on this review thread is probably enough.

[commit-bot: I haz the power, 2013-01-15 01:05:38]

Failed to trigger a try job on mac_rel
HTTP Error 400: Bad Request

[commit-bot: I haz the power, 2013-01-15 01:06:00]

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

[Joe Thomas, 2013-01-15 03:51:17]

@Yoyo:

FYI -  Fixed HomepageURLManifestTest.GetHomepageURL unittest

[commit-bot: I haz the power, 2013-01-15 03:55:56]

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

[commit-bot: I haz the power, 2013-01-15 07:27:24]

Change committed as 176824