Add wallpaper experimental api framework and add a simple category list.

Add wallpaper experimental api framework and add a simple category list.


BUG=118684
TEST=

[bshe, 2012-05-25 18:27:10]

Hi Rob.

Could you please take a look at this CL? I added an experimental wallpaper api
framework. And to verify the framework works, I just add a simple getString api.
The simple category list is by no mean complete. It is just there to verify the
getString api works. I may try to follow the uber frame example (navigation bar
in settings page) for the category list. It will be in a separate CL.

Thanks!

[flackr, 2012-05-26 14:16:45]

I assume not being for final review I shouldn't thoroughly go over things. The
general idea seems good, I have a few questions about some parts.

http://codereview.chromium.org/10453032/diff/1/chrome/app/generated_resources...
File chrome/app/generated_resources.grd (right):

http://codereview.chromium.org/10453032/diff/1/chrome/app/generated_resources...
chrome/app/generated_resources.grd:10489: +        Custom...
Other than this, I think wallpaper categories should be loaded from the manifest
file so that these can be managed in one location.

http://codereview.chromium.org/10453032/diff/1/chrome/browser/chromeos/extens...
File chrome/browser/chromeos/extensions/wallpaper_manager_api.h (right):

http://codereview.chromium.org/10453032/diff/1/chrome/browser/chromeos/extens...
chrome/browser/chromeos/extensions/wallpaper_manager_api.h:24: };
DISALLOW_COPY_AND..

http://codereview.chromium.org/10453032/diff/1/chrome/browser/resources/chrom...
File chrome/browser/resources/chromeos/wallpaper_manager/js/main.js (right):

http://codereview.chromium.org/10453032/diff/1/chrome/browser/resources/chrom...
chrome/browser/resources/chromeos/wallpaper_manager/js/main.js:13: * Called by
main.html after the dom has been parsed.
This doesn't seem to be true (anymore?). The addEventListener below is self
explanatory so you don't have to document this.

http://codereview.chromium.org/10453032/diff/1/chrome/browser/resources/chrom...
File chrome/browser/resources/chromeos/wallpaper_manager/js/main_scripts.js
(right):

http://codereview.chromium.org/10453032/diff/1/chrome/browser/resources/chrom...
chrome/browser/resources/chromeos/wallpaper_manager/js/main_scripts.js:7:
//<include src="main.js"/>
What are you doing here?

http://codereview.chromium.org/10453032/diff/1/chrome/browser/resources/chrom...
File chrome/browser/resources/chromeos/wallpaper_manager/main.html (right):

http://codereview.chromium.org/10453032/diff/1/chrome/browser/resources/chrom...
chrome/browser/resources/chromeos/wallpaper_manager/main.html:15: <!-- This file
has not been flattened, load individual scripts.
What do you mean? Are you using the <if> expression to prevent flattening?

http://codereview.chromium.org/10453032/diff/1/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/10453032/diff/1/chrome/chrome_browser.gypi#new...
chrome/chrome_browser.gypi:4877: ['exclude',
'^browser/chromeos/extensions/wallpaper_manager_api'],
This is probably unnecessary given that it's in browser/chromeos, I imagine this
entire directory is already excluded.

[bshe, 2012-05-29 16:07:11]

Done and inlined. Thanks!

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/app/generated_r...
File chrome/app/generated_resources.grd (right):

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/app/generated_r...
chrome/app/generated_resources.grd:10489: +        Custom...
Agree. I will add them in manifest once I get to it.

On 2012/05/26 14:16:45, flackr wrote:
> Other than this, I think wallpaper categories should be loaded from the
manifest
> file so that these can be managed in one location.

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/chromeo...
File chrome/browser/chromeos/extensions/wallpaper_manager_api.h (right):

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/chromeo...
chrome/browser/chromeos/extensions/wallpaper_manager_api.h:24: };
On 2012/05/26 14:16:45, flackr wrote:
> DISALLOW_COPY_AND..

It seems some of the extension function doesn't have "DISALLOW_COPY.. " macro. I
am not sure if there is some reason behind it or just a mistake. Take the get
string function of file manager for example: 
chrome/browser/chromeos/extensions/file_browser_private_api.h:383

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/resourc...
File chrome/browser/resources/chromeos/wallpaper_manager/js/main.js (right):

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/resourc...
chrome/browser/resources/chromeos/wallpaper_manager/js/main.js:13: * Called by
main.html after the dom has been parsed.
Sounds good. I will remove the comment here.

On 2012/05/26 14:16:45, flackr wrote:
> This doesn't seem to be true (anymore?). The addEventListener below is self
> explanatory so you don't have to document this.

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/resourc...
File chrome/browser/resources/chromeos/wallpaper_manager/js/main_scripts.js
(right):

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/resourc...
chrome/browser/resources/chromeos/wallpaper_manager/js/main_scripts.js:7:
//<include src="main.js"/>
This is for grit to flatten all the js files listed here to one single js file.
But when accessed directly from file system, this wont do anything. Same as file
manager:
chrome/browser/resources/file_manager/js/main_scripts.js

