Disable surprise me automatically if the current wallpaper was set by a third party application.

If the current wallpaper was set by a third party application, we should disable surprise me feature,
otherwise the wallpaper will still be changed periodically.

Also fix the issue that wallpaper picker application can't load the local resource:
chrome://resources/css/text_defaults.css.

BUG=466709
TEST=Manually tested

Committed: https://crrev.com/bba16e7a9323bca8d411ff5f51a94093d889df2b
Cr-Commit-Position: refs/heads/master@{#320972}

[xdai1, 2015-03-13 17:50:44]

Patchset #1 (id:1) has been deleted

[xdai1, 2015-03-13 20:44:40]

xdai@chromium.org changed reviewers:
+ bshe@chromium.org

[xdai1, 2015-03-13 20:44:40]

Biao, could you help to review the CL, please? Thanks!

[xdai1, 2015-03-13 20:56:31]

https://codereview.chromium.org/1004263002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/wallpaper_manager/main.html (right):

https://codereview.chromium.org/1004263002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/wallpaper_manager/main.html:12: <link
rel="stylesheet" href="../../../../../ui/webui/resources/css/text_defaults.css">
This could also be fixed by another way: modify the manifest.json file to
include the permission: "chrome://resources". And all the other linked document
below(e.g.: ../../../../../ui/webui/resources/css/widgets.css,
../../../../../ui/webui/resources/css/menu.css, etc) could be replaced by
chrome://resources/** (e.g.: chrome://resources/css/widgets.css, etc) 

I'm not sure which way is better.

[bshe, 2015-03-13 21:02:45]

Interesting case. In general, I feel like that we should disable surprise me
feature when user use a 3rd party wallpaper app changed their wallpaper. WDYT?

https://codereview.chromium.org/1004263002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
(right):

https://codereview.chromium.org/1004263002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js:207:
$('wallpaper-set-by-message').textContent = '';
I think we should display wallpaper set by message if the current wallpaper is
not set by our wallpaper picker.

Perhaps a 3rd party wallpaper change should disable surprise me feature
automatically. User dont expect their wallpaper changed to a new one the second
day they changed their wallpaper through 3rd party app.

https://codereview.chromium.org/1004263002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/chromeos/wallpaper_manager/main.html (right):

https://codereview.chromium.org/1004263002/diff/20001/chrome/browser/resource...
chrome/browser/resources/chromeos/wallpaper_manager/main.html:12: <link
rel="stylesheet" href="../../../../../ui/webui/resources/css/text_defaults.css">
On 2015/03/13 20:56:31, xdai1 wrote:
> This could also be fixed by another way: modify the manifest.json file to
> include the permission: "chrome://resources". And all the other linked
document
> below(e.g.: ../../../../../ui/webui/resources/css/widgets.css,
> ../../../../../ui/webui/resources/css/menu.css, etc) could be replaced by
> chrome://resources/** (e.g.: chrome://resources/css/widgets.css, etc) 
> 
> I'm not sure which way is better. 

Using "../../" should be good enough.

[xdai1, 2015-03-14 01:22:31]

On 2015/03/13 21:02:45, bshe wrote:
> Interesting case. In general, I feel like that we should disable surprise me
> feature when user use a 3rd party wallpaper app changed their wallpaper. WDYT?
> 
>
https://codereview.chromium.org/1004263002/diff/20001/chrome/browser/resource...
> File
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js
> (right):
> 
>
https://codereview.chromium.org/1004263002/diff/20001/chrome/browser/resource...
>
chrome/browser/resources/chromeos/wallpaper_manager/js/wallpaper_manager.js:207:
> $('wallpaper-set-by-message').textContent = '';
> I think we should display wallpaper set by message if the current wallpaper is
> not set by our wallpaper picker.
> 
> Perhaps a 3rd party wallpaper change should disable surprise me feature
> automatically. User dont expect their wallpaper changed to a new one the
second
> day they changed their wallpaper through 3rd party app.
> 
>
https://codereview.chromium.org/1004263002/diff/20001/chrome/browser/resource...
> File chrome/browser/resources/chromeos/wallpaper_manager/main.html (right):
> 
>
https://codereview.chromium.org/1004263002/diff/20001/chrome/browser/resource...
> chrome/browser/resources/chromeos/wallpaper_manager/main.html:12: <link
> rel="stylesheet"
href="../../../../../ui/webui/resources/css/text_defaults.css">
> On 2015/03/13 20:56:31, xdai1 wrote:
> > This could also be fixed by another way: modify the manifest.json file to
> > include the permission: "chrome://resources". And all the other linked
> document
> > below(e.g.: ../../../../../ui/webui/resources/css/widgets.css,
> > ../../../../../ui/webui/resources/css/menu.css, etc) could be replaced by
> > chrome://resources/** (e.g.: chrome://resources/css/widgets.css, etc) 
> > 
> > I'm not sure which way is better. 
> 
> Using "../../" should be good enough.

Biao, I have addressed the comments as we discussed offline. please take another
look at the CL, thanks!

[bshe, 2015-03-16 14:02:41]

https://codereview.chromium.org/1004263002/diff/40001/chrome/browser/chromeos...
File chrome/browser/chromeos/extensions/wallpaper_api.cc (right):

https://codereview.chromium.org/1004263002/diff/40001/chrome/browser/chromeos...
chrome/browser/chromeos/extensions/wallpaper_api.cc:132: event.Pass());
We should send the event only after we know for sure that the new wallpaper
request success. So move the new code to OnWallpaperDecoded is better.

