Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2965)

Unified Diff: chrome/browser/resources/chromeos/wallpaper_manager/js/event_page.js

Issue 956403003: Fix the issue that wallpaper changes twice when both "onChanged" and "onAlarm" are fired. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Address the code review comments. Created 5 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
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 9e693e4c5843be3f6589941d6df2b8dcbe520418..44a31a875c7091df3167c1e8dc31a599ecf78042 100644
--- a/chrome/browser/resources/chromeos/wallpaper_manager/js/event_page.js
+++ b/chrome/browser/resources/chromeos/wallpaper_manager/js/event_page.js
@@ -31,6 +31,8 @@ SurpriseWallpaper.prototype.tryChangeWallpaper = function() {
var self = this;
var onFailure = function() {
self.retryLater_();
+ };
+ var onNotFound = function() {
bshe 2015/03/09 14:16:09 If not found, I think we should just use a random
xdai1 2015/03/09 21:07:10 If not found (404), we randomly set a wallpaper di
self.fallbackToLocalRss_();
};
// Try to fetch newest rss as document from server first. If any error occurs,
@@ -39,7 +41,7 @@ SurpriseWallpaper.prototype.tryChangeWallpaper = function() {
WallpaperUtil.saveToStorage(Constants.AccessRssKey,
new XMLSerializer().serializeToString(xhr.responseXML), false);
self.updateSurpriseWallpaper(xhr.responseXML);
- }, onFailure);
+ }, onFailure, onNotFound);
};
/**
@@ -89,15 +91,26 @@ SurpriseWallpaper.prototype.updateSurpriseWallpaper = function(opt_rss) {
var regionsString = item.getElementsByTagNameNS(
Constants.WallpaperNameSpaceURI, 'regions')[0].textContent;
var regions = regionsString.split(', ');
+
+ var self = this;
+ var onSuccess = function(url, layout) {
+ var dateString = new Date().toDateString();
+ WallpaperUtil.saveWallpaperInfo(url, layout,
+ Constants.WallpaperSourceEnum.Online);
+ WallpaperUtil.saveToStorage(
+ Constants.AccessLastSurpriseWallpaperChangedDate,
+ dateString, true);
+ };
+ var onFailure = function() {
+ self.retryLater_();
+ };
+ var onNotFound = function() {
+ self.updateRandomWallpaper_();
bshe 2015/03/09 14:16:09 would be nice to handle the not found case in fail
xdai1 2015/03/09 21:07:10 Done.
+ };
+
if (enableDate <= date && disableDate > date &&
regions.indexOf(navigator.language) != -1) {
- var self = this;
- this.setWallpaperFromRssItem_(item,
- function() {},
- function() {
- self.retryLater_();
- self.updateRandomWallpaper_();
- });
+ this.setWallpaperFromRssItem_(item, onSuccess, onFailure, onNotFound);
return;
}
}
@@ -146,6 +159,8 @@ SurpriseWallpaper.prototype.setRandomWallpaper_ = function(dateString) {
var wallpaper = filtered[index];
var wallpaperURL = wallpaper.base_url + Constants.HighResolutionSuffix;
var onSuccess = function() {
+ WallpaperUtil.saveWallpaperInfo(wallpaperURL, wallpaper.default_layout,
+ Constants.WallpaperSourceEnum.Online);
WallpaperUtil.saveToStorage(
Constants.AccessLastSurpriseWallpaperChangedDate,
dateString,
@@ -163,11 +178,13 @@ SurpriseWallpaper.prototype.setRandomWallpaper_ = function(dateString) {
* @param {Element} item The wallpaper rss item element.
* @param {function} onSuccess Success callback.
* @param {function} onFailure Failure callback.
+ * @param {function} onNotFound Not found callback.
* @private
*/
SurpriseWallpaper.prototype.setWallpaperFromRssItem_ = function(item,
onSuccess,
- onFailure) {
+ onFailure,
+ onNotFound) {
var url = item.querySelector('link').textContent;
var layout = item.getElementsByTagNameNS(
Constants.WallpaperNameSpaceURI, 'layout')[0].textContent;
@@ -175,12 +192,11 @@ SurpriseWallpaper.prototype.setWallpaperFromRssItem_ = function(item,
WallpaperUtil.fetchURL(url, 'arraybuffer', function(xhr) {
if (xhr.response != null) {
chrome.wallpaperPrivate.setCustomWallpaper(xhr.response, layout, false,
- 'surprise_wallpaper',
- onSuccess);
+ 'surprise_wallpaper', onSuccess(url, layout));
} else {
onFailure();
}
- }, onFailure);
+ }, onFailure, onNotFound);
};
/**
@@ -277,33 +293,57 @@ chrome.storage.onChanged.addListener(function(changes, namespace) {
}
if (changes[Constants.AccessSyncWallpaperInfoKey]) {
- var newValue = changes[Constants.AccessSyncWallpaperInfoKey].newValue;
- Constants.WallpaperLocalStorage.get(Constants.AccessLocalWallpaperInfoKey,
- function(items) {
- // 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.
- var localValue = items[Constants.AccessLocalWallpaperInfoKey];
- if (localValue == undefined ||
- localValue.url != newValue.url ||
- localValue.layout != newValue.layout ||
- localValue.source != newValue.source) {
- if (newValue.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(newValue.url, newValue.layout,
- function() {}, function() {});
- } else if (newValue.source == Constants.WallpaperSourceEnum.Custom) {
- WallpaperUtil.setCustomWallpaperFromSyncFS(newValue.url,
- newValue.layout);
- } else if (newValue.source == Constants.WallpaperSourceEnum.Default) {
- chrome.wallpaperPrivate.resetWallpaper();
+ var syncInfo = changes[Constants.AccessSyncWallpaperInfoKey].newValue;
+
+ Constants.WallpaperSyncStorage.get(Constants.AccessSurpriseMeEnabledKey,
+ function(enabledItems) {
+ var syncSurpriseMeEnabled =
+ enabledItems[Constants.AccessSurpriseMeEnabledKey];
+
+ Constants.WallpaperSyncStorage.get(
+ Constants.AccessLastSurpriseWallpaperChangedDate, function(items) {
+ var syncLastSurpriseMeChangedDate =
+ items[Constants.AccessLastSurpriseWallpaperChangedDate];
+
+ var today = new Date().toDateString();
+ // If SurpriseMe is enabled and surprise wallpaper hasn't been changed
+ // today, we should not sync the change, instead onAlarm() will be
+ // triggered to update a surprise me wallpaper.
+ if (!syncSurpriseMeEnabled ||
+ (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 ||
+ 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);
+ } else if (syncInfo.source ==
+ Constants.WallpaperSourceEnum.Default) {
+ chrome.wallpaperPrivate.resetWallpaper();
+ }
+ WallpaperUtil.saveToStorage(
+ Constants.AccessLocalWallpaperInfoKey, syncInfo, false);
+ }
+ });
}
- WallpaperUtil.saveToStorage(Constants.AccessLocalWallpaperInfoKey,
- newValue, false);
- }
+ });
});
}
});

Powered by Google App Engine
This is Rietveld 408576698