Sync 3rd party wallpaper app name

chrome/browser/resources/chromeos/wallpaper_manager/js/event_page.js

Index: chrome/browser/resources/chromeos/wallpaper_manager/js/event_page.js
diff --git a/chrome/browser/resources/chromeos/wallpaper_manager/js/event_page.js b/chrome/browser/resources/chromeos/wallpaper_manager/js/event_page.js
index 800b25477dc1836503281fb8c58dd3c963345ed3..202012995334209530ddb0b6f68856d39f69b591 100644
--- a/chrome/browser/resources/chromeos/wallpaper_manager/js/event_page.js
+++ b/chrome/browser/resources/chromeos/wallpaper_manager/js/event_page.js
@@ -272,30 +272,32 @@ chrome.syncFileSystem.onFileStatusChanged.addListener(function(detail) {
   WallpaperUtil.enabledSyncThemesCallback(function(syncEnabled) {
     if (!syncEnabled)
       return;
-    if (detail.status == 'synced') {
-      if (detail.direction == 'remote_to_local') {
-        if (detail.action == 'added') {
-          Constants.WallpaperLocalStorage.get(
-              Constants.AccessLocalWallpaperInfoKey,
-              function(items) {
-                var localData = items[Constants.AccessLocalWallpaperInfoKey];
-                if (localData && localData.url == detail.fileEntry.name &&
-                    localData.source == Constants.WallpaperSourceEnum.Custom) {
-                  WallpaperUtil.setCustomWallpaperFromSyncFS(localData.url,
-                                                             localData.layout);
-                } else if (!localData || localData.url !=
-                           detail.fileEntry.name.replace(
-                               Constants.CustomWallpaperThumbnailSuffix, '')) {
-                  // localData might be null on a powerwashed device.
-                  WallpaperUtil.storeWallpaperFromSyncFSToLocalFS(
-                      detail.fileEntry);
-                }
-             });
-        } else if (detail.action == 'deleted') {
-          var fileName = detail.fileEntry.name.replace(
-              Constants.CustomWallpaperThumbnailSuffix, '');
-          WallpaperUtil.deleteWallpaperFromLocalFS(fileName);
+    if (detail.status == 'synced' && detail.direction == 'remote_to_local') {

[tbarzic, 2016/02/05 22:27:37]

nit: I think this would be easier to read:

if (detail.status !== 'synced' || detail.direction != 'remote_to_local')
  return;

[xdai1, 2016/02/06 01:42:30]

Done.

+      if (detail.action == 'added') {
+        // TODO(xdai): Get rid of this setCustomWallpaperFromSyncFS logic.
+        // WallpaperInfo is saved in the sync filesystem before the corresonding

[tbarzic, 2016/02/05 22:27:37]

I'm not sure if this is necessary true. I don't think there is guarantee on
ordering of change events on sync storage (where wallpaper info is stored) and
sync file system (where wallpaper files are stored).

I think localData.url == detail.fileEntry.name is supposed to handle this (i.e.
if local data has already been updated, set the wallpaper, otherwise, expect
wallpaper to be set when sync storage is updated with new wallpaper info)

On the other hand if this event comes before storage.onChange,
setCustomWallpaperFromSyncFS would fail when it attempts to get the wallpaper
from the sync file system.

[xdai1, 2016/02/06 01:42:30]

As discussed offline, modified the comment to better describe it. 

+        // custom wallpaper and thumbnail are saved, thus the onChanged() might
+        // not set the custom wallpaper correctly.
+        Constants.WallpaperLocalStorage.get(
+            Constants.AccessLocalWallpaperInfoKey, function(items) {
+          var localData = items[Constants.AccessLocalWallpaperInfoKey];
+          if (localData &&
+              localData.url == detail.fileEntry.name &&
+              localData.source == Constants.WallpaperSourceEnum.Custom) {
+            WallpaperUtil.setCustomWallpaperFromSyncFS(localData.url,
+                                                       localData.layout);
+          }
+        });
+        // We only need to store the custom wallpaper if it was set by the
+        // built-in wallpaper picker.
+        if (detail.fileEntry.name.indexOf(
+            Constants.ThirdPartyWallpaperPrefix) == -1) {
+          WallpaperUtil.storeWallpaperFromSyncFSToLocalFS(detail.fileEntry);

[tbarzic, 2016/02/05 22:27:37]

Previously, local FS was not updated if call to
wallpaperPrivate.setCustomWallpaper failed, are you sure we don't want to keep
that behaviour?

[xdai1, 2016/02/06 01:42:30]

I prefer the current behavior. Even if the synced wallpaper is not set directly,
we still should save it to the local fs to make it possible to retry/let user to
manually select it from the wallpaper picker. 
No check mark will appear in the wallpaper picker if the synced wallpaper is not
set up correctly.

[tbarzic, 2016/02/06 01:50:14]

OK. 

         }
+      } else if (detail.action == 'deleted') {
+        var fileName = detail.fileEntry.name.replace(
+            Constants.CustomWallpaperThumbnailSuffix, '');
+        WallpaperUtil.deleteWallpaperFromLocalFS(fileName);
       }
     }
   });
@@ -319,7 +321,8 @@ chrome.storage.onChanged.addListener(function(changes, namespace) {
         // If the old wallpaper is a third party wallpaper we should remove it
         // from the local & sync file system to free space.
         var oldInfo = changes[Constants.AccessLocalWallpaperInfoKey].oldValue;
-        if (oldInfo.url.indexOf(Constants.ThirdPartyWallpaperPrefix) != -1) {
+        if (oldInfo &&
+            oldInfo.url.indexOf(Constants.ThirdPartyWallpaperPrefix) != -1) {
           WallpaperUtil.deleteWallpaperFromLocalFS(oldInfo.url);
           WallpaperUtil.deleteWallpaperFromSyncFS(oldInfo.url);
         }
@@ -354,7 +357,7 @@ chrome.storage.onChanged.addListener(function(changes, namespace) {
                 // storage are the same. If the synced value changed by sync
                 // service, they may different. In that case, change wallpaper
                 // to the one saved in sync storage and update the local value.
-                if (localInfo == undefined ||
+                if (!localInfo ||
                     localInfo.url != syncInfo.url ||
                     localInfo.layout != syncInfo.layout ||
                     localInfo.source != syncInfo.source) {
@@ -409,7 +412,7 @@ chrome.alarms.onAlarm.addListener(function() {
 });
 
 chrome.wallpaperPrivate.onWallpaperChangedBy3rdParty.addListener(function(
-    wallpaper, thumbnail, layout) {
+    wallpaper, thumbnail, layout, appName) {
   WallpaperUtil.saveToLocalStorage(
       Constants.AccessLocalSurpriseMeEnabledKey, false, function() {
     WallpaperUtil.saveToSyncStorage(Constants.AccessSyncSurpriseMeEnabledKey,
@@ -418,11 +421,10 @@ chrome.wallpaperPrivate.onWallpaperChangedBy3rdParty.addListener(function(
   SurpriseWallpaper.getInstance().disable();
 
   // Make third party wallpaper syncable through different devices.
-  // TODO(xdai): also sync the third party app name.
   var filename = Constants.ThirdPartyWallpaperPrefix + new Date().getTime();
   var thumbnailFilename = filename + Constants.CustomWallpaperThumbnailSuffix;
   WallpaperUtil.storeWallpaperToSyncFS(filename, wallpaper);
   WallpaperUtil.storeWallpaperToSyncFS(thumbnailFilename, thumbnail);
-  WallpaperUtil.saveWallpaperInfo(filename, layout,
-                                  Constants.WallpaperSourceEnum.Custom);
+  WallpaperUtil.saveWallpaperInfo(
+      filename, layout, Constants.WallpaperSourceEnum.Custom, appName);
 });