https://codereview.chromium.org/1004263002/diff/40001/chrome/browser/resource...
File chrome/browser/resources/chromeos/wallpaper_manager/manifest.json (right):

https://codereview.chromium.org/1004263002/diff/40001/chrome/browser/resource...
chrome/browser/resources/chromeos/wallpaper_manager/manifest.json:26:
"wallpaper",
After move the event to wallpaperPrivate namespace, you shouldn't need this
permission.

https://codereview.chromium.org/1004263002/diff/40001/chrome/common/extension...
File chrome/common/extensions/api/wallpaper.json (right):

https://codereview.chromium.org/1004263002/diff/40001/chrome/common/extension...
chrome/common/extensions/api/wallpaper.json:64: "events": [
this should go to wallpaper_private.json. Only our built in wallpaper app cares
about the event.

[xdai1, 2015-03-16 17:26:02]

On 2015/03/16 14:02:41, bshe wrote:
>
https://codereview.chromium.org/1004263002/diff/40001/chrome/browser/chromeos...
> File chrome/browser/chromeos/extensions/wallpaper_api.cc (right):
> 
>
https://codereview.chromium.org/1004263002/diff/40001/chrome/browser/chromeos...
> chrome/browser/chromeos/extensions/wallpaper_api.cc:132: event.Pass());
> We should send the event only after we know for sure that the new wallpaper
> request success. So move the new code to OnWallpaperDecoded is better.
> 
>
https://codereview.chromium.org/1004263002/diff/40001/chrome/browser/resource...
> File chrome/browser/resources/chromeos/wallpaper_manager/manifest.json
(right):
> 
>
https://codereview.chromium.org/1004263002/diff/40001/chrome/browser/resource...
> chrome/browser/resources/chromeos/wallpaper_manager/manifest.json:26:
> "wallpaper",
> After move the event to wallpaperPrivate namespace, you shouldn't need this
> permission.
> 
>
https://codereview.chromium.org/1004263002/diff/40001/chrome/common/extension...
> File chrome/common/extensions/api/wallpaper.json (right):
> 
>
https://codereview.chromium.org/1004263002/diff/40001/chrome/common/extension...
> chrome/common/extensions/api/wallpaper.json:64: "events": [
> this should go to wallpaper_private.json. Only our built in wallpaper app
cares
> about the event.

Thanks for the review! Please take another look. Thanks.

[xdai1, 2015-03-17 01:05:07]

xdai@chromium.org changed reviewers:
+ kalman@chromium.org

[xdai1, 2015-03-17 01:06:15]

Add kalman@ for review of chrome/common/extensions/api/wallpaper_private.json
file. Thanks!

[not at google - send to devlin, 2015-03-17 13:42:16]

lgtm with 1 comment

https://codereview.chromium.org/1004263002/diff/60001/chrome/common/extension...
File chrome/common/extensions/api/wallpaper_private.json (right):

https://codereview.chromium.org/1004263002/diff/60001/chrome/common/extension...
chrome/common/extensions/api/wallpaper_private.json:263: "name":
"OnWallpaperChangedBy3rdParty",
"on" not "On"

[bshe, 2015-03-17 13:50:09]

lgtm

[xdai1, 2015-03-17 20:39:30]

The CQ bit was checked by xdai@chromium.org

[xdai1, 2015-03-17 20:39:32]

The patchset sent to the CQ was uploaded after l-g-t-m from bshe@chromium.org,
kalman@chromium.org
Link to the patchset: https://codereview.chromium.org/1004263002/#ps100001
(title: "Address kalman@'s comment.")

[commit-bot: I haz the power, 2015-03-17 20:39:52]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1004263002/100001

[commit-bot: I haz the power, 2015-03-17 21:08:38]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2015-03-17 21:09:52]

Patchset 5 (id:??) landed as
https://crrev.com/bba16e7a9323bca8d411ff5f51a94093d889df2b
Cr-Commit-Position: refs/heads/master@{#320972}