On 2012/05/26 14:16:45, flackr wrote:
> What are you doing here?

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/resourc...
File chrome/browser/resources/chromeos/wallpaper_manager/main.html (right):

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/resourc...
chrome/browser/resources/chromeos/wallpaper_manager/main.html:15: <!-- This file
has not been flattened, load individual scripts.
Yes. These unflattened files will be used for accessing wallpaper manager
directly from local file system. It's primarily for development purpose. So when
access from the local file, these js files are not flattened into one single and
probably large js file. It is the same technique that file manager used.

On 2012/05/26 14:16:45, flackr wrote:
> What do you mean? Are you using the <if> expression to prevent flattening?

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/chrome_browser....
File chrome/chrome_browser.gypi (right):

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/chrome_browser....
chrome/chrome_browser.gypi:4877: ['exclude',
'^browser/chromeos/extensions/wallpaper_manager_api'],
Right. Deleted it.

On 2012/05/26 14:16:45, flackr wrote:
> This is probably unnecessary given that it's in browser/chromeos, I imagine
this
> entire directory is already excluded.

[flackr, 2012-05-31 18:41:55]

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/resourc...
File chrome/browser/resources/chromeos/wallpaper_manager/main.html (right):

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/resourc...
chrome/browser/resources/chromeos/wallpaper_manager/main.html:15: <!-- This file
has not been flattened, load individual scripts.
I'm just unclear as to why you can't simply list all of the scripts here and do
away with main_scripts.js. It seems like that would work just as well and not
require any special code like this.

On 2012/05/29 16:07:11, bshe wrote:
> Yes. These unflattened files will be used for accessing wallpaper manager
> directly from local file system. It's primarily for development purpose. So
when
> access from the local file, these js files are not flattened into one single
and
> probably large js file. It is the same technique that file manager used.
> 
> On 2012/05/26 14:16:45, flackr wrote:
> > What do you mean? Are you using the <if> expression to prevent flattening?
>

[bshe, 2012-06-19 14:43:29]

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/resourc...
File chrome/browser/resources/chromeos/wallpaper_manager/main.html (right):

https://chromiumcodereview.appspot.com/10453032/diff/1/chrome/browser/resourc...
chrome/browser/resources/chromeos/wallpaper_manager/main.html:15: <!-- This file
has not been flattened, load individual scripts.
I guess the problem was the new added Content Security Policy. It disallowed
inline script. The flatter will inline the javascript if we directly list them
all here. The related CL for file manager is
http://codereview.chromium.org/10068007

On 2012/05/31 18:41:55, flackr wrote:
> I'm just unclear as to why you can't simply list all of the scripts here and
do
> away with main_scripts.js. It seems like that would work just as well and not
> require any special code like this.
> 
> On 2012/05/29 16:07:11, bshe wrote:
> > Yes. These unflattened files will be used for accessing wallpaper manager
> > directly from local file system. It's primarily for development purpose. So
> when
> > access from the local file, these js files are not flattened into one single
> and
> > probably large js file. It is the same technique that file manager used.
> > 
> > On 2012/05/26 14:16:45, flackr wrote:
> > > What do you mean? Are you using the <if> expression to prevent flattening?
> >

[flackr, 2012-07-12 19:40:51]

https://chromiumcodereview.appspot.com/10453032/diff/11001/chrome/browser/res...
File chrome/browser/resources/chromeos/wallpaper_manager/js/main.js (right):

https://chromiumcodereview.appspot.com/10453032/diff/11001/chrome/browser/res...
chrome/browser/resources/chromeos/wallpaper_manager/js/main.js:6: * Global
wallpaperManager reference useful for poking at from the console.
I don't think it's common to include useful debugging code in commits, is it?

https://chromiumcodereview.appspot.com/10453032/diff/11001/chrome/browser/res...
File chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
(right):

https://chromiumcodereview.appspot.com/10453032/diff/11001/chrome/browser/res...
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js:63:
WallpaperManager.prototype.createCategoryItem_ = function(label) {
Document.

https://chromiumcodereview.appspot.com/10453032/diff/11001/chrome/browser/res...
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js:76:
this.createCategoryItem_(str('FEATURED_LABEL')));
Perhaps createCategoryItem should also add the category to the categories list?

https://chromiumcodereview.appspot.com/10453032/diff/11001/chrome/common/exte...
File chrome/common/extensions/api/experimental_wallpaper_manager.json (right):

https://chromiumcodereview.appspot.com/10453032/diff/11001/chrome/common/exte...
chrome/common/extensions/api/experimental_wallpaper_manager.json:13:
"description": "Gets Strings.",
I think you need a better description here. Something like Fetches the localized
strings in use for the wallpaper manager UI.

[Mihai Parparita -not on Chrome, 2012-07-12 19:58:58]

Do you actually want this to be an extension API that's exposed to other
extensions? Or does it make more sense as a system private API for CrOS use only
(see system_private.json, file_browser_private.json, etc.)?

If the former, then this needs to go through the extension API review process:
http://go/extensionsreview