Sync 3rd party wallpaper app name

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

 // Copyright (c) 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 var WALLPAPER_PICKER_WIDTH = 574;
 var WALLPAPER_PICKER_HEIGHT = 420;
 
 var wallpaperPickerWindow;
 
 var surpriseWallpaper = null;
@@ skipping 250 matching lines @@
   }, function(w) {
     wallpaperPickerWindow = w;
     chrome.wallpaperPrivate.minimizeInactiveWindows();
     w.onClosed.addListener(function() {
       chrome.wallpaperPrivate.restoreMinimizedWindows();
     });
     WallpaperUtil.testSendMessage('wallpaper-window-created');
   });
 });
 
 chrome.syncFileSystem.onFileStatusChanged.addListener(function(detail) {

[tbarzic, 2016/02/04 21:31:24]

More suitable for another cl:
sync file system is updated both for thumbnails and wallpaper files, should they
be handled differently (e.g. ignore changes to thumbnail changes)?

[xdai1, 2016/02/05 01:09:52]

Sync file system is updated both for thumbnails and wallpaper files. But they
are handled in the same way. We simply store them to the local fs (see line
295-300 in this patch, or 291-295 in the new patch). 

But after looking into this function, I think this function needs to be
rewritten. The 'added' event should only make sense for a newly setup device to
sync down all custom wallpapers and thumbnails. Add a TODO here.

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

Oh, yeah, I see what you mean after second reading of the code, my bad :)

   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 &&
+                if (!localData) {
+                  // localData might be null on a powerwashed device.
+                  WallpaperUtil.storeWallpaperFromSyncFSToLocalFS(
+                      detail.fileEntry);
+                } else if (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.
+                  if (localData.hasOwnProperty('appName')) {
+                    WallpaperUtil.setCustomWallpaperFromSyncFS(
+                        localData.url, localData.layout, localData.appName);

[tbarzic, 2016/02/04 21:31:24]

I'd just use localData.appName || '' (instead of if (hasProperty(appName)) {set
with appName} else {set with ''})

Also, localData.appName === undefined would be a good signal that the wallpaper
in sync storage came from a device that does not yet support syncing third party
app name.

+                  } else {
+                    WallpaperUtil.setCustomWallpaperFromSyncFS(
+                        localData.url, localData.layout, '');
+                  }
+                } else if (localData.url != detail.fileEntry.name.replace(
+                    Constants.CustomWallpaperThumbnailSuffix, '') &&
+                    detail.fileEntry.name.indexOf(
+                        Constants.ThirdPartyWallpaperPrefix) == -1) {
                   WallpaperUtil.storeWallpaperFromSyncFSToLocalFS(
                       detail.fileEntry);
                 }
              });
         } else if (detail.action == 'deleted') {
           var fileName = detail.fileEntry.name.replace(
               Constants.CustomWallpaperThumbnailSuffix, '');
           WallpaperUtil.deleteWallpaperFromLocalFS(fileName);
         }
       }
@@ skipping 12 matching lines @@
           SurpriseWallpaper.getInstance().next();
         } else {
           SurpriseWallpaper.getInstance().disable();
         }
       }
 
       if (changes[Constants.AccessLocalWallpaperInfoKey]) {
         // 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);
         }
       }
 
       if (changes[Constants.AccessSyncWallpaperInfoKey]) {
         var syncInfo = changes[Constants.AccessSyncWallpaperInfoKey].newValue;
 
         Constants.WallpaperSyncStorage.get(
             Constants.AccessSyncSurpriseMeEnabledKey, function(enabledItems) {
@@ skipping 14 matching lines @@
                 (syncSurpriseMeEnabled &&
                  syncLastSurpriseMeChangedDate == today)) {
               Constants.WallpaperLocalStorage.get(
                   Constants.AccessLocalWallpaperInfoKey, function(infoItems) {
                 var localInfo =
                     infoItems[Constants.AccessLocalWallpaperInfoKey];
                 // Normally, the wallpaper info saved in local storage and sync
                 // 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) {
                   if (syncInfo.source == Constants.WallpaperSourceEnum.Online) {
                     // TODO(bshe): Consider schedule an alarm to set online
                     // wallpaper later when failed. Note that we need to cancel
                     // the retry if user set another wallpaper before retry
                     // alarm invoked.
                     WallpaperUtil.setOnlineWallpaper(syncInfo.url,
                         syncInfo.layout, function() {}, function() {});
                   } else if (syncInfo.source ==
                              Constants.WallpaperSourceEnum.Custom) {
-                    WallpaperUtil.setCustomWallpaperFromSyncFS(syncInfo.url,
-                                                               syncInfo.layout);
+                    if (syncInfo.hasOwnProperty('appName')) {
+                      WallpaperUtil.setCustomWallpaperFromSyncFS(
+                          syncInfo.url, syncInfo.layout, syncInfo.appName);

[tbarzic, 2016/02/04 21:31:24]

How about:
WallpaperUtil.setCustomWallpaperFromSyncFS(url, layout, syncInfo.appName || '')

[xdai1, 2016/02/05 01:09:52]

As explained in WallpaperUtil.setCustomWallpaperFromSyncFS() function, I think
we might not need appName as a parameter. Revert it back.

+                    } else {
+                      WallpaperUtil.setCustomWallpaperFromSyncFS(
+                          syncInfo.url, syncInfo.layout, '');
+                    }
                   } else if (syncInfo.source ==
                               Constants.WallpaperSourceEnum.Default) {
                     chrome.wallpaperPrivate.resetWallpaper();
                   }
 
                   // If the old wallpaper is a third party wallpaper we should
                   // remove it from the local & sync file system to free space.
                   if (localInfo && localInfo.url.indexOf(
                       Constants.ThirdPartyWallpaperPrefix) != -1) {
                     WallpaperUtil.deleteWallpaperFromLocalFS(localInfo.url);
@@ skipping 20 matching lines @@
       }
     }
   });
 });
 
 chrome.alarms.onAlarm.addListener(function() {
   SurpriseWallpaper.getInstance().next();
 });
 
 chrome.wallpaperPrivate.onWallpaperChangedBy3rdParty.addListener(function(
-    wallpaper, thumbnail, layout) {
+    wallpaper, thumbnail, layout, appName) {
   WallpaperUtil.saveToLocalStorage(
       Constants.AccessLocalSurpriseMeEnabledKey, false, function() {
     WallpaperUtil.saveToSyncStorage(Constants.AccessSyncSurpriseMeEnabledKey,
                                     false);
   });
   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(

[tbarzic, 2016/02/04 21:31:24]

Not introduced here, but might make for a good follow-up:
it looks like chrome.syncFileSystem.onFileStatusChanged listener depend on local
data being set, in which case this seems a bit racy.
Maybe update sync fs in a callback to saveWallpaperInfo?

[xdai1, 2016/02/05 01:09:52]

Might not true. It's the onChanged() event which is triggered by wallpaperInfo
change that sync down and set the custom/third party wallpaper.

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

Oh, yeah, syncFileSystem,onFileStatusChange only handles remote -> local
updates. D'oh.

+      filename, layout, Constants.WallpaperSourceEnum.Custom, appName);
 });