From 93bc8c85c7a291717b03a4eae1ceb30b4235031d Mon Sep 17 00:00:00 2001 From: Neil Deakin Date: Wed, 21 Aug 2013 09:36:07 +0200 Subject: [PATCH 01/13] Bug 847863 - Part 6 of 8 - Convert tests for clearing recent history. r=paolo --- browser/base/content/sanitize.js | 80 ++-- .../test/browser_sanitize-timespans.js | 436 +++++++----------- .../content/test/browser_sanitizeDialog.js | 165 +++---- 3 files changed, 309 insertions(+), 372 deletions(-) diff --git a/browser/base/content/sanitize.js b/browser/base/content/sanitize.js index 771783e69eb..5c3aa05e936 100644 --- a/browser/base/content/sanitize.js +++ b/browser/base/content/sanitize.js @@ -16,7 +16,7 @@ XPCOMUtils.defineLazyModuleGetter(this, "Task", "resource://gre/modules/Task.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "DownloadsCommon", "resource:///modules/DownloadsCommon.jsm"); - + function Sanitizer() {} Sanitizer.prototype = { // warning to the caller: this one may raise an exception (e.g. bug #265028) @@ -37,14 +37,14 @@ Sanitizer.prototype = { aCallback(aItemName, canClear, aArg); return canClear; }, - + prefDomain: "", - + getNameFromPreference: function (aPreferenceName) { return aPreferenceName.substr(this.prefDomain.length); }, - + /** * Deletes privacy sensitive data in a batch, according to user preferences. * Returns a promise which is resolved if no errors occurred. If an error @@ -87,7 +87,8 @@ Sanitizer.prototype = { item.clear(); } catch(er) { seenError = true; - Cu.reportError("Error sanitizing " + itemName + ": " + er + "\n"); + Components.utils.reportError("Error sanitizing " + itemName + + ": " + er + "\n"); } onItemComplete(); }; @@ -99,7 +100,7 @@ Sanitizer.prototype = { return deferred.promise; }, - + // Time span only makes sense in certain cases. Consumers who want // to only clear some private data can opt in by setting this to false, // and can optionally specify a specific range. If timespan is not ignored, @@ -107,7 +108,7 @@ Sanitizer.prototype = { // pref to determine a range ignoreTimespan : true, range : null, - + items: { cache: { clear: function () @@ -126,13 +127,13 @@ Sanitizer.prototype = { imageCache.clearCache(false); // true=chrome, false=content } catch(er) {} }, - + get canClear() { return true; } }, - + cookies: { clear: function () { @@ -143,7 +144,7 @@ Sanitizer.prototype = { var cookiesEnum = cookieMgr.enumerator; while (cookiesEnum.hasMoreElements()) { var cookie = cookiesEnum.getNext().QueryInterface(Ci.nsICookie2); - + if (cookie.creationTime > this.range[0]) // This cookie was created after our cutoff, clear it cookieMgr.remove(cookie.host, cookie.name, cookie.path, false); @@ -211,14 +212,14 @@ Sanitizer.prototype = { PlacesUtils.history.removeVisitsByTimeframe(this.range[0], this.range[1]); else PlacesUtils.history.removeAllPages(); - + try { var os = Components.classes["@mozilla.org/observer-service;1"] .getService(Components.interfaces.nsIObserverService); os.notifyObservers(null, "browser:purge-session-history", ""); } catch (e) { } - + // Clear last URL of the Open Web Location dialog var prefs = Components.classes["@mozilla.org/preferences-service;1"] .getService(Components.interfaces.nsIPrefBranch); @@ -227,7 +228,7 @@ Sanitizer.prototype = { } catch (e) { } }, - + get canClear() { // bug 347231: Always allow clearing history due to dependencies on @@ -235,7 +236,7 @@ Sanitizer.prototype = { return true; } }, - + formdata: { clear: function () { @@ -305,15 +306,20 @@ Sanitizer.prototype = { return false; } }, - + downloads: { clear: function () { if (DownloadsCommon.useJSTransfer) { Task.spawn(function () { - let filterByTime = this.range ? - (download => download.startTime >= this.range[0] && - download.startTime <= this.range[1]) : null; + let filterByTime = null; + if (this.range) { + // Convert microseconds back to milliseconds for date comparisons. + let rangeBeginMs = this.range[0] / 1000; + let rangeEndMs = this.range[1] / 1000; + filterByTime = download => download.startTime >= rangeBeginMs && + download.startTime <= rangeEndMs; + } // Clear all completed/cancelled downloads let publicList = yield Downloads.getPublicDownloadList(); @@ -321,7 +327,7 @@ Sanitizer.prototype = { let privateList = yield Downloads.getPrivateDownloadList(); privateList.removeFinished(filterByTime); - }.bind(this)).then(null, Cu.reportError); + }.bind(this)).then(null, Components.utils.reportError); } else { var dlMgr = Components.classes["@mozilla.org/download-manager;1"] @@ -352,7 +358,7 @@ Sanitizer.prototype = { return false; } }, - + passwords: { clear: function () { @@ -361,7 +367,7 @@ Sanitizer.prototype = { // Passwords are timeless, and don't respect the timeSpan setting pwmgr.removeAllLogins(); }, - + get canClear() { var pwmgr = Components.classes["@mozilla.org/login-manager;1"] @@ -370,7 +376,7 @@ Sanitizer.prototype = { return (count > 0); } }, - + sessions: { clear: function () { @@ -384,13 +390,13 @@ Sanitizer.prototype = { .getService(Components.interfaces.nsIObserverService); os.notifyObservers(null, "net:clear-active-logins", null); }, - + get canClear() { return true; } }, - + siteSettings: { clear: function () { @@ -398,12 +404,12 @@ Sanitizer.prototype = { var pm = Components.classes["@mozilla.org/permissionmanager;1"] .getService(Components.interfaces.nsIPermissionManager); pm.removeAll(); - + // Clear site-specific settings like page-zoom level var cps = Components.classes["@mozilla.org/content-pref/service;1"] .getService(Components.interfaces.nsIContentPrefService2); cps.removeAllDomains(null); - + // Clear "Never remember passwords for this site", which is not handled by // the permission manager var pwmgr = Components.classes["@mozilla.org/login-manager;1"] @@ -413,7 +419,7 @@ Sanitizer.prototype = { pwmgr.setLoginSavingEnabled(host, true); } }, - + get canClear() { return true; @@ -446,7 +452,7 @@ Sanitizer.getClearRange = function (ts) { ts = Sanitizer.prefs.getIntPref("timeSpan"); if (ts === Sanitizer.TIMESPAN_EVERYTHING) return null; - + // PRTime is microseconds while JS time is milliseconds var endDate = Date.now() * 1000; switch (ts) { @@ -473,7 +479,7 @@ Sanitizer.getClearRange = function (ts) { }; Sanitizer._prefs = null; -Sanitizer.__defineGetter__("prefs", function() +Sanitizer.__defineGetter__("prefs", function() { return Sanitizer._prefs ? Sanitizer._prefs : Sanitizer._prefs = Components.classes["@mozilla.org/preferences-service;1"] @@ -482,7 +488,7 @@ Sanitizer.__defineGetter__("prefs", function() }); // Shows sanitization UI -Sanitizer.showUI = function(aParentWindow) +Sanitizer.showUI = function(aParentWindow) { var ww = Components.classes["@mozilla.org/embedcomp/window-watcher;1"] .getService(Components.interfaces.nsIWindowWatcher); @@ -497,32 +503,32 @@ Sanitizer.showUI = function(aParentWindow) null); }; -/** - * Deletes privacy sensitive data in a batch, optionally showing the +/** + * Deletes privacy sensitive data in a batch, optionally showing the * sanitize UI, according to user preferences */ -Sanitizer.sanitize = function(aParentWindow) +Sanitizer.sanitize = function(aParentWindow) { Sanitizer.showUI(aParentWindow); }; -Sanitizer.onStartup = function() +Sanitizer.onStartup = function() { // we check for unclean exit with pending sanitization Sanitizer._checkAndSanitize(); }; -Sanitizer.onShutdown = function() +Sanitizer.onShutdown = function() { // we check if sanitization is needed and perform it Sanitizer._checkAndSanitize(); }; // this is called on startup and shutdown, to perform pending sanitizations -Sanitizer._checkAndSanitize = function() +Sanitizer._checkAndSanitize = function() { const prefs = Sanitizer.prefs; - if (prefs.getBoolPref(Sanitizer.prefShutdown) && + if (prefs.getBoolPref(Sanitizer.prefShutdown) && !prefs.prefHasUserValue(Sanitizer.prefDidShutdown)) { // this is a shutdown or a startup after an unclean exit var s = new Sanitizer(); diff --git a/browser/base/content/test/browser_sanitize-timespans.js b/browser/base/content/test/browser_sanitize-timespans.js index 6c6fb059b9f..e470b22cc61 100644 --- a/browser/base/content/test/browser_sanitize-timespans.js +++ b/browser/base/content/test/browser_sanitize-timespans.js @@ -2,10 +2,10 @@ Cu.import("resource://gre/modules/XPCOMUtils.jsm"); Cu.import("resource://gre/modules/Services.jsm"); // Bug 453440 - Test the timespan-based logic of the sanitizer code -var now_uSec = Date.now() * 1000; - -const dm = Cc["@mozilla.org/download-manager;1"].getService(Ci.nsIDownloadManager); +let now_mSec = Date.now(); +let now_uSec = now_mSec * 1000; +const kMsecPerMin = 60 * 1000; const kUsecPerMin = 60 * 1000000; let tempScope = {}; @@ -14,6 +14,7 @@ Cc["@mozilla.org/moz/jssubscript-loader;1"].getService(Ci.mozIJSSubScriptLoader) let Sanitizer = tempScope.Sanitizer; let FormHistory = (Components.utils.import("resource://gre/modules/FormHistory.jsm", {})).FormHistory; +let Downloads = (Components.utils.import("resource://gre/modules/Downloads.jsm", {})).Downloads; function promiseFormHistoryRemoved() { let deferred = Promise.defer(); @@ -24,15 +25,30 @@ function promiseFormHistoryRemoved() { return deferred.promise; } +function promiseDownloadRemoved(list) { + let deferred = Promise.defer(); + + let view = { + onDownloadRemoved: function(download) { + list.removeView(view); + deferred.resolve(); + } + }; + + list.addView(view); + + return deferred.promise; +} + function test() { waitForExplicitFinish(); Task.spawn(function() { - setupDownloads(); + yield setupDownloads(); yield setupFormHistory(); yield setupHistory(); yield onHistoryReady(); - }).then(finish); + }).then(null, ex => ok(false, ex)).then(finish); } function countEntries(name, message, check) { @@ -80,12 +96,16 @@ function onHistoryReady() { itemPrefs.setBoolPref("sessions", false); itemPrefs.setBoolPref("siteSettings", false); + let publicList = yield Downloads.getPublicDownloadList(); + let downloadPromise = promiseDownloadRemoved(publicList); + // Clear 10 minutes ago s.range = [now_uSec - 10*60*1000000, now_uSec]; s.sanitize(); s.range = null; yield promiseFormHistoryRemoved(); + yield downloadPromise; ok(!(yield promiseIsURIVisited(makeURI("http://10minutes.com"))), "Pretend visit to 10minutes.com should now be deleted"); @@ -122,23 +142,26 @@ function onHistoryReady() { yield countEntries("today", "today form entry should still exist", checkOne); yield countEntries("b4today", "b4today form entry should still exist", checkOne); - ok(!downloadExists(5555555), "10 minute download should now be deleted"); - ok(downloadExists(5555551), "<1 hour download should still be present"); - ok(downloadExists(5555556), "1 hour 10 minute download should still be present"); - ok(downloadExists(5555550), "Year old download should still be present"); - ok(downloadExists(5555552), "<2 hour old download should still be present"); - ok(downloadExists(5555557), "2 hour 10 minute download should still be present"); - ok(downloadExists(5555553), "<4 hour old download should still be present"); - ok(downloadExists(5555558), "4 hour 10 minute download should still be present"); + ok(!(yield downloadExists(publicList, "fakefile-10-minutes")), "10 minute download should now be deleted"); + ok((yield downloadExists(publicList, "fakefile-1-hour")), "<1 hour download should still be present"); + ok((yield downloadExists(publicList, "fakefile-1-hour-10-minutes")), "1 hour 10 minute download should still be present"); + ok((yield downloadExists(publicList, "fakefile-old")), "Year old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-2-hour")), "<2 hour old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-2-hour-10-minutes")), "2 hour 10 minute download should still be present"); + ok((yield downloadExists(publicList, "fakefile-4-hour")), "<4 hour old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-4-hour-10-minutes")), "4 hour 10 minute download should still be present"); if (minutesSinceMidnight > 10) - ok(downloadExists(5555554), "'Today' download should still be present"); + ok((yield downloadExists(publicList, "fakefile-today")), "'Today' download should still be present"); + + downloadPromise = promiseDownloadRemoved(publicList); // Clear 1 hour Sanitizer.prefs.setIntPref("timeSpan", 1); s.sanitize(); yield promiseFormHistoryRemoved(); + yield downloadPromise; ok(!(yield promiseIsURIVisited(makeURI("http://1hour.com"))), "Pretend visit to 1hour.com should now be deleted"); @@ -169,23 +192,26 @@ function onHistoryReady() { yield countEntries("today", "today form entry should still exist", checkOne); yield countEntries("b4today", "b4today form entry should still exist", checkOne); - ok(!downloadExists(5555551), "<1 hour download should now be deleted"); - ok(downloadExists(5555556), "1 hour 10 minute download should still be present"); - ok(downloadExists(5555550), "Year old download should still be present"); - ok(downloadExists(5555552), "<2 hour old download should still be present"); - ok(downloadExists(5555557), "2 hour 10 minute download should still be present"); - ok(downloadExists(5555553), "<4 hour old download should still be present"); - ok(downloadExists(5555558), "4 hour 10 minute download should still be present"); + ok(!(yield downloadExists(publicList, "fakefile-1-hour")), "<1 hour download should now be deleted"); + ok((yield downloadExists(publicList, "fakefile-1-hour-10-minutes")), "1 hour 10 minute download should still be present"); + ok((yield downloadExists(publicList, "fakefile-old")), "Year old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-2-hour")), "<2 hour old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-2-hour-10-minutes")), "2 hour 10 minute download should still be present"); + ok((yield downloadExists(publicList, "fakefile-4-hour")), "<4 hour old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-4-hour-10-minutes")), "4 hour 10 minute download should still be present"); if (hoursSinceMidnight > 1) - ok(downloadExists(5555554), "'Today' download should still be present"); + ok((yield downloadExists(publicList, "fakefile-today")), "'Today' download should still be present"); + downloadPromise = promiseDownloadRemoved(publicList); + // Clear 1 hour 10 minutes s.range = [now_uSec - 70*60*1000000, now_uSec]; s.sanitize(); s.range = null; yield promiseFormHistoryRemoved(); + yield downloadPromise; ok(!(yield promiseIsURIVisited(makeURI("http://1hour10minutes.com"))), "Pretend visit to 1hour10minutes.com should now be deleted"); @@ -213,20 +239,23 @@ function onHistoryReady() { yield countEntries("today", "today form entry should still exist", checkOne); yield countEntries("b4today", "b4today form entry should still exist", checkOne); - ok(!downloadExists(5555556), "1 hour 10 minute old download should now be deleted"); - ok(downloadExists(5555550), "Year old download should still be present"); - ok(downloadExists(5555552), "<2 hour old download should still be present"); - ok(downloadExists(5555557), "2 hour 10 minute download should still be present"); - ok(downloadExists(5555553), "<4 hour old download should still be present"); - ok(downloadExists(5555558), "4 hour 10 minute download should still be present"); + ok(!(yield downloadExists(publicList, "fakefile-1-hour-10-minutes")), "1 hour 10 minute old download should now be deleted"); + ok((yield downloadExists(publicList, "fakefile-old")), "Year old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-2-hour")), "<2 hour old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-2-hour-10-minutes")), "2 hour 10 minute download should still be present"); + ok((yield downloadExists(publicList, "fakefile-4-hour")), "<4 hour old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-4-hour-10-minutes")), "4 hour 10 minute download should still be present"); if (minutesSinceMidnight > 70) - ok(downloadExists(5555554), "'Today' download should still be present"); + ok((yield downloadExists(publicList, "fakefile-today")), "'Today' download should still be present"); + + downloadPromise = promiseDownloadRemoved(publicList); // Clear 2 hours Sanitizer.prefs.setIntPref("timeSpan", 2); s.sanitize(); yield promiseFormHistoryRemoved(); + yield downloadPromise; ok(!(yield promiseIsURIVisited(makeURI("http://2hour.com"))), "Pretend visit to 2hour.com should now be deleted"); @@ -251,20 +280,23 @@ function onHistoryReady() { yield countEntries("today", "today form entry should still exist", checkOne); yield countEntries("b4today", "b4today form entry should still exist", checkOne); - ok(!downloadExists(5555552), "<2 hour old download should now be deleted"); - ok(downloadExists(5555550), "Year old download should still be present"); - ok(downloadExists(5555557), "2 hour 10 minute download should still be present"); - ok(downloadExists(5555553), "<4 hour old download should still be present"); - ok(downloadExists(5555558), "4 hour 10 minute download should still be present"); + ok(!(yield downloadExists(publicList, "fakefile-2-hour")), "<2 hour old download should now be deleted"); + ok((yield downloadExists(publicList, "fakefile-old")), "Year old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-2-hour-10-minutes")), "2 hour 10 minute download should still be present"); + ok((yield downloadExists(publicList, "fakefile-4-hour")), "<4 hour old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-4-hour-10-minutes")), "4 hour 10 minute download should still be present"); if (hoursSinceMidnight > 2) - ok(downloadExists(5555554), "'Today' download should still be present"); + ok((yield downloadExists(publicList, "fakefile-today")), "'Today' download should still be present"); + downloadPromise = promiseDownloadRemoved(publicList); + // Clear 2 hours 10 minutes s.range = [now_uSec - 130*60*1000000, now_uSec]; s.sanitize(); s.range = null; yield promiseFormHistoryRemoved(); + yield downloadPromise; ok(!(yield promiseIsURIVisited(makeURI("http://2hour10minutes.com"))), "Pretend visit to 2hour10minutes.com should now be deleted"); @@ -286,18 +318,21 @@ function onHistoryReady() { yield countEntries("today", "today form entry should still exist", checkOne); yield countEntries("b4today", "b4today form entry should still exist", checkOne); - ok(!downloadExists(5555557), "2 hour 10 minute old download should now be deleted"); - ok(downloadExists(5555553), "<4 hour old download should still be present"); - ok(downloadExists(5555558), "4 hour 10 minute download should still be present"); - ok(downloadExists(5555550), "Year old download should still be present"); + ok(!(yield downloadExists(publicList, "fakefile-2-hour-10-minutes")), "2 hour 10 minute old download should now be deleted"); + ok((yield downloadExists(publicList, "fakefile-4-hour")), "<4 hour old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-4-hour-10-minutes")), "4 hour 10 minute download should still be present"); + ok((yield downloadExists(publicList, "fakefile-old")), "Year old download should still be present"); if (minutesSinceMidnight > 130) - ok(downloadExists(5555554), "'Today' download should still be present"); + ok((yield downloadExists(publicList, "fakefile-today")), "'Today' download should still be present"); + + downloadPromise = promiseDownloadRemoved(publicList); // Clear 4 hours Sanitizer.prefs.setIntPref("timeSpan", 3); s.sanitize(); yield promiseFormHistoryRemoved(); + yield downloadPromise; ok(!(yield promiseIsURIVisited(makeURI("http://4hour.com"))), "Pretend visit to 4hour.com should now be deleted"); @@ -316,11 +351,13 @@ function onHistoryReady() { yield countEntries("today", "today form entry should still exist", checkOne); yield countEntries("b4today", "b4today form entry should still exist", checkOne); - ok(!downloadExists(5555553), "<4 hour old download should now be deleted"); - ok(downloadExists(5555558), "4 hour 10 minute download should still be present"); - ok(downloadExists(5555550), "Year old download should still be present"); + ok(!(yield downloadExists(publicList, "fakefile-4-hour")), "<4 hour old download should now be deleted"); + ok((yield downloadExists(publicList, "fakefile-4-hour-10-minutes")), "4 hour 10 minute download should still be present"); + ok((yield downloadExists(publicList, "fakefile-old")), "Year old download should still be present"); if (hoursSinceMidnight > 4) - ok(downloadExists(5555554), "'Today' download should still be present"); + ok((yield downloadExists(publicList, "fakefile-today")), "'Today' download should still be present"); + + downloadPromise = promiseDownloadRemoved(publicList); // Clear 4 hours 10 minutes s.range = [now_uSec - 250*60*1000000, now_uSec]; @@ -328,6 +365,7 @@ function onHistoryReady() { s.range = null; yield promiseFormHistoryRemoved(); + yield downloadPromise; ok(!(yield promiseIsURIVisited(makeURI("http://4hour10minutes.com"))), "Pretend visit to 4hour10minutes.com should now be deleted"); @@ -343,48 +381,60 @@ function onHistoryReady() { yield countEntries("today", "today form entry should still exist", checkOne); yield countEntries("b4today", "b4today form entry should still exist", checkOne); - ok(!downloadExists(5555558), "4 hour 10 minute download should now be deleted"); - ok(downloadExists(5555550), "Year old download should still be present"); + ok(!(yield downloadExists(publicList, "fakefile-4-hour-10-minutes")), "4 hour 10 minute download should now be deleted"); + ok((yield downloadExists(publicList, "fakefile-old")), "Year old download should still be present"); if (minutesSinceMidnight > 250) - ok(downloadExists(5555554), "'Today' download should still be present"); + ok((yield downloadExists(publicList, "fakefile-today")), "'Today' download should still be present"); + + // The 'Today' download might have been already deleted, in which case we + // should not wait for a download removal notification. + if (minutesSinceMidnight > 250) { + downloadPromise = promiseDownloadRemoved(publicList); + } else { + downloadPromise = Promise.resolve(); + } // Clear Today Sanitizer.prefs.setIntPref("timeSpan", 4); s.sanitize(); yield promiseFormHistoryRemoved(); + yield downloadPromise; // Be careful. If we add our objectss just before midnight, and sanitize // runs immediately after, they won't be expired. This is expected, but // we should not test in that case. We cannot just test for opposite // condition because we could cross midnight just one moment after we // cache our time, then we would have an even worse random failure. - var today = isToday(new Date(now_uSec/1000)); + var today = isToday(new Date(now_mSec)); if (today) { ok(!(yield promiseIsURIVisited(makeURI("http://today.com"))), "Pretend visit to today.com should now be deleted"); yield countEntries("today", "today form entry should be deleted", checkZero); - ok(!downloadExists(5555554), "'Today' download should now be deleted"); + ok(!(yield downloadExists(publicList, "fakefile-today")), "'Today' download should now be deleted"); } ok((yield promiseIsURIVisited(makeURI("http://before-today.com"))), "Pretend visit to before-today.com should still exist"); yield countEntries("b4today", "b4today form entry should still exist", checkOne); - ok(downloadExists(5555550), "Year old download should still be present"); + ok((yield downloadExists(publicList, "fakefile-old")), "Year old download should still be present"); + + downloadPromise = promiseDownloadRemoved(publicList); // Choose everything Sanitizer.prefs.setIntPref("timeSpan", 0); s.sanitize(); yield promiseFormHistoryRemoved(); + yield downloadPromise; ok(!(yield promiseIsURIVisited(makeURI("http://before-today.com"))), "Pretend visit to before-today.com should now be deleted"); yield countEntries("b4today", "b4today form entry should be deleted", checkZero); - ok(!downloadExists(5555550), "Year old download should now be deleted"); + ok(!(yield downloadExists(publicList, "fakefile-old")), "Year old download should now be deleted"); } function setupHistory() { @@ -562,227 +612,103 @@ function setupFormHistory() { function setupDownloads() { - // Add 10-minutes download to DB - let data = { - id: "5555555", - name: "fakefile-10-minutes", - source: "https://bugzilla.mozilla.org/show_bug.cgi?id=480169", - target: "fakefile-10-minutes", - startTime: now_uSec - 10 * kUsecPerMin, // 10 minutes ago, in uSec - endTime: now_uSec - 11 * kUsecPerMin, // 1 minute later - state: Ci.nsIDownloadManager.DOWNLOAD_FINISHED, - currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0, - guid: "a1bcD23eF4g5" - }; + let publicList = yield Downloads.getPublicDownloadList(); - let db = dm.DBConnection; - let stmt = db.createStatement( - "INSERT INTO moz_downloads (id, name, source, target, startTime, endTime, " + - "state, currBytes, maxBytes, preferredAction, autoResume, guid) " + - "VALUES (:id, :name, :source, :target, :startTime, :endTime, :state, " + - ":currBytes, :maxBytes, :preferredAction, :autoResume, :guid)"); - try { - for (let prop in data) - stmt.params[prop] = data[prop]; - stmt.execute(); - } - finally { - stmt.reset(); - } - - // Add within-1-hour download to DB - data = { - id: "5555551", - name: "fakefile-1-hour", + let download = yield Downloads.createDownload({ + source: "https://bugzilla.mozilla.org/show_bug.cgi?id=480169", + target: "fakefile-10-minutes" + }); + download.startTime = new Date(now_mSec - 10 * kMsecPerMin), // 10 minutes ago + download.canceled = true; + publicList.add(download); + + download = yield Downloads.createDownload({ source: "https://bugzilla.mozilla.org/show_bug.cgi?id=453440", - target: "fakefile-1-hour", - startTime: now_uSec - 45 * kUsecPerMin, // 45 minutes ago, in uSec - endTime: now_uSec - 44 * kUsecPerMin, // 1 minute later - state: Ci.nsIDownloadManager.DOWNLOAD_FINISHED, - currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0, - guid: "1bcD23eF4g5a" - }; + target: "fakefile-1-hour" + }); + download.startTime = new Date(now_mSec - 45 * kMsecPerMin), // 45 minutes ago + download.canceled = true; + publicList.add(download); - try { - for (let prop in data) - stmt.params[prop] = data[prop]; - stmt.execute(); - } - finally { - stmt.reset(); - } - - // Add 1-hour-10-minutes download to DB - data = { - id: "5555556", - name: "fakefile-1-hour-10-minutes", + download = yield Downloads.createDownload({ source: "https://bugzilla.mozilla.org/show_bug.cgi?id=480169", - target: "fakefile-1-hour-10-minutes", - startTime: now_uSec - 70 * kUsecPerMin, // 70 minutes ago, in uSec - endTime: now_uSec - 71 * kUsecPerMin, // 1 minute later - state: Ci.nsIDownloadManager.DOWNLOAD_FINISHED, - currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0, - guid: "a1cbD23e4Fg5" - }; + target: "fakefile-1-hour-10-minutes" + }); + download.startTime = new Date(now_mSec - 70 * kMsecPerMin), // 70 minutes ago + download.canceled = true; + publicList.add(download); - try { - for (let prop in data) - stmt.params[prop] = data[prop]; - stmt.execute(); - } - finally { - stmt.reset(); - } - - // Add within-2-hour download - data = { - id: "5555552", - name: "fakefile-2-hour", + download = yield Downloads.createDownload({ source: "https://bugzilla.mozilla.org/show_bug.cgi?id=453440", - target: "fakefile-2-hour", - startTime: now_uSec - 90 * kUsecPerMin, // 90 minutes ago, in uSec - endTime: now_uSec - 89 * kUsecPerMin, // 1 minute later - state: Ci.nsIDownloadManager.DOWNLOAD_FINISHED, - currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0, - guid: "b1aDc23eFg54" - }; + target: "fakefile-2-hour" + }); + download.startTime = new Date(now_mSec - 90 * kMsecPerMin), // 90 minutes ago + download.canceled = true; + publicList.add(download); - try { - for (let prop in data) - stmt.params[prop] = data[prop]; - stmt.execute(); - } - finally { - stmt.reset(); - } - - // Add 2-hour-10-minutes download - data = { - id: "5555557", - name: "fakefile-2-hour-10-minutes", + download = yield Downloads.createDownload({ source: "https://bugzilla.mozilla.org/show_bug.cgi?id=480169", - target: "fakefile-2-hour-10-minutes", - startTime: now_uSec - 130 * kUsecPerMin, // 130 minutes ago, in uSec - endTime: now_uSec - 131 * kUsecPerMin, // 1 minute later - state: Ci.nsIDownloadManager.DOWNLOAD_FINISHED, - currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0, - guid: "z1bcD23eF4g5" - }; + target: "fakefile-2-hour-10-minutes" + }); + download.startTime = new Date(now_mSec - 130 * kMsecPerMin), // 130 minutes ago + download.canceled = true; + publicList.add(download); - try { - for (let prop in data) - stmt.params[prop] = data[prop]; - stmt.execute(); - } - finally { - stmt.reset(); - } - - // Add within-4-hour download - data = { - id: "5555553", - name: "fakefile-4-hour", + download = yield Downloads.createDownload({ source: "https://bugzilla.mozilla.org/show_bug.cgi?id=453440", - target: "fakefile-4-hour", - startTime: now_uSec - 180 * kUsecPerMin, // 180 minutes ago, in uSec - endTime: now_uSec - 179 * kUsecPerMin, // 1 minute later - state: Ci.nsIDownloadManager.DOWNLOAD_FINISHED, - currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0, - guid: "zzzcD23eF4g5" - }; - - try { - for (let prop in data) - stmt.params[prop] = data[prop]; - stmt.execute(); - } - finally { - stmt.reset(); - } - - // Add 4-hour-10-minutes download - data = { - id: "5555558", - name: "fakefile-4-hour-10-minutes", + target: "fakefile-4-hour" + }); + download.startTime = new Date(now_mSec - 180 * kMsecPerMin), // 180 minutes ago + download.canceled = true; + publicList.add(download); + + download = yield Downloads.createDownload({ source: "https://bugzilla.mozilla.org/show_bug.cgi?id=480169", - target: "fakefile-4-hour-10-minutes", - startTime: now_uSec - 250 * kUsecPerMin, // 250 minutes ago, in uSec - endTime: now_uSec - 251 * kUsecPerMin, // 1 minute later - state: Ci.nsIDownloadManager.DOWNLOAD_FINISHED, - currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0, - guid: "z1bzz23eF4gz" - }; - - try { - for (let prop in data) - stmt.params[prop] = data[prop]; - stmt.execute(); - } - finally { - stmt.reset(); - } + target: "fakefile-4-hour-10-minutes" + }); + download.startTime = new Date(now_mSec - 250 * kMsecPerMin), // 250 minutes ago + download.canceled = true; + publicList.add(download); // Add "today" download let today = new Date(); today.setHours(0); today.setMinutes(0); today.setSeconds(1); - - data = { - id: "5555554", - name: "fakefile-today", + + download = yield Downloads.createDownload({ source: "https://bugzilla.mozilla.org/show_bug.cgi?id=453440", - target: "fakefile-today", - startTime: today.getTime() * 1000, // 12:00:30am this morning, in uSec - endTime: (today.getTime() + 1000) * 1000, // 1 second later - state: Ci.nsIDownloadManager.DOWNLOAD_FINISHED, - currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0, - guid: "ffffD23eF4g5" - }; - - try { - for (let prop in data) - stmt.params[prop] = data[prop]; - stmt.execute(); - } - finally { - stmt.reset(); - } + target: "fakefile-today" + }); + download.startTime = today, // 12:00:01 AM this morning + download.canceled = true; + publicList.add(download); // Add "before today" download let lastYear = new Date(); lastYear.setFullYear(lastYear.getFullYear() - 1); - data = { - id: "5555550", - name: "fakefile-old", + + download = yield Downloads.createDownload({ source: "https://bugzilla.mozilla.org/show_bug.cgi?id=453440", - target: "fakefile-old", - startTime: lastYear.getTime() * 1000, // 1 year ago, in uSec - endTime: (lastYear.getTime() + 1000) * 1000, // 1 second later - state: Ci.nsIDownloadManager.DOWNLOAD_FINISHED, - currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0, - guid: "ggggg23eF4g5" - }; - - try { - for (let prop in data) - stmt.params[prop] = data[prop]; - stmt.execute(); - } - finally { - stmt.finalize(); - } + target: "fakefile-old" + }); + download.startTime = lastYear, + download.canceled = true; + publicList.add(download); // Confirm everything worked - ok(downloadExists(5555550), "Pretend download for everything case should exist"); - ok(downloadExists(5555555), "Pretend download for 10-minutes case should exist"); - ok(downloadExists(5555551), "Pretend download for 1-hour case should exist"); - ok(downloadExists(5555556), "Pretend download for 1-hour-10-minutes case should exist"); - ok(downloadExists(5555552), "Pretend download for 2-hour case should exist"); - ok(downloadExists(5555557), "Pretend download for 2-hour-10-minutes case should exist"); - ok(downloadExists(5555553), "Pretend download for 4-hour case should exist"); - ok(downloadExists(5555558), "Pretend download for 4-hour-10-minutes case should exist"); - ok(downloadExists(5555554), "Pretend download for Today case should exist"); + let downloads = yield publicList.getAll(); + is(downloads.length, 9, "9 Pretend downloads added"); + + ok((yield downloadExists(publicList, "fakefile-old")), "Pretend download for everything case should exist"); + ok((yield downloadExists(publicList, "fakefile-10-minutes")), "Pretend download for 10-minutes case should exist"); + ok((yield downloadExists(publicList, "fakefile-1-hour")), "Pretend download for 1-hour case should exist"); + ok((yield downloadExists(publicList, "fakefile-1-hour-10-minutes")), "Pretend download for 1-hour-10-minutes case should exist"); + ok((yield downloadExists(publicList, "fakefile-2-hour")), "Pretend download for 2-hour case should exist"); + ok((yield downloadExists(publicList, "fakefile-2-hour-10-minutes")), "Pretend download for 2-hour-10-minutes case should exist"); + ok((yield downloadExists(publicList, "fakefile-4-hour")), "Pretend download for 4-hour case should exist"); + ok((yield downloadExists(publicList, "fakefile-4-hour-10-minutes")), "Pretend download for 4-hour-10-minutes case should exist"); + ok((yield downloadExists(publicList, "fakefile-today")), "Pretend download for Today case should exist"); } /** @@ -791,18 +717,12 @@ function setupDownloads() { * @param aID * The ids of the downloads to check. */ -function downloadExists(aID) +function downloadExists(list, path) { - let db = dm.DBConnection; - let stmt = db.createStatement( - "SELECT * " + - "FROM moz_downloads " + - "WHERE id = :id" - ); - stmt.params.id = aID; - var rows = stmt.executeStep(); - stmt.finalize(); - return rows; + return Task.spawn(function() { + let listArray = yield list.getAll(); + throw new Task.Result(listArray.some(i => i.target.path == path)); + }); } function isToday(aDate) { diff --git a/browser/base/content/test/browser_sanitizeDialog.js b/browser/base/content/test/browser_sanitizeDialog.js index 8f87b61a114..7f4e75d317a 100644 --- a/browser/base/content/test/browser_sanitizeDialog.js +++ b/browser/base/content/test/browser_sanitizeDialog.js @@ -21,18 +21,18 @@ Components.utils.import("resource://gre/modules/XPCOMUtils.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "FormHistory", "resource://gre/modules/FormHistory.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "Downloads", + "resource://gre/modules/Downloads.jsm"); let tempScope = {}; Cc["@mozilla.org/moz/jssubscript-loader;1"].getService(Ci.mozIJSSubScriptLoader) .loadSubScript("chrome://browser/content/sanitize.js", tempScope); let Sanitizer = tempScope.Sanitizer; -const dm = Cc["@mozilla.org/download-manager;1"]. - getService(Ci.nsIDownloadManager); - +const kMsecPerMin = 60 * 1000; const kUsecPerMin = 60 * 1000000; -let formEntries; +let formEntries, downloadIDs, olderDownloadIDs; // Add tests here. Each is a function that's called by doNextTest(). var gAllTests = [ @@ -92,6 +92,23 @@ var gAllTests = [ }); }, + function () { + // Add downloads (within the past hour). + Task.spawn(function () { + downloadIDs = []; + for (let i = 0; i < 5; i++) { + yield addDownloadWithMinutesAgo(downloadIDs, i); + } + // Add downloads (over an hour ago). + olderDownloadIDs = []; + for (let i = 0; i < 5; i++) { + yield addDownloadWithMinutesAgo(olderDownloadIDs, 61 + i); + } + + doNextTest(); + }).then(null, Components.utils.reportError); + }, + /** * Ensures that the combined history-downloads checkbox clears both history * visits and downloads when checked; the dialog respects simple timespan. @@ -115,16 +132,6 @@ var gAllTests = [ } addVisits(places, function() { - // Add downloads (within the past hour). - let downloadIDs = []; - for (let i = 0; i < 5; i++) { - downloadIDs.push(addDownloadWithMinutesAgo(i)); - } - // Add downloads (over an hour ago). - let olderDownloadIDs = []; - for (let i = 0; i < 5; i++) { - olderDownloadIDs.push(addDownloadWithMinutesAgo(61 + i)); - } let totalHistoryVisits = uris.length + olderURIs.length; let wh = new WindowHelper(); @@ -146,16 +153,16 @@ var gAllTests = [ wh.onunload = function () { // History visits and downloads within one hour should be cleared. yield promiseHistoryClearedState(uris, true); - ensureDownloadsClearedState(downloadIDs, true); + yield ensureDownloadsClearedState(downloadIDs, true); // Visits and downloads > 1 hour should still exist. yield promiseHistoryClearedState(olderURIs, false); - ensureDownloadsClearedState(olderDownloadIDs, false); + yield ensureDownloadsClearedState(olderDownloadIDs, false); // OK, done, cleanup after ourselves. yield blankSlate(); yield promiseHistoryClearedState(olderURIs, true); - ensureDownloadsClearedState(olderDownloadIDs, true); + yield ensureDownloadsClearedState(olderDownloadIDs, true); }; wh.open(); }); @@ -178,6 +185,18 @@ var gAllTests = [ iter.next(); }, + function () { + // Add downloads (within the past hour). + Task.spawn(function () { + downloadIDs = []; + for (let i = 0; i < 5; i++) { + yield addDownloadWithMinutesAgo(downloadIDs, i); + } + + doNextTest(); + }).then(null, Components.utils.reportError); + }, + /** * Ensures that the combined history-downloads checkbox removes neither * history visits nor downloads when not checked. @@ -194,11 +213,6 @@ var gAllTests = [ } addVisits(places, function() { - let downloadIDs = []; - for (let i = 0; i < 5; i++) { - downloadIDs.push(addDownloadWithMinutesAgo(i)); - } - let wh = new WindowHelper(); wh.onload = function () { is(this.isWarningPanelVisible(), false, @@ -224,7 +238,7 @@ var gAllTests = [ wh.onunload = function () { // Of the three only form entries should be cleared. yield promiseHistoryClearedState(uris, false); - ensureDownloadsClearedState(downloadIDs, false); + yield ensureDownloadsClearedState(downloadIDs, false); formEntries.forEach(function (entry) { let exists = yield formNameExists(entry); @@ -234,7 +248,7 @@ var gAllTests = [ // OK, done, cleanup after ourselves. yield blankSlate(); yield promiseHistoryClearedState(uris, true); - ensureDownloadsClearedState(downloadIDs, true); + yield ensureDownloadsClearedState(downloadIDs, true); }; wh.open(); }); @@ -639,15 +653,12 @@ var gAllTests = [ } ]; -// Used as the download database ID for a new download. Incremented for each -// new download. See addDownloadWithMinutesAgo(). -var gDownloadId = 5555551; - // Index in gAllTests of the test currently being run. Incremented for each // test run. See doNextTest(). var gCurrTest = 0; -var now_uSec = Date.now() * 1000; +let now_mSec = Date.now(); +let now_uSec = now_mSec * 1000; /////////////////////////////////////////////////////////////////////////////// @@ -847,7 +858,7 @@ WindowHelper.prototype = { if (wh.onunload) { Task.spawn(wh.onunload).then(function() { waitForAsyncUpdates(doNextTest); - }); + }).then(null, Components.utils.reportError); } else { waitForAsyncUpdates(doNextTest); } @@ -900,40 +911,23 @@ WindowHelper.prototype = { * @param aMinutesAgo * The download will be downloaded this many minutes ago */ -function addDownloadWithMinutesAgo(aMinutesAgo) { +function addDownloadWithMinutesAgo(aExpectedPathList, aMinutesAgo) { + let publicList = yield Downloads.getPublicDownloadList(); + let name = "fakefile-" + aMinutesAgo + "-minutes-ago"; - let data = { - id: gDownloadId, - name: name, - source: "https://bugzilla.mozilla.org/show_bug.cgi?id=480169", - target: name, - startTime: now_uSec - (aMinutesAgo * kUsecPerMin), - endTime: now_uSec - ((aMinutesAgo + 1) * kUsecPerMin), - state: Ci.nsIDownloadManager.DOWNLOAD_FINISHED, - currBytes: 0, maxBytes: -1, preferredAction: 0, autoResume: 0 - }; + let download = yield Downloads.createDownload({ + source: "https://bugzilla.mozilla.org/show_bug.cgi?id=480169", + target: name + }); + download.startTime = new Date(now_mSec - (aMinutesAgo * kMsecPerMin)); + download.canceled = true; + publicList.add(download); - let db = dm.DBConnection; - let stmt = db.createStatement( - "INSERT INTO moz_downloads (id, name, source, target, startTime, endTime, " + - "state, currBytes, maxBytes, preferredAction, autoResume) " + - "VALUES (:id, :name, :source, :target, :startTime, :endTime, :state, " + - ":currBytes, :maxBytes, :preferredAction, :autoResume)"); - try { - for (let prop in data) { - stmt.params[prop] = data[prop]; - } - stmt.execute(); - } - finally { - stmt.reset(); - } - - is(downloadExists(gDownloadId), true, - "Sanity check: download " + gDownloadId + + ok((yield downloadExists(name)), + "Sanity check: download " + name + " should exist after creating it"); - return gDownloadId++; + aExpectedPathList.push(name); } /** @@ -984,15 +978,37 @@ function formNameExists(name) */ function blankSlate() { PlacesUtils.bhistory.removeAllPages(); - dm.cleanUp(); + // The promise is resolved only when removing both downloads and form history are done. let deferred = Promise.defer(); + let formHistoryDone = false, downloadsDone = false; + + Task.spawn(function deleteAllDownloads() { + let publicList = yield Downloads.getPublicDownloadList(); + let downloads = yield publicList.getAll(); + for (let download of downloads) { + publicList.remove(download); + yield download.finalize(true); + } + downloadsDone = true; + if (formHistoryDone) { + deferred.resolve(); + } + }).then(null, Components.utils.reportError); + FormHistory.update({ op: "remove" }, { handleError: function (error) { do_throw("Error occurred updating form history: " + error); deferred.reject(error); }, - handleCompletion: function (reason) { if (!reason) deferred.resolve(); } + handleCompletion: function (reason) { + if (!reason) { + formHistoryDone = true; + if (downloadsDone) { + deferred.resolve(); + } + } + } }); return deferred.promise; } @@ -1012,24 +1028,19 @@ function boolPrefIs(aPrefName, aExpectedVal, aMsg) { } /** - * Checks to see if the download with the specified ID exists. + * Checks to see if the download with the specified path exists. * - * @param aID - * The ID of the download to check + * @param aPath + * The path of the download to check * @return True if the download exists, false otherwise */ -function downloadExists(aID) +function downloadExists(aPath) { - let db = dm.DBConnection; - let stmt = db.createStatement( - "SELECT * " + - "FROM moz_downloads " + - "WHERE id = :id" - ); - stmt.params.id = aID; - let rows = stmt.executeStep(); - stmt.finalize(); - return !!rows; + return Task.spawn(function() { + let publicList = yield Downloads.getPublicDownloadList(); + let listArray = yield publicList.getAll(); + throw new Task.Result(listArray.some(i => i.target.path == aPath)); + }); } /** @@ -1059,7 +1070,7 @@ function doNextTest() { function ensureDownloadsClearedState(aDownloadIDs, aShouldBeCleared) { let niceStr = aShouldBeCleared ? "no longer" : "still"; aDownloadIDs.forEach(function (id) { - is(downloadExists(id), !aShouldBeCleared, + is((yield downloadExists(id)), !aShouldBeCleared, "download " + id + " should " + niceStr + " exist"); }); } From 7c7590414936a089cc6902ff6c47545bd4d66b84 Mon Sep 17 00:00:00 2001 From: Paolo Amadini Date: Wed, 21 Aug 2013 09:36:07 +0200 Subject: [PATCH 02/13] Bug 847863 - Part 7 of 8 - Convert Downloads Panel tests. r=enn --- .../browser/browser_basic_functionality.js | 48 ++- .../browser/browser_first_download_panel.js | 53 ++-- .../components/downloads/test/browser/head.js | 286 ++++-------------- 3 files changed, 107 insertions(+), 280 deletions(-) diff --git a/browser/components/downloads/test/browser/browser_basic_functionality.js b/browser/components/downloads/test/browser/browser_basic_functionality.js index b1a8f6595e5..1df28aa7c7f 100644 --- a/browser/components/downloads/test/browser/browser_basic_functionality.js +++ b/browser/components/downloads/test/browser/browser_basic_functionality.js @@ -7,39 +7,33 @@ * Make sure the downloads panel can display items in the right order and * contains the expected data. */ -function gen_test() +function test_task() { // Display one of each download state. const DownloadData = [ - { endTime: 1180493839859239, state: nsIDM.DOWNLOAD_NOTSTARTED }, - { endTime: 1180493839859238, state: nsIDM.DOWNLOAD_DOWNLOADING }, - { endTime: 1180493839859237, state: nsIDM.DOWNLOAD_PAUSED }, - { endTime: 1180493839859236, state: nsIDM.DOWNLOAD_SCANNING }, - { endTime: 1180493839859235, state: nsIDM.DOWNLOAD_QUEUED }, - { endTime: 1180493839859234, state: nsIDM.DOWNLOAD_FINISHED }, - { endTime: 1180493839859233, state: nsIDM.DOWNLOAD_FAILED }, - { endTime: 1180493839859232, state: nsIDM.DOWNLOAD_CANCELED }, - { endTime: 1180493839859231, state: nsIDM.DOWNLOAD_BLOCKED_PARENTAL }, - { endTime: 1180493839859230, state: nsIDM.DOWNLOAD_DIRTY }, - { endTime: 1180493839859229, state: nsIDM.DOWNLOAD_BLOCKED_POLICY }, + { state: nsIDM.DOWNLOAD_NOTSTARTED }, + { state: nsIDM.DOWNLOAD_PAUSED }, + { state: nsIDM.DOWNLOAD_FINISHED }, + { state: nsIDM.DOWNLOAD_FAILED }, + { state: nsIDM.DOWNLOAD_CANCELED }, ]; - // For testing purposes, show all the download items at once. - var originalCountLimit = DownloadsView.kItemCountLimit; - DownloadsView.kItemCountLimit = DownloadData.length; - registerCleanupFunction(function () { - DownloadsView.kItemCountLimit = originalCountLimit; - }); - try { // Ensure that state is reset in case previous tests didn't finish. - for (let yy in gen_resetState(DownloadsCommon.getData(window))) yield undefined; + yield task_resetState(); + + // For testing purposes, show all the download items at once. + var originalCountLimit = DownloadsView.kItemCountLimit; + DownloadsView.kItemCountLimit = DownloadData.length; + registerCleanupFunction(function () { + DownloadsView.kItemCountLimit = originalCountLimit; + }); // Populate the downloads database with the data required by this test. - for (let yy in gen_addDownloadRows(DownloadData)) yield undefined; + yield task_addDownloads(DownloadData); // Open the user interface and wait for data to be fully loaded. - for (let yy in gen_openPanel(DownloadsCommon.getData(window))) yield undefined; + yield task_openPanel(); // Test item data and count. This also tests the ordering of the display. let richlistbox = document.getElementById("downloadsListBox"); @@ -47,16 +41,14 @@ function gen_test() is(richlistbox.children.length, DownloadData.length, "There is the correct number of richlistitems"); */ - for (let i = 0; i < richlistbox.children.length; i++) { - let element = richlistbox.children[i]; + let itemCount = richlistbox.children.length; + for (let i = 0; i < itemCount; i++) { + let element = richlistbox.children[itemCount - i - 1]; let dataItem = new DownloadsViewItemController(element).dataItem; - is(dataItem.target, DownloadData[i].name, "Download names match up"); is(dataItem.state, DownloadData[i].state, "Download states match up"); - is(dataItem.file, DownloadData[i].target, "Download targets match up"); - is(dataItem.uri, DownloadData[i].source, "Download sources match up"); } } finally { // Clean up when the test finishes. - for (let yy in gen_resetState(DownloadsCommon.getData(window))) yield undefined; + yield task_resetState(); } } diff --git a/browser/components/downloads/test/browser/browser_first_download_panel.js b/browser/components/downloads/test/browser/browser_first_download_panel.js index ed1885ba3f7..1faeb6c7e33 100644 --- a/browser/components/downloads/test/browser/browser_first_download_panel.js +++ b/browser/components/downloads/test/browser/browser_first_download_panel.js @@ -8,19 +8,19 @@ * download it notices. All subsequent downloads, even across sessions, should * not open the panel automatically. */ -function gen_test() +function test_task() { try { // Ensure that state is reset in case previous tests didn't finish. - for (let yy in gen_resetState(DownloadsCommon.getData(window))) yield undefined; + yield task_resetState(); - // With this set to false, we should automatically open the panel - // the first time a download is started. + // With this set to false, we should automatically open the panel the first + // time a download is started. DownloadsCommon.getData(window).panelHasShownBefore = false; - prepareForPanelOpen(); + let promise = promisePanelOpened(); DownloadsCommon.getData(window)._notifyDownloadEvent("start"); - yield undefined; + yield promise; // If we got here, that means the panel opened. DownloadsPanel.hidePanel(); @@ -28,29 +28,26 @@ function gen_test() ok(DownloadsCommon.getData(window).panelHasShownBefore, "Should have recorded that the panel was opened on a download.") - // Next, make sure that if we start another download, we don't open - // the panel automatically. - panelShouldNotOpen(); - DownloadsCommon.getData(window)._notifyDownloadEvent("start"); - yield waitFor(2); - } catch(e) { - ok(false, e); + // Next, make sure that if we start another download, we don't open the + // panel automatically. + let originalOnPopupShown = DownloadsPanel.onPopupShown; + DownloadsPanel.onPopupShown = function () { + originalOnPopupShown.apply(this, arguments); + ok(false, "Should not have opened the downloads panel."); + }; + + try { + DownloadsCommon.getData(window)._notifyDownloadEvent("start"); + + // Wait 2 seconds to ensure that the panel does not open. + let deferTimeout = Promise.defer(); + setTimeout(deferTimeout.resolve, 2000); + yield deferTimeout.promise; + } finally { + DownloadsPanel.onPopupShown = originalOnPopupShown; + } } finally { // Clean up when the test finishes. - for (let yy in gen_resetState(DownloadsCommon.getData(window))) yield undefined; + yield task_resetState(); } } - -/** - * Call this to record a test failure for the next time the downloads panel - * opens. - */ -function panelShouldNotOpen() -{ - // Hook to wait until the test data has been loaded. - let originalOnViewLoadCompleted = DownloadsPanel.onViewLoadCompleted; - DownloadsPanel.onViewLoadCompleted = function () { - DownloadsPanel.onViewLoadCompleted = originalOnViewLoadCompleted; - ok(false, "Should not have opened the downloads panel."); - }; -} diff --git a/browser/components/downloads/test/browser/head.js b/browser/components/downloads/test/browser/head.js index 7e4d857c818..35526334ca1 100644 --- a/browser/components/downloads/test/browser/head.js +++ b/browser/components/downloads/test/browser/head.js @@ -10,10 +10,16 @@ //////////////////////////////////////////////////////////////////////////////// //// Globals -XPCOMUtils.defineLazyModuleGetter(this, "FileUtils", - "resource://gre/modules/FileUtils.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "Downloads", + "resource://gre/modules/Downloads.jsm"); XPCOMUtils.defineLazyModuleGetter(this, "DownloadsCommon", "resource:///modules/DownloadsCommon.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "FileUtils", + "resource://gre/modules/FileUtils.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "Promise", + "resource://gre/modules/Promise.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, "Task", + "resource://gre/modules/Task.jsm"); const nsIDM = Ci.nsIDownloadManager; let gTestTargetFile = FileUtils.getFile("TmpD", ["dm-ui-test.file"]); @@ -22,253 +28,85 @@ registerCleanupFunction(function () { gTestTargetFile.remove(false); }); -/** - * This objects contains a property for each column in the downloads table. - */ -let gDownloadRowTemplate = { - name: "test-download.txt", - source: "http://www.example.com/test-download.txt", - target: NetUtil.newURI(gTestTargetFile).spec, - startTime: 1180493839859230, - endTime: 1180493839859234, - state: nsIDM.DOWNLOAD_FINISHED, - currBytes: 0, - maxBytes: -1, - preferredAction: 0, - autoResume: 0 -}; - //////////////////////////////////////////////////////////////////////////////// //// Infrastructure -// All test are run through the test runner. function test() { - testRunner.runTest(this.gen_test); + waitForExplicitFinish(); + Task.spawn(test_task).then(null, ex => ok(false, ex)).then(finish); } -/** - * Runs a browser-chrome test defined through a generator function. - * - * This object is a singleton, initialized automatically when this script is - * included. Every browser-chrome test file includes a new copy of this object. - */ -var testRunner = { - _testIterator: null, - _lastEventResult: undefined, - _testRunning: false, - _eventRaised: false, - - // --- Main test runner --- - - /** - * Runs the test described by the provided generator function asynchronously. - * - * Calling yield in the generator will cause it to wait until continueTest is - * called. The parameter provided to continueTest will be the return value of - * the yield operator. - * - * @param aGenerator - * Test generator function. The function will be called with no - * arguments to retrieve its iterator. - */ - runTest: function TR_runTest(aGenerator) { - waitForExplicitFinish(); - testRunner._testIterator = aGenerator(); - testRunner.continueTest(); - }, - - /** - * Continues the currently running test. - * - * @param aEventResult - * This will be the return value of the yield operator in the test. - */ - continueTest: function TR_continueTest(aEventResult) { - // Store the last event result, or set it to undefined. - testRunner._lastEventResult = aEventResult; - - // Never reenter the main loop, but notify that the event has been raised. - if (testRunner._testRunning) { - testRunner._eventRaised = true; - return; - } - - // Enter the main iteration loop. - testRunner._testRunning = true; - try { - do { - // Call the iterator, but don't leave the loop if the expected event is - // raised during the execution of the generator. - testRunner._eventRaised = false; - testRunner._testIterator.send(testRunner._lastEventResult); - } while (testRunner._eventRaised); - } - catch (e) { - // This block catches exceptions raised by the generator, including the - // normal StopIteration exception. Unexpected exceptions are reported as - // test failures. - if (!(e instanceof StopIteration)) - ok(false, e); - // In any case, stop the tests in this file. - finish(); - } - - // Wait for the next event or finish. - testRunner._testRunning = false; - } -}; - //////////////////////////////////////////////////////////////////////////////// -//// Asynchronous generator-based support subroutines +//// Asynchronous support subroutines -// -// The following functions are all generators that can be used inside the main -// test generator to perform specific tasks asynchronously. To invoke these -// subroutines correctly, an iteration syntax should be used: -// -// for (let yy in gen_example("Parameter")) yield undefined; -// - -function gen_resetState(aData) +function promiseFocus() { - let statement = Services.downloads.DBConnection.createAsyncStatement( - "DELETE FROM moz_downloads"); - try { - statement.executeAsync({ - handleResult: function(aResultSet) { }, - handleError: function(aError) - { - Cu.reportError(aError); - }, - handleCompletion: function(aReason) - { - testRunner.continueTest(); - } - }); - yield undefined; - } finally { - statement.finalize(); + let deferred = Promise.defer(); + waitForFocus(deferred.resolve); + return deferred.promise; +} + +function promisePanelOpened() +{ + let deferred = Promise.defer(); + + // Hook to wait until the panel is shown. + let originalOnPopupShown = DownloadsPanel.onPopupShown; + DownloadsPanel.onPopupShown = function () { + DownloadsPanel.onPopupShown = originalOnPopupShown; + originalOnPopupShown.apply(this, arguments); + + // Defer to the next tick of the event loop so that we don't continue + // processing during the DOM event handler itself. + setTimeout(deferred.resolve, 0); + }; + + return deferred.promise; +} + +function task_resetState() +{ + // Remove all downloads. + let publicList = yield Downloads.getPublicDownloadList(); + let downloads = yield publicList.getAll(); + for (let download of downloads) { + publicList.remove(download); + yield download.finalize(true); } // Reset any prefs that might have been changed. Services.prefs.clearUserPref("browser.download.panel.shown"); - // Ensure that the panel is closed and data is unloaded. - aData.clear(); - aData._loadState = aData.kLoadNone; DownloadsPanel.hidePanel(); - // Wait for focus on the main window. - waitForFocus(testRunner.continueTest); - yield undefined; + yield promiseFocus(); } -function gen_addDownloadRows(aDataRows) +function task_addDownloads(aItems) { - let columnNames = Object.keys(gDownloadRowTemplate).join(", "); - let parameterNames = Object.keys(gDownloadRowTemplate) - .map(function(n) ":" + n) - .join(", "); - let statement = Services.downloads.DBConnection.createAsyncStatement( - "INSERT INTO moz_downloads (" + columnNames + - ", guid) VALUES(" + parameterNames + ", GENERATE_GUID())"); - try { - // Execute the statement for each of the provided downloads in reverse. - for (let i = aDataRows.length - 1; i >= 0; i--) { - let dataRow = aDataRows[i]; + let startTimeMs = Date.now(); - // Populate insert parameters from the provided data. - for (let columnName in gDownloadRowTemplate) { - if (!(columnName in dataRow)) { - // Update the provided row object with data from the global template, - // for columns whose value is not provided explicitly. - dataRow[columnName] = gDownloadRowTemplate[columnName]; - } - statement.params[columnName] = dataRow[columnName]; - } - - // Run the statement asynchronously and wait. - statement.executeAsync({ - handleResult: function(aResultSet) { }, - handleError: function(aError) - { - Cu.reportError(aError.message + " (Result = " + aError.result + ")"); - }, - handleCompletion: function(aReason) - { - testRunner.continueTest(); - } - }); - yield undefined; - - // At each iteration, ensure that the start and end time in the global - // template is distinct, as these column are used to sort each download - // in its category. - gDownloadRowTemplate.startTime++; - gDownloadRowTemplate.endTime++; - } - } finally { - statement.finalize(); + let publicList = yield Downloads.getPublicDownloadList(); + for (let item of aItems) { + publicList.add(yield Downloads.createDownload({ + source: "http://www.example.com/test-download.txt", + target: gTestTargetFile, + succeeded: item.state == nsIDM.DOWNLOAD_FINISHED, + canceled: item.state == nsIDM.DOWNLOAD_CANCELED || + item.state == nsIDM.DOWNLOAD_PAUSED, + error: item.state == nsIDM.DOWNLOAD_FAILED ? new Error("Failed.") : null, + hasPartialData: item.state == nsIDM.DOWNLOAD_PAUSED, + startTime: new Date(startTimeMs++), + })); } } -function gen_openPanel(aData) +function task_openPanel() { - // Hook to wait until the test data has been loaded. - let originalOnViewLoadCompleted = DownloadsPanel.onViewLoadCompleted; - DownloadsPanel.onViewLoadCompleted = function () { - DownloadsPanel.onViewLoadCompleted = originalOnViewLoadCompleted; - originalOnViewLoadCompleted.apply(this); - testRunner.continueTest(); - }; + yield promiseFocus(); - // Start loading all the downloads from the database asynchronously. - aData.ensurePersistentDataLoaded(false); - - // Wait for focus on the main window. - waitForFocus(testRunner.continueTest); - yield undefined; - - // Open the downloads panel, waiting until loading is completed. + let promise = promisePanelOpened(); DownloadsPanel.showPanel(); - yield undefined; -} - -/** - * Spin the event loop for aSeconds seconds, and then signal the test to - * continue. - * - * @param aSeconds the number of seconds to wait. - * @note This helper should _only_ be used when there's no valid event to - * listen to and one can't be made. - */ -function waitFor(aSeconds) -{ - setTimeout(function() { - testRunner.continueTest(); - }, aSeconds * 1000); -} - -/** - * Make it so that the next time the downloads panel opens, we signal to - * continue the test. This function needs to be called each time you want - * to wait for the panel to open. - * - * Example usage: - * - * prepareForPanelOpen(); - * // Do something to open the panel - * yield undefined; - * // We can assume the panel is open now. - */ -function prepareForPanelOpen() -{ - // Hook to wait until the test data has been loaded. - let originalOnPopupShown = DownloadsPanel.onPopupShown; - DownloadsPanel.onPopupShown = function (aEvent) { - DownloadsPanel.onPopupShown = originalOnPopupShown; - DownloadsPanel.onPopupShown.apply(this, [aEvent]); - testRunner.continueTest(); - }; + yield promise; } From 353706cefc0065d32a36293165b8093bfa5f66c3 Mon Sep 17 00:00:00 2001 From: Paolo Amadini Date: Wed, 21 Aug 2013 09:36:07 +0200 Subject: [PATCH 03/13] Bug 847863 - Part 8 of 8 - Use the JavaScript API instead of nsIDownloadManager as the back-end for the panel. r=enn --- browser/app/profile/firefox.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 6dadc853ef3..801ccf845f7 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -336,7 +336,7 @@ pref("browser.download.manager.scanWhenDone", true); pref("browser.download.manager.resumeOnWakeDelay", 10000); // Enables the asynchronous Downloads API in the Downloads Panel. -pref("browser.download.useJSTransfer", false); +pref("browser.download.useJSTransfer", true); // This allows disabling the Downloads Panel in favor of the old interface. pref("browser.download.useToolkitUI", false); From cc5581e873098a2c211fc10d8b7d75d99039e042 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Wed, 21 Aug 2013 08:32:47 -0400 Subject: [PATCH 04/13] Bug 901126 - Split browser_newtab_drag_drop.js into two tests. r=ttaubert --- browser/base/content/test/newtab/Makefile.in | 1 + .../test/newtab/browser_newtab_drag_drop.js | 43 --------------- .../newtab/browser_newtab_drag_drop_ext.js | 55 +++++++++++++++++++ 3 files changed, 56 insertions(+), 43 deletions(-) create mode 100644 browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js diff --git a/browser/base/content/test/newtab/Makefile.in b/browser/base/content/test/newtab/Makefile.in index 0a61302b694..6ac460d14d3 100644 --- a/browser/base/content/test/newtab/Makefile.in +++ b/browser/base/content/test/newtab/Makefile.in @@ -14,6 +14,7 @@ MOCHITEST_BROWSER_FILES = \ browser_newtab_block.js \ browser_newtab_disable.js \ browser_newtab_drag_drop.js \ + browser_newtab_drag_drop_ext.js \ browser_newtab_drop_preview.js \ browser_newtab_focus.js \ browser_newtab_reset.js \ diff --git a/browser/base/content/test/newtab/browser_newtab_drag_drop.js b/browser/base/content/test/newtab/browser_newtab_drag_drop.js index 449032a7d7c..412969c2944 100644 --- a/browser/base/content/test/newtab/browser_newtab_drag_drop.js +++ b/browser/base/content/test/newtab/browser_newtab_drag_drop.js @@ -71,47 +71,4 @@ function runTests() { yield simulateDrop(0, 4); checkGrid("3,1p,2p,4,0p,5p,6,7,8"); - - // drag a new site onto the very first cell - yield setLinks("0,1,2,3,4,5,6,7,8"); - setPinnedLinks(",,,,,,,7,8"); - - yield addNewTabPageTab(); - checkGrid("0,1,2,3,4,5,6,7p,8p"); - - yield simulateExternalDrop(0); - checkGrid("99p,0,1,2,3,4,5,7p,8p"); - - // drag a new site onto the grid and make sure that pinned cells don't get - // pushed out - yield setLinks("0,1,2,3,4,5,6,7,8"); - setPinnedLinks(",,,,,,,7,8"); - - yield addNewTabPageTab(); - checkGrid("0,1,2,3,4,5,6,7p,8p"); - - yield simulateExternalDrop(7); - checkGrid("0,1,2,3,4,5,7p,99p,8p"); - - // drag a new site beneath a pinned cell and make sure the pinned cell is - // not moved - yield setLinks("0,1,2,3,4,5,6,7,8"); - setPinnedLinks(",,,,,,,,8"); - - yield addNewTabPageTab(); - checkGrid("0,1,2,3,4,5,6,7,8p"); - - yield simulateExternalDrop(7); - checkGrid("0,1,2,3,4,5,6,99p,8p"); - - // drag a new site onto a block of pinned sites and make sure they're shifted - // around accordingly - yield setLinks("0,1,2,3,4,5,6,7,8"); - setPinnedLinks("0,1,2,,,,,,"); - - yield addNewTabPageTab(); - checkGrid("0p,1p,2p"); - - yield simulateExternalDrop(1); - checkGrid("0p,99p,1p,2p,3,4,5,6,7"); } diff --git a/browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js b/browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js new file mode 100644 index 00000000000..1ba7cfed8be --- /dev/null +++ b/browser/base/content/test/newtab/browser_newtab_drag_drop_ext.js @@ -0,0 +1,55 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/* + * These tests make sure that dragging and dropping sites works as expected. + * Sites contained in the grid need to shift around to indicate the result + * of the drag-and-drop operation. If the grid is full and we're dragging + * a new site into it another one gets pushed out. + * This is a continuation of browser_newtab_drag_drop.js + * to decrease test run time, focusing on external sites. + */ +function runTests() { + // drag a new site onto the very first cell + yield setLinks("0,1,2,3,4,5,6,7,8"); + setPinnedLinks(",,,,,,,7,8"); + + yield addNewTabPageTab(); + checkGrid("0,1,2,3,4,5,6,7p,8p"); + + yield simulateExternalDrop(0); + checkGrid("99p,0,1,2,3,4,5,7p,8p"); + + // drag a new site onto the grid and make sure that pinned cells don't get + // pushed out + yield setLinks("0,1,2,3,4,5,6,7,8"); + setPinnedLinks(",,,,,,,7,8"); + + yield addNewTabPageTab(); + checkGrid("0,1,2,3,4,5,6,7p,8p"); + + yield simulateExternalDrop(7); + checkGrid("0,1,2,3,4,5,7p,99p,8p"); + + // drag a new site beneath a pinned cell and make sure the pinned cell is + // not moved + yield setLinks("0,1,2,3,4,5,6,7,8"); + setPinnedLinks(",,,,,,,,8"); + + yield addNewTabPageTab(); + checkGrid("0,1,2,3,4,5,6,7,8p"); + + yield simulateExternalDrop(7); + checkGrid("0,1,2,3,4,5,6,99p,8p"); + + // drag a new site onto a block of pinned sites and make sure they're shifted + // around accordingly + yield setLinks("0,1,2,3,4,5,6,7,8"); + setPinnedLinks("0,1,2,,,,,,"); + + yield addNewTabPageTab(); + checkGrid("0p,1p,2p"); + + yield simulateExternalDrop(1); + checkGrid("0p,99p,1p,2p,3,4,5,6,7"); +} \ No newline at end of file From 4a0e1e2f876bcd2d6ff862f6f5732749fd3d1971 Mon Sep 17 00:00:00 2001 From: Andrew Wang Date: Wed, 21 Aug 2013 08:33:17 -0400 Subject: [PATCH 05/13] Bug 904480 - Return values of _SessionFile.wipe() and .writeLoadStateOnceAfterStartup() are unused. r=ttaubert --- browser/components/sessionstore/src/_SessionFile.jsm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/browser/components/sessionstore/src/_SessionFile.jsm b/browser/components/sessionstore/src/_SessionFile.jsm index 0f2508faac3..74e55af0da3 100644 --- a/browser/components/sessionstore/src/_SessionFile.jsm +++ b/browser/components/sessionstore/src/_SessionFile.jsm @@ -75,7 +75,7 @@ this._SessionFile = { * state. This must only be called once, it will throw an error otherwise. */ writeLoadStateOnceAfterStartup: function (aLoadState) { - return SessionFileInternal.writeLoadStateOnceAfterStartup(aLoadState); + SessionFileInternal.writeLoadStateOnceAfterStartup(aLoadState); }, /** * Create a backup copy, asynchronously. @@ -95,7 +95,7 @@ this._SessionFile = { * Wipe the contents of the session file, asynchronously. */ wipe: function () { - return SessionFileInternal.wipe(); + SessionFileInternal.wipe(); } }; @@ -231,7 +231,7 @@ let SessionFileInternal = { }, writeLoadStateOnceAfterStartup: function (aLoadState) { - return SessionWorker.post("writeLoadStateOnceAfterStartup", [aLoadState]).then(msg => { + SessionWorker.post("writeLoadStateOnceAfterStartup", [aLoadState]).then(msg => { this._recordTelemetry(msg.telemetry); return msg; }); @@ -246,7 +246,7 @@ let SessionFileInternal = { }, wipe: function () { - return SessionWorker.post("wipe"); + SessionWorker.post("wipe"); }, _recordTelemetry: function(telemetry) { From 164140298fa8abd20ad603c88f2dd3bb74549982 Mon Sep 17 00:00:00 2001 From: Nicolas Carlo Date: Wed, 21 Aug 2013 08:33:45 -0400 Subject: [PATCH 06/13] Bug 906798 - Add periods to if statements in _stripHost function in aboutReader.js. r=margaret --- mobile/android/chrome/content/aboutReader.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/mobile/android/chrome/content/aboutReader.js b/mobile/android/chrome/content/aboutReader.js index c6d7a8c9ecf..bf3efffe771 100644 --- a/mobile/android/chrome/content/aboutReader.js +++ b/mobile/android/chrome/content/aboutReader.js @@ -587,11 +587,11 @@ AboutReader.prototype = { let start = 0; - if (host.startsWith("www")) + if (host.startsWith("www.")) start = 4; - else if (host.startsWith("m")) + else if (host.startsWith("m.")) start = 2; - else if (host.startsWith("mobile")) + else if (host.startsWith("mobile.")) start = 7; return host.substring(start); From e8dd4674471003868c8431d103ef4331d8b64629 Mon Sep 17 00:00:00 2001 From: Steven MacLeod Date: Wed, 21 Aug 2013 12:14:33 -0400 Subject: [PATCH 07/13] Bug 902280 - Update _lastSaveTime before writing to avoid deceeding the write interval. r=ttaubert --- browser/components/sessionstore/src/SessionSaver.jsm | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/browser/components/sessionstore/src/SessionSaver.jsm b/browser/components/sessionstore/src/SessionSaver.jsm index b01dac26d43..b1977d9727c 100644 --- a/browser/components/sessionstore/src/SessionSaver.jsm +++ b/browser/components/sessionstore/src/SessionSaver.jsm @@ -262,6 +262,12 @@ let SessionSaverInternal = { return; } + // We update the time stamp before writing so that we don't write again + // too soon, if saving is requested before the write completes. Without + // this update we may save repeatedly if actions cause a runDelayed + // before writing has completed. See Bug 902280 + this.updateLastSaveTime(); + // Write (atomically) to a session file, using a tmp file. Once the session // file is successfully updated, save the time stamp of the last save and // notify the observers. From f562440456487ccc8a05c81a93059bff2f256857 Mon Sep 17 00:00:00 2001 From: Shane Caraveo Date: Wed, 21 Aug 2013 11:32:46 -0700 Subject: [PATCH 08/13] bug 891216 multiple workers enabled, r=markh --- browser/base/content/aboutSocialError.xhtml | 5 +- browser/base/content/browser-social.js | 44 +++++--- browser/base/content/nsContextMenu.js | 5 +- browser/base/content/test/social/Makefile.in | 1 + .../test/social/browser_social_multiworker.js | 101 ++++++++++++++++++ browser/modules/Social.jsm | 60 ++++++----- toolkit/components/social/MozSocialAPI.jsm | 20 ++-- toolkit/components/social/SocialService.jsm | 19 +++- .../test/browser/browser_SocialProvider.js | 58 ++++++++-- .../test/xpcshell/test_SocialService.js | 42 ++++---- 10 files changed, 268 insertions(+), 87 deletions(-) create mode 100644 browser/base/content/test/social/browser_social_multiworker.js diff --git a/browser/base/content/aboutSocialError.xhtml b/browser/base/content/aboutSocialError.xhtml index 6bef2d7bdbf..8c5a856d2ea 100644 --- a/browser/base/content/aboutSocialError.xhtml +++ b/browser/base/content/aboutSocialError.xhtml @@ -112,10 +112,7 @@ } function reloadProvider() { - Social.enabled = false; - Services.tm.mainThread.dispatch(function() { - Social.enabled = true; - }, Components.interfaces.nsIThread.DISPATCH_NORMAL); + Social.provider.reload(); } parseQueryString(); diff --git a/browser/base/content/browser-social.js b/browser/base/content/browser-social.js index 514a46f1d45..f831f0a9cc2 100644 --- a/browser/base/content/browser-social.js +++ b/browser/base/content/browser-social.js @@ -36,6 +36,7 @@ SocialUI = { Services.obs.addObserver(this, "social:frameworker-error", false); Services.obs.addObserver(this, "social:provider-set", false); Services.obs.addObserver(this, "social:providers-changed", false); + Services.obs.addObserver(this, "social:provider-reload", false); Services.prefs.addObserver("social.sidebar.open", this, false); Services.prefs.addObserver("social.toast-notifications.enabled", this, false); @@ -60,6 +61,7 @@ SocialUI = { Services.obs.removeObserver(this, "social:frameworker-error"); Services.obs.removeObserver(this, "social:provider-set"); Services.obs.removeObserver(this, "social:providers-changed"); + Services.obs.removeObserver(this, "social:provider-reload"); Services.prefs.removeObserver("social.sidebar.open", this); Services.prefs.removeObserver("social.toast-notifications.enabled", this); @@ -74,6 +76,16 @@ SocialUI = { // manually :( try { switch (topic) { + case "social:provider-reload": + // if the reloaded provider is our current provider, fall through + // to social:provider-set so the ui will be reset + if (!Social.provider || Social.provider.origin != data) + return; + // be sure to unload the sidebar as it will not reload if the origin + // has not changed, it will be loaded in provider-set below. Other + // panels will be unloaded or handle reload. + SocialSidebar.unloadSidebar(); + // fall through to social:provider-set case "social:provider-set": // Social.provider has changed (possibly to null), update any state // which depends on it. @@ -142,7 +154,7 @@ SocialUI = { // Miscellaneous helpers showProfile: function SocialUI_showProfile() { - if (Social.haveLoggedInUser()) + if (Social.provider.haveLoggedInUser()) openUILinkIn(Social.provider.profile.profileURL, "tab"); else { // XXX Bug 789585 will implement an API for provider-specified login pages. @@ -976,22 +988,20 @@ SocialToolbar = { let toggleNotificationsCommand = document.getElementById("Social:ToggleNotifications"); toggleNotificationsCommand.setAttribute("hidden", !socialEnabled); - if (!Social.haveLoggedInUser() || !socialEnabled) { - let parent = document.getElementById("social-notification-panel"); - while (parent.hasChildNodes()) { - let frame = parent.firstChild; - SharedFrame.forgetGroup(frame.id); - parent.removeChild(frame); - } + let parent = document.getElementById("social-notification-panel"); + while (parent.hasChildNodes()) { + let frame = parent.firstChild; + SharedFrame.forgetGroup(frame.id); + parent.removeChild(frame); + } - let tbi = document.getElementById("social-toolbar-item"); - if (tbi) { - // SocialMark is the last button allways - let next = SocialMark.button.previousSibling; - while (next != this.button) { - tbi.removeChild(next); - next = SocialMark.button.previousSibling; - } + let tbi = document.getElementById("social-toolbar-item"); + if (tbi) { + // SocialMark is the last button allways + let next = SocialMark.button.previousSibling; + while (next != this.button) { + tbi.removeChild(next); + next = SocialMark.button.previousSibling; } } }, @@ -1035,7 +1045,7 @@ SocialToolbar = { // provider.profile == undefined means no response yet from the provider // to tell us whether the user is logged in or not. if (!SocialUI.enabled || - (!Social.haveLoggedInUser() && Social.provider.profile !== undefined)) { + (!Social.provider.haveLoggedInUser() && Social.provider.profile !== undefined)) { // Either no enabled provider, or there is a provider and it has // responded with a profile and the user isn't loggedin. The icons // etc have already been removed by updateButtonHiddenState, so we want diff --git a/browser/base/content/nsContextMenu.js b/browser/base/content/nsContextMenu.js index 2497dd4462a..5aa48f6c3f1 100644 --- a/browser/base/content/nsContextMenu.js +++ b/browser/base/content/nsContextMenu.js @@ -905,10 +905,7 @@ nsContextMenu.prototype = { reload: function(event) { if (this.onSocial) { // full reload of social provider - Social.enabled = false; - Services.tm.mainThread.dispatch(function() { - Social.enabled = true; - }, Components.interfaces.nsIThread.DISPATCH_NORMAL); + Social.provider.reload(); } else { BrowserReloadOrDuplicate(event); } diff --git a/browser/base/content/test/social/Makefile.in b/browser/base/content/test/social/Makefile.in index 37c28c1dd02..ba9be15479b 100644 --- a/browser/base/content/test/social/Makefile.in +++ b/browser/base/content/test/social/Makefile.in @@ -30,6 +30,7 @@ MOCHITEST_BROWSER_FILES = \ browser_social_chatwindow_resize.js \ browser_social_chatwindowfocus.js \ browser_social_multiprovider.js \ + browser_social_multiworker.js \ browser_social_errorPage.js \ browser_social_window.js \ social_activate.html \ diff --git a/browser/base/content/test/social/browser_social_multiworker.js b/browser/base/content/test/social/browser_social_multiworker.js new file mode 100644 index 00000000000..1306a89da30 --- /dev/null +++ b/browser/base/content/test/social/browser_social_multiworker.js @@ -0,0 +1,101 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +function test() { + waitForExplicitFinish(); + + Services.prefs.setBoolPref("social.allowMultipleWorkers", true); + runSocialTestWithProvider(gProviders, function (finishcb) { + runSocialTests(tests, undefined, undefined, function() { + Services.prefs.clearUserPref("social.allowMultipleWorkers"); + finishcb(); + }); + }); +} + +let gProviders = [ + { + name: "provider 1", + origin: "https://example.com", + sidebarURL: "https://example.com/browser/browser/base/content/test/social/social_sidebar.html?provider1", + workerURL: "https://example.com/browser/browser/base/content/test/social/social_worker.js", + iconURL: "chrome://branding/content/icon48.png" + }, + { + name: "provider 2", + origin: "https://test1.example.com", + sidebarURL: "https://test1.example.com/browser/browser/base/content/test/social/social_sidebar.html?provider2", + workerURL: "https://test1.example.com/browser/browser/base/content/test/social/social_worker.js", + iconURL: "chrome://branding/content/icon48.png" + } +]; + +var tests = { + testWorkersAlive: function(next) { + // verify we can get a message from all providers that are enabled + let messageReceived = 0; + function oneWorkerTest(provider) { + let port = provider.getWorkerPort(); + port.onmessage = function (e) { + let topic = e.data.topic; + switch (topic) { + case "test-init-done": + ok(true, "got message from provider " + provider.name); + port.close(); + messageReceived++; + break; + } + }; + port.postMessage({topic: "test-init"}); + } + + for (let p of Social.providers) { + oneWorkerTest(p); + } + + waitForCondition(function() messageReceived == Social.providers.length, + next, "received messages from all workers"); + }, + testWorkerDisabling: function(next) { + Social.enabled = false; + is(Social.providers.length, gProviders.length, "providers still available"); + for (let p of Social.providers) { + ok(!p.enabled, "provider disabled"); + ok(!p.getWorkerPort(), "worker disabled"); + } + next(); + }, + + testSingleWorkerEnabling: function(next) { + // test that only one worker is enabled when we limit workers + Services.prefs.setBoolPref("social.allowMultipleWorkers", false); + Social.enabled = true; + for (let p of Social.providers) { + if (p == Social.provider) { + ok(p.enabled, "primary provider enabled"); + let port = p.getWorkerPort(); + ok(port, "primary worker enabled"); + port.close(); + } else { + ok(!p.enabled, "secondary provider is not enabled"); + ok(!p.getWorkerPort(), "secondary worker disabled"); + } + } + next(); + }, + + testMultipleWorkerEnabling: function(next) { + // test that all workers are enabled when we allow multiple workers + Social.enabled = false; + Services.prefs.setBoolPref("social.allowMultipleWorkers", true); + Social.enabled = true; + for (let p of Social.providers) { + ok(p.enabled, "provider enabled"); + let port = p.getWorkerPort(); + ok(port, "worker enabled"); + port.close(); + } + next(); + } +} diff --git a/browser/modules/Social.jsm b/browser/modules/Social.jsm index 1abd13fb014..d01feba4f66 100644 --- a/browser/modules/Social.jsm +++ b/browser/modules/Social.jsm @@ -91,6 +91,11 @@ this.Social = { providers: [], _disabledForSafeMode: false, + get allowMultipleWorkers() { + return Services.prefs.prefHasUserValue("social.allowMultipleWorkers") && + Services.prefs.getBoolPref("social.allowMultipleWorkers"); + }, + get _currentProviderPref() { try { return Services.prefs.getComplexValue("social.provider.current", @@ -114,15 +119,14 @@ this.Social = { this._setProvider(val); }, - // Sets the current provider and enables it. Also disables the - // previously set provider, and notifies observers of the change. + // Sets the current provider and notifies observers of the change. _setProvider: function (provider) { if (this._provider == provider) return; - // Disable the previous provider, if any, since we want only one provider to - // be enabled at once. - if (this._provider) + // Disable the previous provider, if we are not allowing multiple workers, + // since we want only one provider to be enabled at once. + if (this._provider && !Social.allowMultipleWorkers) this._provider.enabled = false; this._provider = provider; @@ -134,7 +138,6 @@ this.Social = { let enabled = !!provider; if (enabled != SocialService.enabled) { SocialService.enabled = enabled; - Services.prefs.setBoolPref("social.enabled", enabled); } let origin = this._provider && this._provider.origin; @@ -159,31 +162,40 @@ this.Social = { if (SocialService.enabled) { // Retrieve the current set of providers, and set the current provider. SocialService.getOrderedProviderList(function (providers) { - this._updateProviderCache(providers); - }.bind(this)); + Social._updateProviderCache(providers); + Social._updateWorkerState(true); + }); } // Register an observer for changes to the provider list SocialService.registerProviderListener(function providerListener(topic, data) { - // An engine change caused by adding/removing a provider should notify + // An engine change caused by adding/removing a provider should notify. + // any providers we receive are enabled in the AddonsManager if (topic == "provider-added" || topic == "provider-removed") { - this._updateProviderCache(data); + Social._updateProviderCache(data); + Social._updateWorkerState(true); Services.obs.notifyObservers(null, "social:providers-changed", null); return; } if (topic == "provider-update") { - // a provider has self-updated its manifest, we need to update our - // cache and possibly reload if it was the current provider. + // a provider has self-updated its manifest, we need to update our cache + // and reload the provider. let provider = data; - // if we need a reload, do it now - if (provider.enabled) { - Social.enabled = false; - Services.tm.mainThread.dispatch(function() { - Social.enabled = true; - }, Components.interfaces.nsIThread.DISPATCH_NORMAL); - } + SocialService.getOrderedProviderList(function(providers) { + Social._updateProviderCache(providers); + provider.reload(); + Services.obs.notifyObservers(null, "social:providers-changed", null); + }); } - }.bind(this)); + }); + }, + + _updateWorkerState: function(enable) { + // ensure that our providers are all disabled, and enabled if we allow + // multiple workers + if (enable && !Social.allowMultipleWorkers) + return; + [p.enabled = enable for (p of Social.providers) if (p.enabled != enable)]; }, // Called to update our cache of providers and set the current provider @@ -203,6 +215,9 @@ this.Social = { set enabled(val) { // Setting .enabled is just a shortcut for setting the provider to either // the default provider or null... + + this._updateWorkerState(val); + if (val) { if (!this.provider) this.provider = this.defaultProvider; @@ -210,6 +225,7 @@ this.Social = { this.provider = null; } }, + get enabled() { return this.provider != null; }, @@ -229,10 +245,6 @@ this.Social = { Services.prefs.setBoolPref("social.toast-notifications.enabled", !prefValue); }, - haveLoggedInUser: function () { - return !!(this.provider && this.provider.profile && this.provider.profile.userName); - }, - setProviderByOrigin: function (origin) { this.provider = this._getProviderFromOrigin(origin); }, diff --git a/toolkit/components/social/MozSocialAPI.jsm b/toolkit/components/social/MozSocialAPI.jsm index c5de68d1757..0e21f1bcb73 100644 --- a/toolkit/components/social/MozSocialAPI.jsm +++ b/toolkit/components/social/MozSocialAPI.jsm @@ -49,7 +49,7 @@ function injectController(doc, topic, data) { return; } - var containingBrowser = window.QueryInterface(Ci.nsIInterfaceRequestor) + let containingBrowser = window.QueryInterface(Ci.nsIInterfaceRequestor) .getInterface(Ci.nsIWebNavigation) .QueryInterface(Ci.nsIDocShell) .chromeEventHandler; @@ -67,7 +67,7 @@ function injectController(doc, topic, data) { } SocialService.getProvider(doc.nodePrincipal.origin, function(provider) { - if (provider && provider.workerURL && provider.enabled) { + if (provider && provider.enabled) { attachToWindow(provider, window); } }); @@ -88,7 +88,7 @@ function attachToWindow(provider, targetWindow) { return; } - var port = provider.getWorkerPort(targetWindow); + let port = provider.workerURL ? provider.getWorkerPort(targetWindow) : null; let mozSocialObj = { // Use a method for backwards compat with existing providers, but we @@ -206,12 +206,14 @@ function attachToWindow(provider, targetWindow) { return targetWindow.navigator.wrappedJSObject.mozSocial = contentObj; }); - targetWindow.addEventListener("unload", function () { - // We want to close the port, but also want the target window to be - // able to use the port during an unload event they setup - so we - // set a timer which will fire after the unload events have all fired. - schedule(function () { port.close(); }); - }); + if (port) { + targetWindow.addEventListener("unload", function () { + // We want to close the port, but also want the target window to be + // able to use the port during an unload event they setup - so we + // set a timer which will fire after the unload events have all fired. + schedule(function () { port.close(); }); + }); + } // We allow window.close() to close the panel, so add an event handler for // this, then cancel the event (so the window itself doesn't die) and diff --git a/toolkit/components/social/SocialService.jsm b/toolkit/components/social/SocialService.jsm index d60e95e7936..924783e0bc1 100644 --- a/toolkit/components/social/SocialService.jsm +++ b/toolkit/components/social/SocialService.jsm @@ -34,7 +34,13 @@ XPCOMUtils.defineLazyServiceGetter(this, "etld", // Internal helper methods and state let SocialServiceInternal = { - enabled: Services.prefs.getBoolPref("social.enabled"), + _enabled: Services.prefs.getBoolPref("social.enabled"), + get enabled() this._enabled, + set enabled(val) { + this._enabled = !!val; + Services.prefs.setBoolPref("social.enabled", !!val); + }, + get providerArray() { return [p for ([, p] of Iterator(this.providers))]; }, @@ -362,7 +368,6 @@ this.SocialService = { SocialServiceInternal.enabled = enable; MozSocialAPI.enabled = enable; - Services.obs.notifyObservers(null, "social:pref-changed", enable ? "enabled" : "disabled"); Services.telemetry.getHistogramById("SOCIAL_TOGGLED").add(enable); }, @@ -723,6 +728,12 @@ function SocialProvider(input) { } SocialProvider.prototype = { + reload: function() { + this._terminate(); + this._activate(); + Services.obs.notifyObservers(null, "social:provider-reload", this.origin); + }, + // Provider enabled/disabled state. Disabled providers do not have active // connections to their FrameWorkers. _enabled: false, @@ -868,6 +879,10 @@ SocialProvider.prototype = { closeAllChatWindows(this); }, + haveLoggedInUser: function () { + return !!(this.profile && this.profile.userName); + }, + // Called by the workerAPI to add/update a notification icon. setAmbientNotification: function(notification) { if (!this.profile.userName) diff --git a/toolkit/components/social/test/browser/browser_SocialProvider.js b/toolkit/components/social/test/browser/browser_SocialProvider.js index a642668fe99..81582f12061 100644 --- a/toolkit/components/social/test/browser/browser_SocialProvider.js +++ b/toolkit/components/social/test/browser/browser_SocialProvider.js @@ -2,6 +2,8 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +let provider; + function test() { waitForExplicitFinish(); @@ -13,10 +15,22 @@ function test() { ensureSocialEnabled(); - SocialService.addProvider(manifest, function (provider) { - // enable the provider - provider.enabled = true; + SocialService.addProvider(manifest, function (p) { + provider = p; + runTests(tests, undefined, undefined, function () { + SocialService.removeProvider(p.origin, function() { + ok(!provider.enabled, "removing an enabled provider should have disabled the provider"); + let port = provider.getWorkerPort(); + ok(!port, "should not be able to get a port after removing the provider"); + provider = null; + finish(); + }); + }); + }); +} +let tests = { + testSingleProvider: function(next) { ok(provider.enabled, "provider is initially enabled"); let port = provider.getWorkerPort(); ok(port, "should be able to get a port from enabled provider"); @@ -37,12 +51,38 @@ function test() { ok(port, "should be able to get a port from re-enabled provider"); port.close(); ok(provider.workerAPI, "should be able to get a workerAPI from re-enabled provider"); - - SocialService.removeProvider(provider.origin, function() { - ok(!provider.enabled, "removing an enabled provider should have disabled the provider"); + next(); + }, + testTwoProviders: function(next) { + // add another provider, test both workers + let manifest = { + origin: 'http://test2.example.com', + name: "Example Provider 2", + workerURL: "http://test2.example.com/browser/toolkit/components/social/test/browser/worker_social.js" + }; + SocialService.addProvider(manifest, function (provider2) { + ok(provider.enabled, "provider is initially enabled"); + ok(!provider2.enabled, "provider2 is not initially enabled"); + provider2.enabled = true; let port = provider.getWorkerPort(); - ok(!port, "should not be able to get a port after removing the provider"); - finish(); + let port2 = provider2.getWorkerPort(); + ok(port, "have port for provider"); + ok(port2, "have port for provider2"); + port.onmessage = function(e) { + if (e.data.topic == "test-initialization-complete") { + ok(true, "first provider initialized"); + port2.postMessage({topic: "test-initialization"}); + } + } + port2.onmessage = function(e) { + if (e.data.topic == "test-initialization-complete") { + ok(true, "second provider initialized"); + SocialService.removeProvider(provider2.origin, function() { + next(); + }); + } + } + port.postMessage({topic: "test-initialization"}); }); - }); + } } diff --git a/toolkit/components/social/test/xpcshell/test_SocialService.js b/toolkit/components/social/test/xpcshell/test_SocialService.js index 8ae1de8d1bc..b5555daebdb 100644 --- a/toolkit/components/social/test/xpcshell/test_SocialService.js +++ b/toolkit/components/social/test/xpcshell/test_SocialService.js @@ -78,38 +78,44 @@ function testGetProviderList(manifests, next) { } } + function testEnabled(manifests, next) { // Check that providers are disabled by default let providers = yield SocialService.getProviderList(next); do_check_true(providers.length >= manifests.length); - do_check_true(SocialService.enabled); + do_check_true(SocialService.enabled, "social enabled at test start"); providers.forEach(function (provider) { do_check_false(provider.enabled); }); - let notificationDisabledCorrect = false; - Services.obs.addObserver(function obs1(subj, topic, data) { - Services.obs.removeObserver(obs1, "social:pref-changed"); - notificationDisabledCorrect = data == "disabled"; - }, "social:pref-changed", false); + do_test_pending(); + function waitForEnableObserver(cb) { + Services.prefs.addObserver("social.enabled", function prefObserver(subj, topic, data) { + Services.prefs.removeObserver("social.enabled", prefObserver); + cb(); + }, false); + } // enable one of the added providers providers[providers.length-1].enabled = true; // now disable the service and check that it disabled that provider (and all others for good measure) + waitForEnableObserver(function() { + do_check_true(!SocialService.enabled); + providers.forEach(function (provider) { + do_check_false(provider.enabled); + }); + waitForEnableObserver(function() { + do_check_true(SocialService.enabled); + // Enabling the service should not enable providers + providers.forEach(function (provider) { + do_check_false(provider.enabled); + }); + do_test_finished(); + }); + SocialService.enabled = true; + }); SocialService.enabled = false; - do_check_true(notificationDisabledCorrect); - do_check_true(!SocialService.enabled); - providers.forEach(function (provider) { - do_check_false(provider.enabled); - }); - - SocialService.enabled = true; - do_check_true(SocialService.enabled); - // Enabling the service should not enable providers - providers.forEach(function (provider) { - do_check_false(provider.enabled); - }); } function testAddRemoveProvider(manifests, next) { From d610a933fa88a737392b2a7e385ca2f2b14ccbb7 Mon Sep 17 00:00:00 2001 From: Shane Caraveo Date: Wed, 21 Aug 2013 11:38:33 -0700 Subject: [PATCH 09/13] bug 891221 add tests for chat from multiple providers, r=markh --- .../test/social/browser_social_chatwindow.js | 126 ++++++++++++++---- browser/base/content/test/social/head.js | 2 +- 2 files changed, 98 insertions(+), 30 deletions(-) diff --git a/browser/base/content/test/social/browser_social_chatwindow.js b/browser/base/content/test/social/browser_social_chatwindow.js index 3d8b920273f..4c749de61a8 100644 --- a/browser/base/content/test/social/browser_social_chatwindow.js +++ b/browser/base/content/test/social/browser_social_chatwindow.js @@ -2,17 +2,71 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +let SocialService = Cu.import("resource://gre/modules/SocialService.jsm", {}).SocialService; + +let manifests = [ + { + name: "provider@example.com", + origin: "https://example.com", + sidebarURL: "https://example.com/browser/browser/base/content/test/social/social_sidebar.html?example.com", + workerURL: "https://example.com/browser/browser/base/content/test/social/social_worker.js", + iconURL: "chrome://branding/content/icon48.png" + }, + { + name: "provider@test1", + origin: "https://test1.example.com", + sidebarURL: "https://test1.example.com/browser/browser/base/content/test/social/social_sidebar.html?test1", + workerURL: "https://test1.example.com/browser/browser/base/content/test/social/social_worker.js", + iconURL: "chrome://branding/content/icon48.png" + }, + { + name: "provider@test2", + origin: "https://test2.example.com", + sidebarURL: "https://test2.example.com/browser/browser/base/content/test/social/social_sidebar.html?test2", + workerURL: "https://test2.example.com/browser/browser/base/content/test/social/social_worker.js", + iconURL: "chrome://branding/content/icon48.png" + } +]; + +let chatId = 0; +function openChat(provider, callback) { + let chatUrl = provider.origin + "/browser/browser/base/content/test/social/social_chat.html"; + let port = provider.getWorkerPort(); + port.onmessage = function(e) { + if (e.data.topic == "got-chatbox-message") { + port.close(); + callback(); + } + } + let url = chatUrl + "?" + (chatId++); + port.postMessage({topic: "test-init"}); + port.postMessage({topic: "test-worker-chat", data: url}); + gURLsNotRemembered.push(url); +} + +function waitPrefChange(cb) { + Services.prefs.addObserver("social.enabled", function prefObserver(subject, topic, data) { + Services.prefs.removeObserver("social.enabled", prefObserver); + executeSoon(cb); + }, false); +} + +function setWorkerMode(multiple, cb) { + waitPrefChange(function() { + if (multiple) + Services.prefs.setBoolPref("social.allowMultipleWorkers", true); + else + Services.prefs.clearUserPref("social.allowMultipleWorkers"); + waitPrefChange(cb); + Social.enabled = true; + }); + Social.enabled = false; +} + function test() { requestLongerTimeout(2); // only debug builds seem to need more time... waitForExplicitFinish(); - let manifest = { // normal provider - name: "provider 1", - origin: "https://example.com", - sidebarURL: "https://example.com/browser/browser/base/content/test/social/social_sidebar.html", - workerURL: "https://example.com/browser/browser/base/content/test/social/social_worker.js", - iconURL: "https://example.com/browser/browser/base/content/test/moz.png" - }; let oldwidth = window.outerWidth; // we futz with these, so we restore them let oldleft = window.screenX; window.moveTo(0, window.screenY) @@ -21,7 +75,7 @@ function test() { ok(chats.children.length == 0, "no chatty children left behind"); cb(); }; - runSocialTestWithProvider(manifest, function (finishcb) { + runSocialTestWithProvider(manifests, function (finishcb) { runSocialTests(tests, undefined, postSubTest, function() { window.moveTo(oldleft, window.screenY) window.resizeTo(oldwidth, window.outerHeight); @@ -147,7 +201,7 @@ var tests = { maybeOpenAnother(); }, testWorkerChatWindow: function(next) { - const chatUrl = "https://example.com/browser/browser/base/content/test/social/social_chat.html"; + const chatUrl = Social.provider.origin + "/browser/browser/base/content/test/social/social_chat.html"; let chats = document.getElementById("pinnedchats"); let port = Social.provider.getWorkerPort(); ok(port, "provider has a port"); @@ -384,7 +438,7 @@ var tests = { testSecondTopLevelWindow: function(next) { // Bug 817782 - check chats work in new top-level windows. - const chatUrl = "https://example.com/browser/browser/base/content/test/social/social_chat.html"; + const chatUrl = Social.provider.origin + "/browser/browser/base/content/test/social/social_chat.html"; let port = Social.provider.getWorkerPort(); let secondWindow; port.onmessage = function(e) { @@ -407,23 +461,9 @@ var tests = { testChatWindowChooser: function(next) { // Tests that when a worker creates a chat, it is opened in the correct // window. - const chatUrl = "https://example.com/browser/browser/base/content/test/social/social_chat.html"; - let chatId = 1; - let port = Social.provider.getWorkerPort(); - port.postMessage({topic: "test-init"}); - - function openChat(callback) { - port.onmessage = function(e) { - if (e.data.topic == "got-chatbox-message") - callback(); - } - let url = chatUrl + "?" + (chatId++); - port.postMessage({topic: "test-worker-chat", data: url}); - } - // open a chat (it will open in the main window) ok(!window.SocialChatBar.hasChats, "first window should start with no chats"); - openChat(function() { + openChat(Social.provider, function() { ok(window.SocialChatBar.hasChats, "first window has the chat"); // create a second window - this will be the "most recent" and will // therefore be the window that hosts the new chat (see bug 835111) @@ -431,27 +471,55 @@ var tests = { secondWindow.addEventListener("load", function loadListener() { secondWindow.removeEventListener("load", loadListener); ok(!secondWindow.SocialChatBar.hasChats, "second window has no chats"); - openChat(function() { + openChat(Social.provider, function() { ok(secondWindow.SocialChatBar.hasChats, "second window now has chats"); is(window.SocialChatBar.chatbar.childElementCount, 1, "first window still has 1 chat"); window.SocialChatBar.chatbar.removeAll(); // now open another chat - it should still open in the second. - openChat(function() { + openChat(Social.provider, function() { ok(!window.SocialChatBar.hasChats, "first window has no chats"); ok(secondWindow.SocialChatBar.hasChats, "second window has a chat"); secondWindow.close(); - port.close(); next(); }); }); }) }); }, + testMultipleProviderChat: function(next) { + // while pref'd off, we need to set the worker mode to multiple providers + setWorkerMode(true, function() { + // test incomming chats from all providers + openChat(Social.providers[0], function() { + openChat(Social.providers[1], function() { + openChat(Social.providers[2], function() { + let chats = document.getElementById("pinnedchats"); + waitForCondition(function() chats.children.length == Social.providers.length, + function() { + ok(true, "one chat window per provider opened"); + // test logout of a single provider + let provider = Social.providers[0]; + let port = provider.getWorkerPort(); + port.postMessage({topic: "test-logout"}); + waitForCondition(function() chats.children.length == Social.providers.length - 1, + function() { + port.close(); + chats.removeAll(); + ok(!chats.selectedChat, "chats are all closed"); + setWorkerMode(false, next); + }, + "chat window didn't close"); + }, "chat windows did not open"); + }); + }); + }); + }); + }, // XXX - note this must be the last test until we restore the login state // between tests... testCloseOnLogout: function(next) { - const chatUrl = "https://example.com/browser/browser/base/content/test/social/social_chat.html"; + const chatUrl = Social.provider.origin + "/browser/browser/base/content/test/social/social_chat.html"; let port = Social.provider.getWorkerPort(); ok(port, "provider has a port"); let opened = false; diff --git a/browser/base/content/test/social/head.js b/browser/base/content/test/social/head.js index 33b8a448341..f6a00e8df29 100644 --- a/browser/base/content/test/social/head.js +++ b/browser/base/content/test/social/head.js @@ -417,8 +417,8 @@ function get3ChatsForCollapsing(mode, cb) { function makeChat(mode, uniqueid, cb) { info("making a chat window '" + uniqueid +"'"); - const chatUrl = "https://example.com/browser/browser/base/content/test/social/social_chat.html"; let provider = Social.provider; + const chatUrl = provider.origin + "/browser/browser/base/content/test/social/social_chat.html"; let isOpened = window.SocialChatBar.openChat(provider, chatUrl + "?id=" + uniqueid, function(chat) { info("chat window has opened"); // we can't callback immediately or we might close the chat during From 99a087bce197b6c114711ef976fb17f6a6c06c0b Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Wed, 21 Aug 2013 11:33:43 -0700 Subject: [PATCH 10/13] Bug 906718 - Pressing BACK should move the user up in the bookmark folder hierarchy. r=sriram --- .../base/home/BookmarksListAdapter.java | 13 +++++--- .../android/base/home/BookmarksListView.java | 32 ++++++++++++++----- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/mobile/android/base/home/BookmarksListAdapter.java b/mobile/android/base/home/BookmarksListAdapter.java index b139e1fa293..bf2018a29d1 100644 --- a/mobile/android/base/home/BookmarksListAdapter.java +++ b/mobile/android/base/home/BookmarksListAdapter.java @@ -60,13 +60,18 @@ class BookmarksListAdapter extends MultiTypeCursorAdapter { /** * Moves to parent folder, if one exists. + * + * @return Whether the adapter successfully moved to a parent folder. */ - public void moveToParentFolder() { + public boolean moveToParentFolder() { // If we're already at the root, we can't move to a parent folder - if (mParentStack.size() != 1) { - mParentStack.removeFirst(); - refreshCurrentFolder(); + if (mParentStack.size() == 1) { + return false; } + + mParentStack.removeFirst(); + refreshCurrentFolder(); + return true; } /** diff --git a/mobile/android/base/home/BookmarksListView.java b/mobile/android/base/home/BookmarksListView.java index c3246aede8e..ab0df636228 100644 --- a/mobile/android/base/home/BookmarksListView.java +++ b/mobile/android/base/home/BookmarksListView.java @@ -13,6 +13,7 @@ import org.mozilla.gecko.home.HomePager.OnUrlOpenListener; import android.content.Context; import android.database.Cursor; import android.util.AttributeSet; +import android.view.KeyEvent; import android.view.MotionEvent; import android.view.View; import android.view.ViewConfiguration; @@ -56,6 +57,17 @@ public class BookmarksListView extends HomeListView super.onAttachedToWindow(); setOnItemClickListener(this); + + setOnKeyListener(new View.OnKeyListener() { + @Override + public boolean onKey(View v, int keyCode, KeyEvent event) { + // If the user hit the BACK key, try to move to the parent folder. + if (keyCode == KeyEvent.KEYCODE_BACK) { + return getBookmarksListAdapter().moveToParentFolder(); + } + return false; + } + }); } @Override @@ -101,14 +113,7 @@ public class BookmarksListView extends HomeListView // Absolute position for the adapter. position -= headerCount; - BookmarksListAdapter adapter; - ListAdapter listAdapter = getAdapter(); - if (listAdapter instanceof HeaderViewListAdapter) { - adapter = (BookmarksListAdapter) ((HeaderViewListAdapter) listAdapter).getWrappedAdapter(); - } else { - adapter = (BookmarksListAdapter) listAdapter; - } - + final BookmarksListAdapter adapter = getBookmarksListAdapter(); if (adapter.isShowingChildFolder()) { if (position == 0) { // If we tap on an opened folder, move back to parent folder. @@ -141,4 +146,15 @@ public class BookmarksListView extends HomeListView getOnUrlOpenListener().onUrlOpen(url, EnumSet.of(OnUrlOpenListener.Flags.ALLOW_SWITCH_TO_TAB)); } } + + private BookmarksListAdapter getBookmarksListAdapter() { + BookmarksListAdapter adapter; + ListAdapter listAdapter = getAdapter(); + if (listAdapter instanceof HeaderViewListAdapter) { + adapter = (BookmarksListAdapter) ((HeaderViewListAdapter) listAdapter).getWrappedAdapter(); + } else { + adapter = (BookmarksListAdapter) listAdapter; + } + return adapter; + } } From a3d6f033bff8e923db5c7081f5fa590b3cb28acf Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Wed, 21 Aug 2013 11:33:58 -0700 Subject: [PATCH 11/13] Bug 885084 - Only return top bookmarks for bookmarks page thumbnails. r=wesj --- mobile/android/base/db/BrowserDB.java | 14 +++++++------- mobile/android/base/db/LocalBrowserDB.java | 10 +++++++--- mobile/android/base/home/BookmarksPage.java | 2 +- 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/mobile/android/base/db/BrowserDB.java b/mobile/android/base/db/BrowserDB.java index 34f254b3309..a076dc7cada 100644 --- a/mobile/android/base/db/BrowserDB.java +++ b/mobile/android/base/db/BrowserDB.java @@ -38,9 +38,9 @@ public class BrowserDB { public Cursor filter(ContentResolver cr, CharSequence constraint, int limit); - // This should onlyl return frecent sites, BrowserDB.getTopSites will do the + // This should only return frecent bookmarks, BrowserDB.getTopBookmarks will do the // work to combine that list with the pinned sites list - public Cursor getTopSites(ContentResolver cr, int limit); + public Cursor getTopBookmarks(ContentResolver cr, int limit); public void updateVisitedHistory(ContentResolver cr, String uri); @@ -137,12 +137,12 @@ public class BrowserDB { return sDb.filter(cr, constraint, limit); } - public static Cursor getTopSites(ContentResolver cr, int limit) { - // Note this is not a single query anymore, but actually returns a mixture of two queries, one for topSites - // and one for pinned sites - Cursor topSites = sDb.getTopSites(cr, limit); + public static Cursor getTopBookmarks(ContentResolver cr, int limit) { + // Note this is not a single query anymore, but actually returns a mixture of two queries, + // one for top bookmarks, and one for pinned sites (which are actually bookmarks as well). + Cursor topBookmarks = sDb.getTopBookmarks(cr, limit); Cursor pinnedSites = sDb.getPinnedSites(cr, limit); - return new TopSitesCursorWrapper(pinnedSites, topSites, limit); + return new TopSitesCursorWrapper(pinnedSites, topBookmarks, limit); } public static void updateVisitedHistory(ContentResolver cr, String uri) { diff --git a/mobile/android/base/db/LocalBrowserDB.java b/mobile/android/base/db/LocalBrowserDB.java index 947685876d6..87cec8073ba 100644 --- a/mobile/android/base/db/LocalBrowserDB.java +++ b/mobile/android/base/db/LocalBrowserDB.java @@ -231,9 +231,13 @@ public class LocalBrowserDB implements BrowserDB.BrowserDBIface { } @Override - public Cursor getTopSites(ContentResolver cr, int limit) { - // Filter out sites that are pinned - String selection = DBUtils.concatenateWhere("", Combined.URL + " NOT IN (SELECT " + + public Cursor getTopBookmarks(ContentResolver cr, int limit) { + // Only select bookmarks. Unfortunately, we need to query the combined view, + // instead of just the bookmarks table, in order to do the frecency calculation. + String selection = Combined.BOOKMARK_ID + " IS NOT NULL"; + + // Filter out sites that are pinned. + selection = DBUtils.concatenateWhere(selection, Combined.URL + " NOT IN (SELECT " + Bookmarks.URL + " FROM bookmarks WHERE " + DBUtils.qualifyColumn("bookmarks", Bookmarks.PARENT) + " == ? AND " + DBUtils.qualifyColumn("bookmarks", Bookmarks.IS_DELETED) + " == 0)"); diff --git a/mobile/android/base/home/BookmarksPage.java b/mobile/android/base/home/BookmarksPage.java index ac5d247a65a..b22057dd49c 100644 --- a/mobile/android/base/home/BookmarksPage.java +++ b/mobile/android/base/home/BookmarksPage.java @@ -362,7 +362,7 @@ public class BookmarksPage extends HomeFragment { @Override public Cursor loadCursor() { final int max = getContext().getResources().getInteger(R.integer.number_of_top_sites); - return BrowserDB.getTopSites(getContext().getContentResolver(), max); + return BrowserDB.getTopBookmarks(getContext().getContentResolver(), max); } } From dfe4267f13fecabebc8cf215cb78ea90b03c2427 Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Wed, 21 Aug 2013 11:34:06 -0700 Subject: [PATCH 12/13] Bug 897772 - Actually get a favicon when creating a homescreen shortcut from the about:home context menu. r=wesj --- mobile/android/base/home/HomeFragment.java | 38 ++++++++++++++++++---- mobile/android/base/home/HomeListView.java | 8 ----- 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/mobile/android/base/home/HomeFragment.java b/mobile/android/base/home/HomeFragment.java index dc87504d58d..9920b373272 100644 --- a/mobile/android/base/home/HomeFragment.java +++ b/mobile/android/base/home/HomeFragment.java @@ -6,6 +6,7 @@ package org.mozilla.gecko.home; import org.mozilla.gecko.EditBookmarkDialog; +import org.mozilla.gecko.Favicons; import org.mozilla.gecko.GeckoAppShell; import org.mozilla.gecko.GeckoEvent; import org.mozilla.gecko.R; @@ -132,14 +133,8 @@ abstract class HomeFragment extends Fragment { return false; } - // FIXME: bug 897772 - Bitmap bitmap = null; - if (info.favicon != null) { - bitmap = BitmapUtils.decodeByteArray(info.favicon); - } - String shortcutTitle = TextUtils.isEmpty(info.title) ? info.url.replaceAll(REGEX_URL_TO_TITLE, "") : info.title; - GeckoAppShell.createShortcut(shortcutTitle, info.url, bitmap, ""); + new AddToLauncherTask(info.url, shortcutTitle).execute(); return true; } @@ -225,6 +220,35 @@ abstract class HomeFragment extends Fragment { } } + private static class AddToLauncherTask extends UiAsyncTask { + private final String mUrl; + private final String mTitle; + + public AddToLauncherTask(String url, String title) { + super(ThreadUtils.getBackgroundHandler()); + + mUrl = url; + mTitle = title; + } + + @Override + public String doInBackground(Void... params) { + return Favicons.getInstance().getFaviconUrlForPageUrl(mUrl); + } + + @Override + public void onPostExecute(String faviconUrl) { + Favicons.OnFaviconLoadedListener listener = new Favicons.OnFaviconLoadedListener() { + @Override + public void onFaviconLoaded(String url, Bitmap favicon) { + GeckoAppShell.createShortcut(mTitle, mUrl, favicon, ""); + } + }; + + Favicons.getInstance().loadFavicon(mUrl, faviconUrl, 0, listener); + } + } + private static class RemoveBookmarkTask extends UiAsyncTask { private final Context mContext; private final int mId; diff --git a/mobile/android/base/home/HomeListView.java b/mobile/android/base/home/HomeListView.java index f8968ca2e11..d2a4a1605b1 100644 --- a/mobile/android/base/home/HomeListView.java +++ b/mobile/android/base/home/HomeListView.java @@ -113,7 +113,6 @@ public class HomeListView extends ListView public int bookmarkId; public int historyId; public String url; - public byte[] favicon; public String title; public int display; public boolean isFolder; @@ -165,13 +164,6 @@ public class HomeListView extends ListView historyId = -1; } - final int faviconCol = cursor.getColumnIndex(Combined.FAVICON); - if (faviconCol != -1) { - favicon = cursor.getBlob(faviconCol); - } else { - favicon = null; - } - // We only have the parent column in cursors from getBookmarksInFolder. final int parentCol = cursor.getColumnIndex(Bookmarks.PARENT); if (parentCol != -1) { From e8c10d777574ee68bdc5b0f16974476ff9f165f0 Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Wed, 21 Aug 2013 11:34:16 -0700 Subject: [PATCH 13/13] Bug 907172 - Periodically invalidate the cached return value for desktopBookmarksExist(). r=lucasr --- mobile/android/base/home/BookmarksPage.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mobile/android/base/home/BookmarksPage.java b/mobile/android/base/home/BookmarksPage.java index b22057dd49c..8c08dbe367e 100644 --- a/mobile/android/base/home/BookmarksPage.java +++ b/mobile/android/base/home/BookmarksPage.java @@ -149,6 +149,10 @@ public class BookmarksPage extends HomeFragment { }); mList.setAdapter(mListAdapter); + // Invalidate the cached value that keeps track of whether or + // not desktop bookmarks (or reading list items) exist. + BrowserDB.invalidateCachedState(); + // Create callbacks before the initial loader is started. mLoaderCallbacks = new CursorLoaderCallbacks(activity, getLoaderManager()); mThumbnailsLoaderCallbacks = new ThumbnailsLoaderCallbacks();