move 'web_accessible_resources' parsing out of Extension class.

move 'web_accessible_resources' parsing out of Extension class.

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

[Joe Thomas, 2013-01-08 23:44:11]

Looks like contents of web_intents_parser & web_intents_handler from the CL
https://codereview.chromium.org/11741014/ can be combined to manifest_web_parser
& manifest_web_handler respectively.

@Yoyo: Please let me know whether that makes sense?

[Yoyo Zhou, 2013-01-10 00:37:58]

While I think it would be nice to find some common area that web-accessible
resources belong to, "web" is not a real area. If you find other extension
features related to web security, that'd be closer. But for now let's leave this
as its own separate profile-keyed blob / manifest handler.

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/extension_protocols.cc (right):

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/extensi...
chrome/browser/extensions/extension_protocols.cc:357:
extensions::WebAccessibleResourcesInfo::HasWebAccessibleResources(
nit: this indent is wrong; it should align with the open paren.

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/manifest_web_parser.h (right):

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/extensi...
chrome/browser/extensions/manifest_web_parser.h:15: class ManifestWebParser :
public ProfileKeyedAPI {
This name doesn't make sense. "web" by itself doesn't mean anything. (Web
intents also have nothing to do with web-accessible resources.)
WebAccessibleResourcesParser would make more sense.

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/profile...
File chrome/browser/profiles/profile_dependency_manager.cc (right):

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/profile...
chrome/browser/profiles/profile_dependency_manager.cc:268:
extensions::ProfileKeyedAPIFactory<extensions::ManifestWebParser>::
Rebase on top of head - you'll want to use GetFactoryInstance instead.

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/common/extensio...
File chrome/common/extensions/manifest_web_handler.h (right):

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/common/extensio...
chrome/common/extensions/manifest_web_handler.h:5: #ifndef
CHROME_COMMON_EXTENSIONS_MANIFEST_WEB_HANDLER_H_
The name of this file doesn't match any class name here.

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/common/extensio...
chrome/common/extensions/manifest_web_handler.h:24: static bool
IsResourceWebAccessible(const std::string& relative_path,
I think it's better to reverse the order of the arguments here.

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/renderer/extens...
File chrome/renderer/extensions/resource_request_policy.cc (right):

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/renderer/extens...
chrome/renderer/extensions/resource_request_policy.cc:55: // Disallow loading of
extension resources which are not explicitely listed
nit: explicitely->explicitly

[Joe Thomas, 2013-01-10 20:56:18]

Thanks for the review. Please take a look at the updated patch.

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/extension_protocols.cc (right):

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/extensi...
chrome/browser/extensions/extension_protocols.cc:357:
extensions::WebAccessibleResourcesInfo::HasWebAccessibleResources(
On 2013/01/10 00:37:58, Yoyo Zhou wrote:
> nit: this indent is wrong; it should align with the open paren.

Done

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/extensi...
File chrome/browser/extensions/manifest_web_parser.h (right):

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/extensi...
chrome/browser/extensions/manifest_web_parser.h:15: class ManifestWebParser :
public ProfileKeyedAPI {
On 2013/01/10 00:37:58, Yoyo Zhou wrote:
> This name doesn't make sense. "web" by itself doesn't mean anything. (Web
> intents also have nothing to do with web-accessible resources.)
> WebAccessibleResourcesParser would make more sense.

Done.

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/profile...
File chrome/browser/profiles/profile_dependency_manager.cc (right):

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/browser/profile...
chrome/browser/profiles/profile_dependency_manager.cc:268:
extensions::ProfileKeyedAPIFactory<extensions::ManifestWebParser>::
On 2013/01/10 00:37:58, Yoyo Zhou wrote:
> Rebase on top of head - you'll want to use GetFactoryInstance instead.

Done.

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/common/extensio...
File chrome/common/extensions/manifest_web_handler.h (right):

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/common/extensio...
chrome/common/extensions/manifest_web_handler.h:5: #ifndef
CHROME_COMMON_EXTENSIONS_MANIFEST_WEB_HANDLER_H_
On 2013/01/10 00:37:58, Yoyo Zhou wrote:
> The name of this file doesn't match any class name here.

Renamed to web_accessible_resources_handler.

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/common/extensio...
chrome/common/extensions/manifest_web_handler.h:24: static bool
IsResourceWebAccessible(const std::string& relative_path,
On 2013/01/10 00:37:58, Yoyo Zhou wrote:
> I think it's better to reverse the order of the arguments here.

Done.

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/renderer/extens...
File chrome/renderer/extensions/resource_request_policy.cc (right):

https://chromiumcodereview.appspot.com/11821009/diff/1/chrome/renderer/extens...
chrome/renderer/extensions/resource_request_policy.cc:55: // Disallow loading of
extension resources which are not explicitely listed
On 2013/01/10 00:37:58, Yoyo Zhou wrote:
> nit: explicitely->explicitly

Done.

[Joe Thomas, 2013-01-15 23:34:17]

@Yoyo:

frindly ping. Could you please review the updated patch?

Thanks

[Yoyo Zhou, 2013-01-15 23:55:50]

LGTM

[Joe Thomas, 2013-01-16 00:39:38]

+ben for chrome/renderer/ & gyp changes
+mirandac for chrome/browser/profiles/

Please review.  Thanks!

[Miranda Callahan, 2013-01-16 14:27:31]

On 2013/01/16 00:39:38, Joe Thomas wrote:
> +ben for chrome/renderer/ & gyp changes
> +mirandac for chrome/browser/profiles/
> 
> Please review.  Thanks!

profile_dependency_manager change LGTM.

[Ben Goodger (Google), 2013-01-16 17:59:51]

renderer and gyp lgtm

[commit-bot: I haz the power, 2013-01-16 18:28:50]

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

[commit-bot: I haz the power, 2013-01-16 21:12:56]

Change committed as 177219