From a66fd6506b4b86f937dd0938ea56d2f4b68cee37 Mon Sep 17 00:00:00 2001 From: Mark Capella Date: Wed, 25 Feb 2015 01:43:35 -0500 Subject: [PATCH 01/21] Bug 1130258 - Avoid closing wrong Text Selection, r=wesj --- mobile/android/base/TextSelection.java | 14 +++++++- .../roboextender/testInputSelections.html | 18 ++++++---- .../roboextender/testSelectionHandler.html | 10 ++++-- .../roboextender/testTextareaSelections.html | 33 ++++++++++++------- .../chrome/content/SelectionHandler.js | 15 ++++++++- 5 files changed, 69 insertions(+), 21 deletions(-) diff --git a/mobile/android/base/TextSelection.java b/mobile/android/base/TextSelection.java index ff4523bacbd..be3d1d091b4 100644 --- a/mobile/android/base/TextSelection.java +++ b/mobile/android/base/TextSelection.java @@ -43,6 +43,7 @@ class TextSelection extends Layer implements GeckoEventListener { private final DrawListener mDrawListener; private boolean mDraggingHandles; + private int selectionID; // Unique ID provided for each selection action. private float mViewLeft; private float mViewTop; private float mViewZoom; @@ -131,6 +132,7 @@ class TextSelection extends Layer implements GeckoEventListener { public void run() { try { if (event.equals("TextSelection:ShowHandles")) { + selectionID = message.getInt("selectionID"); final JSONArray handles = message.getJSONArray("handles"); for (int i=0; i < handles.length(); i++) { String handle = handles.getString(i); @@ -315,7 +317,17 @@ class TextSelection extends Layer implements GeckoEventListener { public void onDestroyActionMode(ActionModeCompat mode) { mActionMode = null; mCallback = null; - GeckoAppShell.sendEventToGecko(GeckoEvent.createBroadcastEvent("TextSelection:End", null)); + final JSONObject args = new JSONObject(); + try { + args.put("selectionID", selectionID); + } catch (JSONException e) { + Log.e(LOGTAG, "Error building JSON arguments for TextSelection:End", e); + return; + } + + final GeckoEvent event = + GeckoEvent.createBroadcastEvent("TextSelection:End", args.toString()); + GeckoAppShell.sendEventToGecko(event); } } } diff --git a/mobile/android/base/tests/roboextender/testInputSelections.html b/mobile/android/base/tests/roboextender/testInputSelections.html index 5978951216a..302fea9165d 100644 --- a/mobile/android/base/tests/roboextender/testInputSelections.html +++ b/mobile/android/base/tests/roboextender/testInputSelections.html @@ -102,7 +102,8 @@ function testLTR_selectAll() { ]).then(function() { // Close selection and complete test. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(!sh.isSelectionActive(), @@ -171,7 +172,8 @@ function testRTL_selectAll() { ]).then(function() { // Close selection and complete test. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(!sh.isSelectionActive(), @@ -216,7 +218,8 @@ function testLTR_dragFocusHandleToSelf() { let focusDragSelectionText = sh._getSelectedText(); // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testLTR_dragFocusHandleToSelf - Test Starts."), @@ -275,7 +278,8 @@ function testLTR_dragAnchorHandleToSelf() { let anchorDragSelectionText = sh._getSelectedText(); // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testLTR_dragAnchorHandleToSelf - Test Starts."), @@ -333,7 +337,8 @@ function testRTL_dragFocusHandleToSelf() { let focusDragSelectionText = sh._getSelectedText(); // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testRTL_dragFocusHandleToSelf - Test Starts."), @@ -392,7 +397,8 @@ function testRTL_dragAnchorHandleToSelf() { let anchorDragSelectionText = sh._getSelectedText(); // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testRTL_dragAnchorHandleToSelf - Test Starts."), diff --git a/mobile/android/base/tests/roboextender/testSelectionHandler.html b/mobile/android/base/tests/roboextender/testSelectionHandler.html index 068062c647b..f5c2042e28f 100644 --- a/mobile/android/base/tests/roboextender/testSelectionHandler.html +++ b/mobile/android/base/tests/roboextender/testSelectionHandler.html @@ -208,7 +208,12 @@ function testCloseSelection() { // Various ways to close an active selection. ]).then(function() { sh.startSelection(inputNode); - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: -1})); + return ok(sh.isSelectionActive(), "unrelated TextSelection:End should not close active selection"); + }).then(function() { + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return ok(!sh.isSelectionActive(), "TextSelection:End should close active selection"); }).then(function() { @@ -310,7 +315,8 @@ function testAttachCaret() { ]); }).then(function() { - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(!sh.isSelectionActive(), "Selection should not be active at end of testAttachCaret"), diff --git a/mobile/android/base/tests/roboextender/testTextareaSelections.html b/mobile/android/base/tests/roboextender/testTextareaSelections.html index 49d599282a0..6435be2c04f 100644 --- a/mobile/android/base/tests/roboextender/testTextareaSelections.html +++ b/mobile/android/base/tests/roboextender/testTextareaSelections.html @@ -77,7 +77,8 @@ function testLTR_selectionPoints() { let midpointSelText = sh._getSelectedText(); // Close selection and complete test. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ selectionExists(selection, "LTR Selection existed at points"), @@ -121,7 +122,8 @@ function testRTL_selectionPoints() { let midpointSelText = sh._getSelectedText(); // Close selection and complete test. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ selectionExists(selection, "RTL Selection existed at points"), @@ -186,7 +188,8 @@ function test_selectionLineHeight() { ]).then(function() { // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ greaterThan(selectionLineHeight, 0, "Distance from one line to another " + @@ -237,7 +240,8 @@ function testLTR_moveFocusHandleDown() { focusPt : new Point(sh._cache.focusPt.x, sh._cache.focusPt.y) }; // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testLTR_moveFocusHandleDown - Test Starts."), @@ -302,7 +306,8 @@ function testLTR_moveFocusHandleUp() { focusPt : new Point(sh._cache.focusPt.x, sh._cache.focusPt.y) }; // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testLTR_moveFocusHandleUp - Test Starts."), @@ -370,7 +375,8 @@ function testLTR_moveAnchorHandleUp() { focusPt : new Point(sh._cache.focusPt.x, sh._cache.focusPt.y) }; // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testLTR_moveAnchorHandleUp - Test Starts."), @@ -434,7 +440,8 @@ function testLTR_moveAnchorHandleDown() { focusPt : new Point(sh._cache.focusPt.x, sh._cache.focusPt.y) }; // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testLTR_moveAnchorHandleDown - Test Starts."), @@ -502,7 +509,8 @@ function testRTL_moveFocusHandleDown() { focusPt : new Point(sh._cache.focusPt.x, sh._cache.focusPt.y) }; // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testRTL_moveFocusHandleDown - Test Starts."), @@ -566,7 +574,8 @@ function testRTL_moveFocusHandleUp() { focusPt : new Point(sh._cache.focusPt.x, sh._cache.focusPt.y) }; // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testRTL_moveFocusHandleUp - Test Starts."), @@ -634,7 +643,8 @@ function testRTL_moveAnchorHandleUp() { focusPt : new Point(sh._cache.focusPt.x, sh._cache.focusPt.y) }; // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testRTL_moveAnchorHandleUp - Test Starts."), @@ -698,7 +708,8 @@ function testRTL_moveAnchorHandleDown() { focusPt : new Point(sh._cache.focusPt.x, sh._cache.focusPt.y) }; // Complete test, and report. - Services.obs.notifyObservers(null, "TextSelection:End", {}); + Services.obs.notifyObservers(null, "TextSelection:End", + JSON.stringify({selectionID: sh._selectionID})); return Promise.all([ ok(true, "testRTL_moveAnchorHandleDown - Test Starts."), diff --git a/mobile/android/chrome/content/SelectionHandler.js b/mobile/android/chrome/content/SelectionHandler.js index c10dc419373..4a7b8435794 100644 --- a/mobile/android/chrome/content/SelectionHandler.js +++ b/mobile/android/chrome/content/SelectionHandler.js @@ -44,6 +44,7 @@ var SelectionHandler = { _focusIsRTL: false, _activeType: 0, // TYPE_NONE + _selectionID: 0, // Unique Selection ID _draggingHandles: false, // True while user drags text selection handles _dragStartAnchorOffset: null, // Editables need initial pos during HandleMove events @@ -134,10 +135,19 @@ var SelectionHandler = { } break; } + case "Tab:Selected": - case "TextSelection:End": this._closeSelection(); break; + + case "TextSelection:End": + let data = JSON.parse(aData); + // End the requested selection only. + if (this._selectionID === data.selectionID) { + this._closeSelection(); + } + break; + case "TextSelection:Action": for (let type in this.actions) { if (this.actions[type].id == aData) { @@ -365,6 +375,7 @@ var SelectionHandler = { // Determine position and show handles, open actionbar this._positionHandles(positions); Messaging.sendRequest({ + selectionID: this._selectionID, type: "TextSelection:ShowHandles", handles: [this.HANDLE_TYPE_ANCHOR, this.HANDLE_TYPE_FOCUS] }); @@ -756,6 +767,7 @@ var SelectionHandler = { // Determine position and show caret, open actionbar this._positionHandles(); Messaging.sendRequest({ + selectionID: this._selectionID, type: "TextSelection:ShowHandles", handles: [this.HANDLE_TYPE_CARET] }); @@ -777,6 +789,7 @@ var SelectionHandler = { aElement.focus(); } + this._selectionID++; this._stopDraggingHandles(); this._contentWindow = aElement.ownerDocument.defaultView; this._targetIsRTL = (this._contentWindow.getComputedStyle(aElement, "").direction == "rtl"); From f1c1a680e5c8820e2cebe3b30ad42c873649c7cb Mon Sep 17 00:00:00 2001 From: Mark Banner Date: Wed, 25 Feb 2015 07:36:50 +0000 Subject: [PATCH 02/21] Bug 1110973 - Add a preference for enabling fake streams for tests, and use it in the Loop functional tests. r=smaug --- .../components/loop/test/functional/config.py | 4 +++- dom/media/MediaManager.cpp | 21 +++++++++++++------ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/browser/components/loop/test/functional/config.py b/browser/components/loop/test/functional/config.py index ed95163322a..940ef234419 100644 --- a/browser/components/loop/test/functional/config.py +++ b/browser/components/loop/test/functional/config.py @@ -16,5 +16,7 @@ FIREFOX_PREFERENCES = { "loop.seenToS": "seen", # this dialog is fragile, and likely to introduce intermittent failures - "media.navigator.permission.disabled": True + "media.navigator.permission.disabled": True, + # Use fake streams only + "media.navigator.streams.fake": True } diff --git a/dom/media/MediaManager.cpp b/dom/media/MediaManager.cpp index 1c199e50c64..540b912b88c 100644 --- a/dom/media/MediaManager.cpp +++ b/dom/media/MediaManager.cpp @@ -1328,14 +1328,15 @@ public: already_AddRefed aOnSuccess, already_AddRefed aOnFailure, uint64_t aWindowId, nsACString& aAudioLoopbackDev, - nsACString& aVideoLoopbackDev) + nsACString& aVideoLoopbackDev, bool aUseFakeDevices) : mConstraints(aConstraints) , mOnSuccess(aOnSuccess) , mOnFailure(aOnFailure) , mManager(MediaManager::GetInstance()) , mWindowId(aWindowId) , mLoopbackAudioDevice(aAudioLoopbackDev) - , mLoopbackVideoDevice(aVideoLoopbackDev) {} + , mLoopbackVideoDevice(aVideoLoopbackDev) + , mUseFakeDevices(aUseFakeDevices) {} void // NS_IMETHOD Run() @@ -1343,7 +1344,7 @@ public: NS_ASSERTION(!NS_IsMainThread(), "Don't call on main thread"); nsRefPtr backend; - if (mConstraints.mFake) + if (mConstraints.mFake || mUseFakeDevices) backend = new MediaEngineDefault(mConstraints.mFakeTracks); else backend = mManager->GetBackend(mWindowId); @@ -1387,6 +1388,7 @@ private: // automated media tests only. nsCString mLoopbackAudioDevice; nsCString mLoopbackVideoDevice; + bool mUseFakeDevices; }; MediaManager::MediaManager() @@ -1596,10 +1598,15 @@ MediaManager::GetUserMedia( if (!Preferences::GetBool("media.navigator.video.enabled", true)) { c.mVideo.SetAsBoolean() = false; } + bool fake = true; + if (!c.mFake && + !Preferences::GetBool("media.navigator.streams.fake", false)) { + fake = false; + } // Pass callbacks and MediaStreamListener along to GetUserMediaTask. nsAutoPtr task; - if (c.mFake) { + if (fake) { // Fake stream from default backend. task = new GetUserMediaTask(c, onSuccess.forget(), onFailure.forget(), windowID, listener, mPrefs, new MediaEngineDefault(c.mFakeTracks)); @@ -1711,7 +1718,7 @@ MediaManager::GetUserMedia( // XXX No full support for picture in Desktop yet (needs proper UI) if (privileged || - (c.mFake && !Preferences::GetBool("media.navigator.permission.fake"))) { + (fake && !Preferences::GetBool("media.navigator.permission.fake"))) { MediaManager::GetMessageLoop()->PostTask(FROM_HERE, task.forget()); } else { bool isHTTPS = false; @@ -1805,12 +1812,14 @@ MediaManager::GetUserMediaDevices(nsPIDOMWindow* aWindow, Preferences::GetCString("media.audio_loopback_dev"); nsAdoptingCString loopbackVideoDevice = Preferences::GetCString("media.video_loopback_dev"); + bool useFakeStreams = + Preferences::GetBool("media.navigator.streams.fake", false); MediaManager::GetMessageLoop()->PostTask(FROM_HERE, new GetUserMediaDevicesTask( aConstraints, onSuccess.forget(), onFailure.forget(), (aInnerWindowID ? aInnerWindowID : aWindow->WindowID()), - loopbackAudioDevice, loopbackVideoDevice)); + loopbackAudioDevice, loopbackVideoDevice, useFakeStreams)); return NS_OK; } From 8862413e7bcd72095f00cb2ebaf3725c5103c514 Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Wed, 25 Feb 2015 18:54:59 +1100 Subject: [PATCH 03/21] Bug 1131410 - Extract sync's log management so it can be reused by the reading-list back-end. r=adw --- services/common/logmanager.js | 277 ++++++++++++++++++ services/common/moz.build | 1 + .../common/tests/unit/test_load_modules.js | 1 + services/common/tests/unit/test_logmanager.js | 103 +++++++ services/common/tests/unit/xpcshell.ini | 1 + services/sync/modules/policies.js | 136 ++------- services/sync/tests/unit/test_errorhandler.js | 14 +- .../tests/unit/test_errorhandler_filelog.js | 38 ++- 8 files changed, 428 insertions(+), 143 deletions(-) create mode 100644 services/common/logmanager.js create mode 100644 services/common/tests/unit/test_logmanager.js diff --git a/services/common/logmanager.js b/services/common/logmanager.js new file mode 100644 index 00000000000..700672576e2 --- /dev/null +++ b/services/common/logmanager.js @@ -0,0 +1,277 @@ +/* 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/. */ + +const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components; + +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); +XPCOMUtils.defineLazyModuleGetter(this, 'Services', + 'resource://gre/modules/Services.jsm'); +XPCOMUtils.defineLazyModuleGetter(this, 'Preferences', + 'resource://gre/modules/Preferences.jsm'); +XPCOMUtils.defineLazyModuleGetter(this, 'FileUtils', + 'resource://gre/modules/FileUtils.jsm'); +XPCOMUtils.defineLazyModuleGetter(this, 'Log', + 'resource://gre/modules/Log.jsm'); +XPCOMUtils.defineLazyModuleGetter(this, 'OS', + 'resource://gre/modules/osfile.jsm'); +XPCOMUtils.defineLazyModuleGetter(this, 'CommonUtils', + 'resource://services-common/utils.js'); +Cu.import("resource://gre/modules/Task.jsm"); + +this.EXPORTED_SYMBOLS = [ + "LogManager", +]; + +const DEFAULT_MAX_ERROR_AGE = 20 * 24 * 60 * 60; // 20 days + +// "shared" logs (ie, where the same log name is used by multiple LogManager +// instances) are a fact of life here - eg, FirefoxAccounts logs are used by +// both Sync and Reading-list. +// However, different instances have different pref branches, so we need to +// handle when one pref branch says "Debug" and the other says "Error" +// So we (a) keep singleton console and dump appenders and (b) keep track +// of the minimum (ie, most verbose) level and use that. +// This avoids (a) the most recent setter winning (as that is indeterminate) +// and (b) multiple dump/console appenders being added to the same log multiple +// times, which would cause messages to appear twice. + +// Singletons used by each instance. +let formatter; +let dumpAppender; +let consoleAppender; + +// A set of all preference roots used by all instances. +let allBranches = new Set(); + +// The public LogManager object. +function LogManager(prefRoot, logNames, logFilePrefix) { + this.init(prefRoot, logNames, logFilePrefix); +} + +LogManager.prototype = { + REASON_SUCCESS: "success", + REASON_ERROR: "error", + + _cleaningUpFileLogs: false, + + _prefObservers: [], + + init(prefRoot, logNames, logFilePrefix) { + if (prefRoot instanceof Preferences) { + this._prefs = prefRoot; + } else { + this._prefs = new Preferences(prefRoot); + } + + this.logFilePrefix = logFilePrefix; + if (!formatter) { + // Create a formatter and various appenders to attach to the logs. + formatter = new Log.BasicFormatter(); + consoleAppender = new Log.ConsoleAppender(formatter); + dumpAppender = new Log.DumpAppender(formatter); + } + + allBranches.add(this._prefs._branchStr); + // We create a preference observer for all our prefs so they are magically + // reflected if the pref changes after creation. + let setupAppender = (appender, prefName, defaultLevel, findSmallest = false) => { + let observer = newVal => { + let level = Log.Level[newVal] || defaultLevel; + if (findSmallest) { + // We need to find the smallest value from all prefs controlling this appender. + for (let branch of allBranches) { + let lookPrefBranch = new Preferences(branch); + let lookVal = Log.Level[lookPrefBranch.get(prefName)]; + if (lookVal && lookVal < level) { + level = lookVal; + } + } + } + appender.level = level; + } + this._prefs.observe(prefName, observer, this); + this._prefObservers.push([prefName, observer]); + // and call the observer now with the current pref value. + observer(this._prefs.get(prefName)); + return observer; + } + + this._observeConsolePref = setupAppender(consoleAppender, "log.appender.console", Log.Level.Error, true); + this._observeDumpPref = setupAppender(dumpAppender, "log.appender.dump", Log.Level.Error, true); + + // The file appender doesn't get the special singleton behaviour. + let fapp = this._fileAppender = new Log.StorageStreamAppender(formatter); + // the stream gets a default of Debug as the user must go out of there way + // to see the stuff spewed to it. + this._observeStreamPref = setupAppender(fapp, "log.appender.file.level", Log.Level.Debug); + + // now attach the appenders to all our logs. + for (let logName of logNames) { + let log = Log.repository.getLogger(logName); + for (let appender of [fapp, dumpAppender, consoleAppender]) { + log.addAppender(appender); + } + } + // and use the first specified log as a "root" for our log. + this._log = Log.repository.getLogger(logNames[0] + ".LogManager"); + }, + + /** + * Cleanup this instance + */ + finalize() { + for (let [name, pref] of this._prefObservers) { + this._prefs.ignore(name, pref, this); + } + this._prefObservers = []; + try { + allBranches.delete(this._prefs._branchStr); + } catch (e) {} + this._prefs = null; + }, + + get _logFileDirectory() { + // At this point we don't allow a custom directory for the logs so + // about:sync-log can be used. We could fix this later if necessary. + return FileUtils.getDir("ProfD", ["weave", "logs"]); + }, + + /** + * Copy an input stream to the named file, doing everything off the main + * thread. + * outputFile is an nsIFile, but is used only for the name. + * Returns a promise that is resolved with the file modification date on + * completion or rejected if there is an error. + */ + _copyStreamToFile: Task.async(function* (inputStream, outputFile) { + // The log data could be large, so we don't want to pass it all in a single + // message, so use BUFFER_SIZE chunks. + const BUFFER_SIZE = 8192; + + // get a binary stream + let binaryStream = Cc['@mozilla.org/binaryinputstream;1'].createInstance(Ci.nsIBinaryInputStream); + binaryStream.setInputStream(inputStream); + // XXX - this somewhat breaks the abstraction of this._logFileDirectory, but + // is necessary until bug 1056442 helps. + let profileDir = Services.dirsvc.get("ProfD", Ci.nsIFile).path; + yield OS.File.makeDir(outputFile.parent.path, { ignoreExisting: true, + from: profileDir }); + let output = yield OS.File.open(outputFile.path, { write: true} ); + try { + while (true) { + let available = binaryStream.available(); + if (!available) { + break; + } + let chunk = binaryStream.readByteArray(Math.min(available, BUFFER_SIZE)); + yield output.write(new Uint8Array(chunk)); + } + } finally { + inputStream.close(); + binaryStream.close(); + yield output.close(); + } + this._log.trace("finished copy to", outputFile.path); + }), + + /** + * Possibly generate a log file for all accumulated log messages and refresh + * the input & output streams. + * Returns a promise that resolves on completion or rejects if the file could + * not be written. + */ + resetFileLog: Task.async(function* (reason) { + try { + let flushToFile; + let reasonPrefix; + switch (reason) { + case this.REASON_SUCCESS: + flushToFile = this._prefs.get("log.appender.file.logOnSuccess", false); + reasonPrefix = "success"; + break; + case this.REASON_ERROR: + flushToFile = this._prefs.get("log.appender.file.logOnError", true); + reasonPrefix = "error"; + break; + default: + throw new Error("Invalid reason"); + } + + // might as well avoid creating an input stream if we aren't going to use it. + if (!flushToFile) { + this._fileAppender.reset(); + return; + } + + let inStream = this._fileAppender.getInputStream(); + this._fileAppender.reset(); + if (inStream) { + this._log.debug("Flushing file log"); + let filename = this.logFilePrefix + "-" + reasonPrefix + "-" + Date.now() + ".txt"; + let file = this._logFileDirectory; + file.append(filename); + this._log.trace("Beginning stream copy to " + file.leafName + ": " + + Date.now()); + try { + yield this._copyStreamToFile(inStream, file); + this._log.trace("onCopyComplete", Date.now()); + } catch (ex) { + this._log.error("Failed to copy log stream to file", ex); + return; + } + // It's not completely clear to markh why we only do log cleanups + // for errors, but for now the Sync semantics have been copied... + // (one theory is that only cleaning up on error makes it less + // likely old error logs would be removed, but that's not true if + // there are occasional errors - let's address this later!) + if (reason == this.REASON_ERROR && !this._cleaningUpFileLogs) { + this._log.trace("Scheduling cleanup."); + // Note we don't return/yield or otherwise wait on this promise - it + // continues in the background + this.cleanupLogs().catch(err => { + this._log.error("Failed to cleanup logs", err); + }); + } + } + } catch (ex) { + this._log.error("Failed to resetFileLog", ex) + reject(ex); + } + }), + + /** + * Finds all logs older than maxErrorAge and deletes them using async I/O. + */ + cleanupLogs: Task.async(function* () { + this._cleaningUpFileLogs = true; + let iterator = new OS.File.DirectoryIterator(this._logFileDirectory.path); + let maxAge = this._prefs.get("log.appender.file.maxErrorAge", DEFAULT_MAX_ERROR_AGE); + let threshold = Date.now() - 1000 * maxAge; + + this._log.debug("Log cleanup threshold time: " + threshold); + yield iterator.forEach(Task.async(function* (entry) { + if (!entry.name.startsWith(this.logFilePrefix + "-")) { + return; + } + try { + // need to call .stat() as the enumerator doesn't give that to us on *nix. + let info = yield OS.File.stat(entry.path); + if (info.lastModificationDate.getTime() >= threshold) { + return; + } + this._log.trace(" > Cleanup removing " + entry.name + + " (" + info.lastModificationDate.getTime() + ")"); + yield OS.File.remove(entry.path); + this._log.trace("Deleted " + entry.name); + } catch (ex) { + this._log.debug("Encountered error trying to clean up old log file " + + entry.name, ex); + } + }.bind(this))); + this._cleaningUpFileLogs = false; + this._log.debug("Done deleting files."); + // This notification is used only for tests. + Services.obs.notifyObservers(null, "services-tests:common:log-manager:cleanup-logs", null); + }), +} diff --git a/services/common/moz.build b/services/common/moz.build index 199077618ee..ef1cf07ac62 100644 --- a/services/common/moz.build +++ b/services/common/moz.build @@ -13,6 +13,7 @@ EXTRA_COMPONENTS += [ EXTRA_JS_MODULES['services-common'] += [ 'hawkclient.js', 'hawkrequest.js', + 'logmanager.js', 'storageservice.js', 'stringbundle.js', 'tokenserverclient.js', diff --git a/services/common/tests/unit/test_load_modules.js b/services/common/tests/unit/test_load_modules.js index 48490a6e227..ca3b4922c76 100644 --- a/services/common/tests/unit/test_load_modules.js +++ b/services/common/tests/unit/test_load_modules.js @@ -4,6 +4,7 @@ const modules = [ "async.js", "bagheeraclient.js", + "logmanager.js", "rest.js", "storageservice.js", "stringbundle.js", diff --git a/services/common/tests/unit/test_logmanager.js b/services/common/tests/unit/test_logmanager.js new file mode 100644 index 00000000000..125190850e9 --- /dev/null +++ b/services/common/tests/unit/test_logmanager.js @@ -0,0 +1,103 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +// NOTE: The sync test_errorhandler_* tests have quite good coverage for +// other aspects of this. + +Cu.import("resource://services-common/logmanager.js"); +Cu.import("resource://gre/modules/Log.jsm"); + +function run_test() { + run_next_test(); +} + +// Returns an array of [consoleAppender, dumpAppender, [fileAppenders]] for +// the specified log. Note that fileAppenders will usually have length=1 +function getAppenders(log) { + let capps = log.appenders.filter(app => app instanceof Log.ConsoleAppender); + equal(capps.length, 1, "should only have one console appender"); + let dapps = log.appenders.filter(app => app instanceof Log.DumpAppender); + equal(dapps.length, 1, "should only have one dump appender"); + let fapps = log.appenders.filter(app => app instanceof Log.StorageStreamAppender); + return [capps[0], dapps[0], fapps]; +} + +// Test that the correct thing happens when no prefs exist for the log manager. +add_task(function* test_noPrefs() { + // tell the log manager to init with a pref branch that doesn't exist. + let lm = new LogManager("no-such-branch.", ["TestLog"], "test"); + + let log = Log.repository.getLogger("TestLog"); + let [capp, dapp, fapps] = getAppenders(log); + // the "dump" and "console" appenders should get Error level + equal(capp.level, Log.Level.Error); + equal(dapp.level, Log.Level.Error); + // and the file (stream) appender gets Dump by default + equal(fapps.length, 1, "only 1 file appender"); + equal(fapps[0].level, Log.Level.Debug); + lm.finalize(); +}); + +// Test that changes to the prefs used by the log manager are updated dynamically. +add_task(function* test_PrefChanges() { + Services.prefs.setCharPref("log-manager.test.log.appender.console", "Trace"); + Services.prefs.setCharPref("log-manager.test.log.appender.dump", "Trace"); + Services.prefs.setCharPref("log-manager.test.log.appender.file.level", "Trace"); + let lm = new LogManager("log-manager.test.", ["TestLog2"], "test"); + + let log = Log.repository.getLogger("TestLog2"); + let [capp, dapp, [fapp]] = getAppenders(log); + equal(capp.level, Log.Level.Trace); + equal(dapp.level, Log.Level.Trace); + equal(fapp.level, Log.Level.Trace); + // adjust the prefs and they should magically be reflected in the appenders. + Services.prefs.setCharPref("log-manager.test.log.appender.console", "Debug"); + Services.prefs.setCharPref("log-manager.test.log.appender.dump", "Debug"); + Services.prefs.setCharPref("log-manager.test.log.appender.file.level", "Debug"); + equal(capp.level, Log.Level.Debug); + equal(dapp.level, Log.Level.Debug); + equal(fapp.level, Log.Level.Debug); + // and invalid values should cause them to fallback to their defaults. + Services.prefs.setCharPref("log-manager.test.log.appender.console", "xxx"); + Services.prefs.setCharPref("log-manager.test.log.appender.dump", "xxx"); + Services.prefs.setCharPref("log-manager.test.log.appender.file.level", "xxx"); + equal(capp.level, Log.Level.Error); + equal(dapp.level, Log.Level.Error); + equal(fapp.level, Log.Level.Debug); + lm.finalize(); +}); + +// Test that the same log used by multiple log managers does the right thing. +add_task(function* test_SharedLogs() { + // create the prefs for the first instance. + Services.prefs.setCharPref("log-manager-1.test.log.appender.console", "Trace"); + Services.prefs.setCharPref("log-manager-1.test.log.appender.dump", "Trace"); + Services.prefs.setCharPref("log-manager-1.test.log.appender.file.level", "Trace"); + let lm1 = new LogManager("log-manager-1.test.", ["TestLog3"], "test"); + + // and the second. + Services.prefs.setCharPref("log-manager-2.test.log.appender.console", "Debug"); + Services.prefs.setCharPref("log-manager-2.test.log.appender.dump", "Debug"); + Services.prefs.setCharPref("log-manager-2.test.log.appender.file.level", "Debug"); + let lm2 = new LogManager("log-manager-2.test.", ["TestLog3"], "test"); + + let log = Log.repository.getLogger("TestLog3"); + let [capp, dapp, fapps] = getAppenders(log); + + // console and dump appenders should be "trace" as it is more verbose than + // "debug" + equal(capp.level, Log.Level.Trace); + equal(dapp.level, Log.Level.Trace); + + // Set the prefs on the -1 branch to "Error" - it should then end up with + // "Debug" from the -2 branch. + Services.prefs.setCharPref("log-manager-1.test.log.appender.console", "Error"); + Services.prefs.setCharPref("log-manager-1.test.log.appender.dump", "Error"); + Services.prefs.setCharPref("log-manager-1.test.log.appender.file.level", "Error"); + + equal(capp.level, Log.Level.Debug); + equal(dapp.level, Log.Level.Debug); + + lm1.finalize(); + lm2.finalize(); +}); diff --git a/services/common/tests/unit/xpcshell.ini b/services/common/tests/unit/xpcshell.ini index d79b3e4e589..9e038c3a894 100644 --- a/services/common/tests/unit/xpcshell.ini +++ b/services/common/tests/unit/xpcshell.ini @@ -29,6 +29,7 @@ skip-if = toolkit == 'gonk' [test_bagheera_client.js] [test_hawkclient.js] [test_hawkrequest.js] +[test_logmanager.js] [test_observers.js] [test_restrequest.js] [test_tokenauthenticatedrequest.js] diff --git a/services/sync/modules/policies.js b/services/sync/modules/policies.js index 4451ca79d98..167dd10dce5 100644 --- a/services/sync/modules/policies.js +++ b/services/sync/modules/policies.js @@ -13,8 +13,7 @@ Cu.import("resource://gre/modules/Log.jsm"); Cu.import("resource://services-sync/constants.js"); Cu.import("resource://services-sync/engines.js"); Cu.import("resource://services-sync/util.js"); -Cu.import("resource://gre/modules/FileUtils.jsm"); -Cu.import("resource://gre/modules/NetUtil.jsm"); +Cu.import("resource://services-common/logmanager.js"); XPCOMUtils.defineLazyModuleGetter(this, "Status", "resource://services-sync/status.js"); @@ -539,9 +538,6 @@ SyncScheduler.prototype = { }, }; -const LOG_PREFIX_SUCCESS = "success-"; -const LOG_PREFIX_ERROR = "error-"; - this.ErrorHandler = function ErrorHandler(service) { this.service = service; this.init(); @@ -574,33 +570,14 @@ ErrorHandler.prototype = { initLogs: function initLogs() { this._log = Log.repository.getLogger("Sync.ErrorHandler"); this._log.level = Log.Level[Svc.Prefs.get("log.logger.service.main")]; - this._cleaningUpFileLogs = false; let root = Log.repository.getLogger("Sync"); root.level = Log.Level[Svc.Prefs.get("log.rootLogger")]; - let formatter = new Log.BasicFormatter(); - let capp = new Log.ConsoleAppender(formatter); - capp.level = Log.Level[Svc.Prefs.get("log.appender.console")]; - root.addAppender(capp); + let logs = ["Sync", "FirefoxAccounts", "Hawk", "Common.TokenServerClient", + "Sync.SyncMigration"]; - let dapp = new Log.DumpAppender(formatter); - dapp.level = Log.Level[Svc.Prefs.get("log.appender.dump")]; - root.addAppender(dapp); - - let fapp = this._logAppender = new Log.StorageStreamAppender(formatter); - fapp.level = Log.Level[Svc.Prefs.get("log.appender.file.level")]; - root.addAppender(fapp); - - // Arrange for a number of other sync-related logs to also go to our - // appenders. - for (let extra of ["FirefoxAccounts", "Hawk", "Common.TokenServerClient", - "Sync.SyncMigration"]) { - let log = Log.repository.getLogger(extra); - for (let appender of [fapp, dapp, capp]) { - log.addAppender(appender); - } - } + this._logManager = new LogManager(Svc.Prefs, logs, "sync"); }, observe: function observe(subject, topic, data) { @@ -625,8 +602,7 @@ ErrorHandler.prototype = { this._log.debug(engine_name + " failed: " + Utils.exceptionStr(exception)); break; case "weave:service:login:error": - this.resetFileLog(Svc.Prefs.get("log.appender.file.logOnError"), - LOG_PREFIX_ERROR); + this.resetFileLog(this._logManager.REASON_ERROR); if (this.shouldReportError()) { this.notifyOnNextTick("weave:ui:login:error"); @@ -641,8 +617,7 @@ ErrorHandler.prototype = { this.service.logout(); } - this.resetFileLog(Svc.Prefs.get("log.appender.file.logOnError"), - LOG_PREFIX_ERROR); + this.resetFileLog(this._logManager.REASON_ERROR); if (this.shouldReportError()) { this.notifyOnNextTick("weave:ui:sync:error"); @@ -668,8 +643,7 @@ ErrorHandler.prototype = { if (Status.service == SYNC_FAILED_PARTIAL) { this._log.debug("Some engines did not sync correctly."); - this.resetFileLog(Svc.Prefs.get("log.appender.file.logOnError"), - LOG_PREFIX_ERROR); + this.resetFileLog(this._logManager.REASON_ERROR); if (this.shouldReportError()) { this.dontIgnoreErrors = false; @@ -677,8 +651,7 @@ ErrorHandler.prototype = { break; } } else { - this.resetFileLog(Svc.Prefs.get("log.appender.file.logOnSuccess"), - LOG_PREFIX_SUCCESS); + this.resetFileLog(this._logManager.REASON_SUCCESS); } this.dontIgnoreErrors = false; this.notifyOnNextTick("weave:ui:sync:finish"); @@ -705,95 +678,22 @@ ErrorHandler.prototype = { Utils.nextTick(this.service.sync, this.service); }, - /** - * Finds all logs older than maxErrorAge and deletes them without tying up I/O. - */ - cleanupLogs: function cleanupLogs() { - let direntries = FileUtils.getDir("ProfD", ["weave", "logs"]).directoryEntries; - let oldLogs = []; - let index = 0; - let threshold = Date.now() - 1000 * Svc.Prefs.get("log.appender.file.maxErrorAge"); - - this._log.debug("Log cleanup threshold time: " + threshold); - while (direntries.hasMoreElements()) { - let logFile = direntries.getNext().QueryInterface(Ci.nsIFile); - if (logFile.lastModifiedTime < threshold) { - this._log.trace(" > Noting " + logFile.leafName + - " for cleanup (" + logFile.lastModifiedTime + ")"); - oldLogs.push(logFile); - } - } - - // Deletes a file from oldLogs each tick until there are none left. - let errorHandler = this; - function deleteFile() { - if (index >= oldLogs.length) { - errorHandler._log.debug("Done deleting files."); - errorHandler._cleaningUpFileLogs = false; - Svc.Obs.notify("weave:service:cleanup-logs"); - return; - } - try { - let file = oldLogs[index]; - file.remove(false); - errorHandler._log.trace("Deleted " + file.leafName + "."); - } catch (ex) { - errorHandler._log._debug("Encountered error trying to clean up old log file '" - + oldLogs[index].leafName + "':" - + Utils.exceptionStr(ex)); - } - index++; - Utils.nextTick(deleteFile); - } - - if (oldLogs.length > 0) { - this._cleaningUpFileLogs = true; - Utils.nextTick(deleteFile); - } else { - this._log.debug("No logs to clean up."); - } - }, - /** * Generate a log file for the sync that just completed * and refresh the input & output streams. * - * @param flushToFile - * the log file to be flushed/reset - * - * @param filenamePrefix - * a value of either LOG_PREFIX_SUCCESS or LOG_PREFIX_ERROR - * to be used as the log filename prefix + * @param reason + * A constant from the LogManager that indicates the reason for the + * reset. */ - resetFileLog: function resetFileLog(flushToFile, filenamePrefix) { - let inStream = this._logAppender.getInputStream(); - this._logAppender.reset(); - if (flushToFile && inStream) { - this._log.debug("Flushing file log."); - try { - let filename = filenamePrefix + Date.now() + ".txt"; - let file = FileUtils.getFile("ProfD", ["weave", "logs", filename]); - let outStream = FileUtils.openFileOutputStream(file); - - this._log.trace("Beginning stream copy to " + file.leafName + ": " + - Date.now()); - NetUtil.asyncCopy(inStream, outStream, function onCopyComplete() { - this._log.trace("onCopyComplete: " + Date.now()); - this._log.trace("Output file timestamp: " + file.lastModifiedTime); - Svc.Obs.notify("weave:service:reset-file-log"); - this._log.trace("Notified: " + Date.now()); - if (filenamePrefix == LOG_PREFIX_ERROR && - !this._cleaningUpFileLogs) { - this._log.trace("Scheduling cleanup."); - Utils.nextTick(this.cleanupLogs, this); - } - }.bind(this)); - } catch (ex) { - Svc.Obs.notify("weave:service:reset-file-log"); - } - } else { + resetFileLog: function resetFileLog(reason) { + let onComplete = () => { Svc.Obs.notify("weave:service:reset-file-log"); - } + this._log.trace("Notified: " + Date.now()); + }; + // Note we do not return the promise here - the caller doesn't need to wait + // for this to complete. + this._logManager.resetFileLog(reason).then(onComplete, onComplete); }, /** diff --git a/services/sync/tests/unit/test_errorhandler.js b/services/sync/tests/unit/test_errorhandler.js index 34e3e6d1baa..a803947ed01 100644 --- a/services/sync/tests/unit/test_errorhandler.js +++ b/services/sync/tests/unit/test_errorhandler.js @@ -14,8 +14,6 @@ Cu.import("resource://gre/modules/FileUtils.jsm"); const FAKE_SERVER_URL = "http://dummy:9000/"; const logsdir = FileUtils.getDir("ProfD", ["weave", "logs"], true); -const LOG_PREFIX_SUCCESS = "success-"; -const LOG_PREFIX_ERROR = "error-"; const PROLONGED_ERROR_DURATION = (Svc.Prefs.get('errorhandler.networkFailureReportTimeout') * 2) * 1000; @@ -1772,8 +1770,7 @@ add_task(function test_sync_engine_generic_fail() { let entries = logsdir.directoryEntries; do_check_true(entries.hasMoreElements()); let logfile = entries.getNext().QueryInterface(Ci.nsILocalFile); - do_check_eq(logfile.leafName.slice(0, LOG_PREFIX_ERROR.length), - LOG_PREFIX_ERROR); + do_check_true(logfile.leafName.startsWith("sync-error-"), logfile.leafName); clean(); server.stop(deferred.resolve); @@ -1804,8 +1801,7 @@ add_test(function test_logs_on_sync_error_despite_shouldReportError() { let entries = logsdir.directoryEntries; do_check_true(entries.hasMoreElements()); let logfile = entries.getNext().QueryInterface(Ci.nsILocalFile); - do_check_eq(logfile.leafName.slice(0, LOG_PREFIX_ERROR.length), - LOG_PREFIX_ERROR); + do_check_true(logfile.leafName.startsWith("sync-error-"), logfile.leafName); clean(); run_next_test(); @@ -1832,8 +1828,7 @@ add_test(function test_logs_on_login_error_despite_shouldReportError() { let entries = logsdir.directoryEntries; do_check_true(entries.hasMoreElements()); let logfile = entries.getNext().QueryInterface(Ci.nsILocalFile); - do_check_eq(logfile.leafName.slice(0, LOG_PREFIX_ERROR.length), - LOG_PREFIX_ERROR); + do_check_true(logfile.leafName.startsWith("sync-error-"), logfile.leafName); clean(); run_next_test(); @@ -1867,8 +1862,7 @@ add_task(function test_engine_applyFailed() { let entries = logsdir.directoryEntries; do_check_true(entries.hasMoreElements()); let logfile = entries.getNext().QueryInterface(Ci.nsILocalFile); - do_check_eq(logfile.leafName.slice(0, LOG_PREFIX_ERROR.length), - LOG_PREFIX_ERROR); + do_check_true(logfile.leafName.startsWith("sync-error-"), logfile.leafName); clean(); server.stop(deferred.resolve); diff --git a/services/sync/tests/unit/test_errorhandler_filelog.js b/services/sync/tests/unit/test_errorhandler_filelog.js index cfaedcb70c7..a530b836f5f 100644 --- a/services/sync/tests/unit/test_errorhandler_filelog.js +++ b/services/sync/tests/unit/test_errorhandler_filelog.js @@ -10,8 +10,6 @@ Cu.import("resource://gre/modules/NetUtil.jsm"); Cu.import("resource://gre/modules/Services.jsm"); const logsdir = FileUtils.getDir("ProfD", ["weave", "logs"], true); -const LOG_PREFIX_SUCCESS = "success-"; -const LOG_PREFIX_ERROR = "error-"; // Delay to wait before cleanup, to allow files to age. // This is so large because the file timestamp granularity is per-second, and @@ -32,6 +30,7 @@ function setLastSync(lastSyncValue) { function run_test() { initTestLogging("Trace"); + Log.repository.getLogger("Sync.LogManager").level = Log.Level.Trace; Log.repository.getLogger("Sync.Service").level = Log.Level.Trace; Log.repository.getLogger("Sync.SyncScheduler").level = Log.Level.Trace; Log.repository.getLogger("Sync.ErrorHandler").level = Log.Level.Trace; @@ -41,7 +40,7 @@ function run_test() { add_test(function test_noOutput() { // Ensure that the log appender won't print anything. - errorHandler._logAppender.level = Log.Level.Fatal + 1; + errorHandler._logManager._fileAppender.level = Log.Level.Fatal + 1; // Clear log output from startup. Svc.Prefs.set("log.appender.file.logOnSuccess", false); @@ -53,7 +52,7 @@ add_test(function test_noOutput() { Svc.Obs.add("weave:service:reset-file-log", function onResetFileLog() { Svc.Obs.remove("weave:service:reset-file-log", onResetFileLog); - errorHandler._logAppender.level = Log.Level.Trace; + errorHandler._logManager._fileAppender.level = Log.Level.Trace; Svc.Prefs.resetBranch(""); run_next_test(); }); @@ -109,8 +108,7 @@ add_test(function test_logOnSuccess_true() { do_check_true(entries.hasMoreElements()); let logfile = entries.getNext().QueryInterface(Ci.nsILocalFile); do_check_eq(logfile.leafName.slice(-4), ".txt"); - do_check_eq(logfile.leafName.slice(0, LOG_PREFIX_SUCCESS.length), - LOG_PREFIX_SUCCESS); + do_check_true(logfile.leafName.startsWith("sync-success-"), logfile.leafName); do_check_false(entries.hasMoreElements()); // Ensure the log message was actually written to file. @@ -162,6 +160,13 @@ add_test(function test_sync_error_logOnError_true() { const MESSAGE = "this WILL show up"; log.info(MESSAGE); + // We need to wait until the log cleanup started by this test is complete + // or the next test will fail as it is ongoing. + Svc.Obs.add("services-tests:common:log-manager:cleanup-logs", function onCleanupLogs() { + Svc.Obs.remove("services-tests:common:log-manager:cleanup-logs", onCleanupLogs); + run_next_test(); + }); + Svc.Obs.add("weave:service:reset-file-log", function onResetFileLog() { Svc.Obs.remove("weave:service:reset-file-log", onResetFileLog); @@ -170,8 +175,7 @@ add_test(function test_sync_error_logOnError_true() { do_check_true(entries.hasMoreElements()); let logfile = entries.getNext().QueryInterface(Ci.nsILocalFile); do_check_eq(logfile.leafName.slice(-4), ".txt"); - do_check_eq(logfile.leafName.slice(0, LOG_PREFIX_ERROR.length), - LOG_PREFIX_ERROR); + do_check_true(logfile.leafName.startsWith("sync-error-"), logfile.leafName); do_check_false(entries.hasMoreElements()); // Ensure the log message was actually written to file. @@ -188,7 +192,6 @@ add_test(function test_sync_error_logOnError_true() { } Svc.Prefs.resetBranch(""); - run_next_test(); }); }); @@ -224,6 +227,13 @@ add_test(function test_login_error_logOnError_true() { const MESSAGE = "this WILL show up"; log.info(MESSAGE); + // We need to wait until the log cleanup started by this test is complete + // or the next test will fail as it is ongoing. + Svc.Obs.add("services-tests:common:log-manager:cleanup-logs", function onCleanupLogs() { + Svc.Obs.remove("services-tests:common:log-manager:cleanup-logs", onCleanupLogs); + run_next_test(); + }); + Svc.Obs.add("weave:service:reset-file-log", function onResetFileLog() { Svc.Obs.remove("weave:service:reset-file-log", onResetFileLog); @@ -232,8 +242,7 @@ add_test(function test_login_error_logOnError_true() { do_check_true(entries.hasMoreElements()); let logfile = entries.getNext().QueryInterface(Ci.nsILocalFile); do_check_eq(logfile.leafName.slice(-4), ".txt"); - do_check_eq(logfile.leafName.slice(0, LOG_PREFIX_ERROR.length), - LOG_PREFIX_ERROR); + do_check_true(logfile.leafName.startsWith("sync-error-"), logfile.leafName); do_check_false(entries.hasMoreElements()); // Ensure the log message was actually written to file. @@ -250,7 +259,6 @@ add_test(function test_login_error_logOnError_true() { } Svc.Prefs.resetBranch(""); - run_next_test(); }); }); @@ -273,7 +281,7 @@ add_test(function test_logErrorCleanup_age() { _("Making some files."); for (let i = 0; i < numLogs; i++) { let now = Date.now(); - let filename = LOG_PREFIX_ERROR + now + "" + i + ".txt"; + let filename = "sync-error-" + now + "" + i + ".txt"; let newLog = FileUtils.getFile("ProfD", ["weave", "logs", filename]); let foStream = FileUtils.openFileOutputStream(newLog); foStream.write(errString, errString.length); @@ -282,8 +290,8 @@ add_test(function test_logErrorCleanup_age() { oldLogs.push(newLog.leafName); } - Svc.Obs.add("weave:service:cleanup-logs", function onCleanupLogs() { - Svc.Obs.remove("weave:service:cleanup-logs", onCleanupLogs); + Svc.Obs.add("services-tests:common:log-manager:cleanup-logs", function onCleanupLogs() { + Svc.Obs.remove("services-tests:common:log-manager:cleanup-logs", onCleanupLogs); // Only the newest created log file remains. let entries = logsdir.directoryEntries; From 535957d100595d51c9132393ca40d9ce8a9938dd Mon Sep 17 00:00:00 2001 From: Mark Hammond Date: Wed, 25 Feb 2015 18:54:59 +1100 Subject: [PATCH 04/21] Bug 1131412 - implement a scheduler for the readinglist. r=adw --- browser/components/readinglist/Scheduler.jsm | 338 ++++++++++++++++++ browser/components/readinglist/moz.build | 6 + .../readinglist/test/xpcshell/head.js | 7 + .../test/xpcshell/test_scheduler.js | 143 ++++++++ .../readinglist/test/xpcshell/xpcshell.ini | 5 + 5 files changed, 499 insertions(+) create mode 100644 browser/components/readinglist/Scheduler.jsm create mode 100644 browser/components/readinglist/test/xpcshell/head.js create mode 100644 browser/components/readinglist/test/xpcshell/test_scheduler.js create mode 100644 browser/components/readinglist/test/xpcshell/xpcshell.ini diff --git a/browser/components/readinglist/Scheduler.jsm b/browser/components/readinglist/Scheduler.jsm new file mode 100644 index 00000000000..8acfc13dbab --- /dev/null +++ b/browser/components/readinglist/Scheduler.jsm @@ -0,0 +1,338 @@ +/* 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/. */ + +"use strict;" + +const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components; + +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); +Cu.import("resource://gre/modules/Services.jsm"); + + +XPCOMUtils.defineLazyModuleGetter(this, 'LogManager', + 'resource://services-common/logmanager.js'); + +XPCOMUtils.defineLazyModuleGetter(this, 'Log', + 'resource://gre/modules/Log.jsm'); + +XPCOMUtils.defineLazyModuleGetter(this, 'Preferences', + 'resource://gre/modules/Preferences.jsm'); + +XPCOMUtils.defineLazyModuleGetter(this, 'setTimeout', + 'resource://gre/modules/Timer.jsm'); +XPCOMUtils.defineLazyModuleGetter(this, 'clearTimeout', + 'resource://gre/modules/Timer.jsm'); + +Cu.import('resource://gre/modules/Task.jsm'); + +this.EXPORTED_SYMBOLS = ["ReadingListScheduler"]; + +// A list of "external" observer topics that may cause us to change when we +// sync. +const OBSERVERS = [ + // We don't sync when offline and restart when online. + "network:offline-status-changed", + // FxA notifications also cause us to check if we should sync. + "fxaccounts:onverified", + // When something notices a local change to an item. + "readinglist:item-changed", + // some notifications the engine might send if we have been requested to backoff. + "readinglist:backoff-requested", + // request to sync now + "readinglist:user-sync", + +]; + +///////// A temp object until we get our "engine" +let engine = { + ERROR_AUTHENTICATION: "authentication error", + sync: Task.async(function* () { + }), +} + +let prefs = new Preferences("readinglist.scheduler."); + +// A helper to manage our interval values. +let intervals = { + // Getters for our intervals. + _fixupIntervalPref(prefName, def) { + // All pref values are seconds, but we return ms. + return prefs.get(prefName, def) * 1000; + }, + + // How long after startup do we do an initial sync? + get initial() this._fixupIntervalPref("initial", 20), // 20 seconds. + // Every interval after the first. + get schedule() this._fixupIntervalPref("schedule", 2 * 60 * 60), // 2 hours + // After we've been told an item has changed + get dirty() this._fixupIntervalPref("dirty", 2 * 60), // 2 mins + // After an error + get retry() this._fixupIntervalPref("retry", 2 * 60), // 2 mins +}; + +// This is the implementation, but it's not exposed directly. +function InternalScheduler() { + // oh, I don't know what logs yet - let's guess! + let logs = ["readinglist", "FirefoxAccounts", "browserwindow.syncui"]; + this._logManager = new LogManager("readinglist.", logs, "readinglist"); + this.log = Log.repository.getLogger("readinglist.scheduler"); + this.log.info("readinglist scheduler created.") + this.state = this.STATE_OK; + + // don't this.init() here, but instead at the module level - tests want to + // add hooks before it is called. +} + +InternalScheduler.prototype = { + // When the next scheduled sync should happen. If we can sync, there will + // be a timer set to fire then. If we can't sync there will not be a timer, + // but it will be set to fire then as soon as we can. + _nextScheduledSync: null, + // The time when the most-recent "backoff request" expires - we will never + // schedule a new timer before this. + _backoffUntil: 0, + // Our current timer. + _timer: null, + // Our timer fires a promise - _timerRunning is true until it resolves or + // rejects. + _timerRunning: false, + // Our sync engine - XXX - maybe just a callback? + _engine: engine, + + // Our state variable and constants. + state: null, + STATE_OK: "ok", + STATE_ERROR_AUTHENTICATION: "authentication error", + STATE_ERROR_OTHER: "other error", + + init() { + this.log.info("scheduler initialzing"); + this._observe = this.observe.bind(this); + for (let notification of OBSERVERS) { + Services.obs.addObserver(this._observe, notification, false); + } + this._nextScheduledSync = Date.now() + intervals.initial; + this._setupTimer(); + }, + + // Note: only called by tests. + finalize() { + this.log.info("scheduler finalizing"); + this._clearTimer(); + for (let notification of OBSERVERS) { + Services.obs.removeObserver(this._observe, notification); + } + this._observe = null; + }, + + observe(subject, topic, data) { + this.log.debug("observed ${}", topic); + switch (topic) { + case "readinglist:backoff-requested": { + // The subject comes in as a string, a number of seconds. + let interval = parseInt(data, 10); + if (isNaN(interval)) { + this.log.warn("Backoff request had non-numeric value", data); + return; + } + this.log.info("Received a request to backoff for ${} seconds", interval); + this._backoffUntil = Date.now() + interval * 1000; + this._maybeReschedule(0); + break; + } + case "readinglist:local:dirty": + this._maybeReschedule(intervals.dirty); + break; + case "readinglist:user-sync": + this._syncNow(); + break; + case "fxaccounts:onverified": + // If we were in an authentication error state, reset that now. + if (this.state == this.STATE_ERROR_AUTHENTICATION) { + this.state = this.STATE_OK; + } + break; + + // The rest just indicate that now is probably a good time to check if + // we can sync as normal using whatever schedule was previously set. + default: + break; + } + // When observers fire we ignore the current sync error state as the + // notification may indicate it's been resolved. + this._setupTimer(true); + }, + + // Is the current error state such that we shouldn't schedule a new sync. + _isBlockedOnError() { + // this needs more thought... + return this.state == this.STATE_ERROR_AUTHENTICATION; + }, + + // canSync indicates if we can currently sync. + _canSync(ignoreBlockingErrors = false) { + if (Services.io.offline) { + this.log.info("canSync=false - we are offline"); + return false; + } + if (!ignoreBlockingErrors && this._isBlockedOnError()) { + this.log.info("canSync=false - we are in a blocked error state", this.state); + return false; + } + this.log.info("canSync=true"); + return true; + }, + + // _setupTimer checks the current state and the environment to see when + // we should next sync and creates the timer with the appropriate delay. + _setupTimer(ignoreBlockingErrors = false) { + if (!this._canSync(ignoreBlockingErrors)) { + this._clearTimer(); + return; + } + if (this._timer) { + let when = new Date(this._nextScheduledSync); + let delay = this._nextScheduledSync - Date.now(); + this.log.info("checkStatus - already have a timer - will fire in ${delay}ms at ${when}", + {delay, when}); + return; + } + if (this._timerRunning) { + this.log.info("checkStatus - currently syncing"); + return; + } + // no timer and we can sync, so start a new one. + let now = Date.now(); + let delay = Math.max(0, this._nextScheduledSync - now); + let when = new Date(now + delay); + this.log.info("next scheduled sync is in ${delay}ms (at ${when})", {delay, when}) + this._timer = this._setTimeout(delay); + }, + + // Something (possibly naively) thinks the next sync should happen in + // delay-ms. If there's a backoff in progress, ignore the requested delay + // and use the back-off. If there's already a timer scheduled for earlier + // than delay, let the earlier timer remain. Otherwise, use the requested + // delay. + _maybeReschedule(delay) { + // If there's no delay specified and there's nothing currently scheduled, + // it means a backoff request while the sync is actually running - there's + // no need to do anything here - the next reschedule after the sync + // completes will take the backoff into account. + if (!delay && !this._nextScheduledSync) { + this.log.debug("_maybeReschedule ignoring a backoff request while running"); + return; + } + let now = Date.now(); + if (!this._nextScheduledSync) { + this._nextScheduledSync = now + delay; + } + // If there is something currently scheduled before the requested delay, + // keep the existing value (eg, if we have a timer firing in 1 second, and + // get a "dirty" notification that says we should sync in 2 seconds, we + // keep the 1 second value) + this._nextScheduledSync = Math.min(this._nextScheduledSync, now + delay); + // But we still need to honor a backoff. + this._nextScheduledSync = Math.max(this._nextScheduledSync, this._backoffUntil); + // And always create a new timer next time _setupTimer is called. + this._clearTimer(); + }, + + // callback for when the timer fires. + _doSync() { + this.log.debug("starting sync"); + this._timer = null; + this._timerRunning = true; + // flag that there's no new schedule yet, so a request coming in while + // we are running does the right thing. + this._nextScheduledSync = 0; + Services.obs.notifyObservers(null, "readinglist:sync:start", null); + this._engine.sync().then(() => { + this.log.info("Sync completed successfully"); + // Write a pref in the same format used to services/sync to indicate + // the last success. + prefs.set("lastSync", new Date().toString()); + this.state = this.STATE_OK; + this._logManager.resetFileLog(this._logManager.REASON_SUCCESS); + Services.obs.notifyObservers(null, "readinglist:sync:finish", null); + return intervals.schedule; + }).catch(err => { + this.log.error("Sync failed", err); + // XXX - how to detect an auth error? + this.state = err == this._engine.ERROR_AUTHENTICATION ? + this.STATE_ERROR_AUTHENTICATION : this.STATE_ERROR_OTHER; + this._logManager.resetFileLog(this._logManager.REASON_ERROR); + Services.obs.notifyObservers(null, "readinglist:sync:error", null); + return intervals.retry; + }).then(nextDelay => { + this._timerRunning = false; + // ensure a new timer is setup for the appropriate next time. + this._maybeReschedule(nextDelay); + this._setupTimer(); + this._onAutoReschedule(); // just for tests... + }).catch(err => { + // We should never get here, but better safe than sorry... + this.log.error("Failed to reschedule after sync completed", err); + }); + }, + + _clearTimer() { + if (this._timer) { + clearTimeout(this._timer); + this._timer = null; + } + }, + + // A function to "sync now", but not allowing it to start if one is + // already running, and rescheduling the timer. + // To call this, just send a "readinglist:user-sync" notification. + _syncNow() { + if (this._timerRunning) { + this.log.info("syncNow() but a sync is already in progress - ignoring"); + return; + } + this._clearTimer(); + this._doSync(); + }, + + // A couple of hook-points for testing. + // xpcshell tests hook this so (a) it can check the expected delay is set + // and (b) to ignore the delay and set a timeout of 0 so the test is fast. + _setTimeout(delay) { + return setTimeout(() => this._doSync(), delay); + }, + // xpcshell tests hook this to make sure that the correct state etc exist + // after a sync has been completed and a new timer created (or not). + _onAutoReschedule() {}, +}; + +let internalScheduler = new InternalScheduler(); +internalScheduler.init(); + +// The public interface into this module is tiny, so a simple object that +// delegates to the implementation. +let ReadingListScheduler = { + get STATE_OK() internalScheduler.STATE_OK, + get STATE_ERROR_AUTHENTICATION() internalScheduler.STATE_ERROR_AUTHENTICATION, + get STATE_ERROR_OTHER() internalScheduler.STATE_ERROR_OTHER, + + get state() internalScheduler.state, +}; + +// These functions are exposed purely for tests, which manage to grab them +// via a BackstagePass. +function createTestableScheduler() { + // kill the "real" scheduler as we don't want it listening to notifications etc. + if (internalScheduler) { + internalScheduler.finalize(); + internalScheduler = null; + } + // No .init() call - that's up to the tests after hooking. + return new InternalScheduler(); +} + +// mochitests want the internal state of the real scheduler for various things. +function getInternalScheduler() { + return internalScheduler; +} diff --git a/browser/components/readinglist/moz.build b/browser/components/readinglist/moz.build index 0dfdf141706..b02a45bf7d7 100644 --- a/browser/components/readinglist/moz.build +++ b/browser/components/readinglist/moz.build @@ -13,3 +13,9 @@ TESTING_JS_MODULES += [ ] BROWSER_CHROME_MANIFESTS += ['test/browser/browser.ini'] + +XPCSHELL_TESTS_MANIFESTS += ['test/xpcshell/xpcshell.ini'] + +EXTRA_JS_MODULES.readinglist += [ + 'Scheduler.jsm', +] diff --git a/browser/components/readinglist/test/xpcshell/head.js b/browser/components/readinglist/test/xpcshell/head.js new file mode 100644 index 00000000000..caf9f95a955 --- /dev/null +++ b/browser/components/readinglist/test/xpcshell/head.js @@ -0,0 +1,7 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +const {classes: Cc, interfaces: Ci, utils: Cu, results: Cr} = Components; + +Cu.import("resource://gre/modules/XPCOMUtils.jsm"); +Cu.import("resource://gre/modules/Services.jsm"); diff --git a/browser/components/readinglist/test/xpcshell/test_scheduler.js b/browser/components/readinglist/test/xpcshell/test_scheduler.js new file mode 100644 index 00000000000..239b68bb45f --- /dev/null +++ b/browser/components/readinglist/test/xpcshell/test_scheduler.js @@ -0,0 +1,143 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +XPCOMUtils.defineLazyModuleGetter(this, 'setTimeout', + 'resource://gre/modules/Timer.jsm'); + +// Setup logging prefs before importing the scheduler module. +Services.prefs.setCharPref("readinglist.log.appender.dump", "Trace"); + +let {createTestableScheduler} = Cu.import("resource:///modules/readinglist/Scheduler.jsm", {}); +Cu.import("resource://gre/modules/Preferences.jsm"); +Cu.import("resource://gre/modules/Timer.jsm"); + +let prefs = new Preferences("readinglist.scheduler."); + +function promiseObserver(topic) { + return new Promise(resolve => { + let obs = (subject, topic, data) => { + Services.obs.removeObserver(obs, topic); + resolve(data); + } + Services.obs.addObserver(obs, topic, false); + }); +} + +function createScheduler(options) { + // avoid typos in the test and other footguns in the options. + let allowedOptions = ["expectedDelay", "expectNewTimer", "syncFunction"]; + for (let key of Object.keys(options)) { + if (!allowedOptions.includes(key)) { + throw new Error("Invalid option " + key); + } + } + let scheduler = createTestableScheduler(); + // make our hooks + let syncFunction = options.syncFunction || Promise.resolve; + scheduler._engine.sync = syncFunction; + // we expect _setTimeout to be called *twice* - first is the initial sync, + // and there's no need to test the delay used for that. options.expectedDelay + // is to check the *subsequent* timer. + let numCalls = 0; + scheduler._setTimeout = function(delay) { + ++numCalls; + print("Test scheduler _setTimeout call number " + numCalls + " with delay=" + delay); + switch (numCalls) { + case 1: + // this is the first and boring schedule as it initializes - do nothing + // other than return a timer that fires immediately. + return setTimeout(() => scheduler._doSync(), 0); + break; + case 2: + // This is the one we are interested in, so check things. + if (options.expectedDelay) { + // a little slop is OK as it takes a few ms to actually set the timer + ok(Math.abs(options.expectedDelay * 1000 - delay) < 500, [options.expectedDelay * 1000, delay]); + } + // and return a timeout that "never" fires + return setTimeout(() => scheduler._doSync(), 10000000); + break; + default: + // This is unexpected! + ok(false, numCalls); + } + }; + // And a callback made once we've determined the next delay. This is always + // called even if _setTimeout isn't (due to no timer being created) + scheduler._onAutoReschedule = () => { + // Most tests expect a new timer, so this is "opt out" + let expectNewTimer = options.expectNewTimer === undefined ? true : options.expectNewTimer; + ok(expectNewTimer ? scheduler._timer : !scheduler._timer); + } + // calling .init fires things off... + scheduler.init(); + return scheduler; +} + +add_task(function* testSuccess() { + // promises which resolve once we've got all the expected notifications. + let allNotifications = [ + promiseObserver("readinglist:sync:start"), + promiseObserver("readinglist:sync:finish"), + ]; + // New delay should be "as regularly scheduled". + prefs.set("schedule", 100); + let scheduler = createScheduler({expectedDelay: 100}); + yield Promise.all(allNotifications); + scheduler.finalize(); +}); + +add_task(function* testOffline() { + let scheduler = createScheduler({expectNewTimer: false}); + Services.io.offline = true; + ok(!scheduler._canSync(), "_canSync is false when offline.") + ok(!scheduler._timer, "there is no current timer while offline.") + Services.io.offline = false; + ok(scheduler._canSync(), "_canSync is true when online.") + ok(scheduler._timer, "there is a new timer when back online.") + scheduler.finalize(); +}); + +add_task(function* testRetryableError() { + let allNotifications = [ + promiseObserver("readinglist:sync:start"), + promiseObserver("readinglist:sync:error"), + ]; + prefs.set("retry", 10); + let scheduler = createScheduler({ + expectedDelay: 10, + syncFunction: () => Promise.reject("transient"), + }); + yield Promise.all(allNotifications); + scheduler.finalize(); +}); + +add_task(function* testAuthError() { + prefs.set("retry", 10); + // We expect an auth error to result in no new timer (as it's waiting for + // some indication it can proceed), but with the next delay being a normal + // "retry" interval (so when we can proceed it is probably already stale, so + // is effectively "immediate") + let scheduler = createScheduler({ + expectedDelay: 10, + syncFunction: () => { + return Promise.reject(ReadingListScheduler._engine.ERROR_AUTHENTICATION); + }, + expectNewTimer: false + }); + // XXX - TODO - send an observer that "unblocks" us and ensure we actually + // do unblock. + scheduler.finalize(); +}); + +add_task(function* testBackoff() { + let scheduler = createScheduler({expectedDelay: 1000}); + Services.obs.notifyObservers(null, "readinglist:backoff-requested", 1000); + // XXX - this needs a little love as nothing checks createScheduler actually + // made the checks we think it does. + scheduler.finalize(); +}); + +function run_test() { + run_next_test(); +} diff --git a/browser/components/readinglist/test/xpcshell/xpcshell.ini b/browser/components/readinglist/test/xpcshell/xpcshell.ini new file mode 100644 index 00000000000..e204dde472e --- /dev/null +++ b/browser/components/readinglist/test/xpcshell/xpcshell.ini @@ -0,0 +1,5 @@ +[DEFAULT] +head = head.js +firefox-appdir = browser + +[test_scheduler.js] From fd83b22816b18d372721791d6b1098fafaa13c35 Mon Sep 17 00:00:00 2001 From: Michael Lopez Date: Mon, 23 Feb 2015 17:07:00 +0100 Subject: [PATCH 05/21] Bug 340432 - Add the ability to cancel site permission changes. r=Unfocused --- browser/components/preferences/permissions.js | 239 +++++++----- .../components/preferences/permissions.xul | 16 +- .../tests/browser_cookies_exceptions.js | 343 +++++++++--------- .../browser/preferences/permissions.dtd | 8 +- 4 files changed, 342 insertions(+), 264 deletions(-) diff --git a/browser/components/preferences/permissions.js b/browser/components/preferences/permissions.js index 0c0b6620a49..219ab8dfe1c 100644 --- a/browser/components/preferences/permissions.js +++ b/browser/components/preferences/permissions.js @@ -3,12 +3,14 @@ * 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/. */ +Components.utils.import("resource://gre/modules/Services.jsm"); + const nsIPermissionManager = Components.interfaces.nsIPermissionManager; const nsICookiePermission = Components.interfaces.nsICookiePermission; const NOTIFICATION_FLUSH_PERMISSIONS = "flush-pending-permissions"; -function Permission(host, rawHost, type, capability) +function Permission(host, rawHost, type, capability) { this.host = host; this.rawHost = rawHost; @@ -17,18 +19,19 @@ function Permission(host, rawHost, type, capability) } var gPermissionManager = { - _type : "", - _permissions : [], - _pm : Components.classes["@mozilla.org/permissionmanager;1"] - .getService(Components.interfaces.nsIPermissionManager), - _bundle : null, - _tree : null, - + _type : "", + _permissions : [], + _permissionsToAdd : new Map(), + _permissionsToDelete : new Map(), + _bundle : null, + _tree : null, + _observerRemoved : false, + _view: { _rowCount: 0, - get rowCount() - { - return this._rowCount; + get rowCount() + { + return this._rowCount; }, getCellText: function (aRow, aColumn) { @@ -56,7 +59,7 @@ var gPermissionManager = { return ""; } }, - + _getCapabilityString: function (aCapability) { var stringKey = null; @@ -76,44 +79,47 @@ var gPermissionManager = { } return this._bundle.getString(stringKey); }, - + addPermission: function (aCapability) { var textbox = document.getElementById("url"); var host = textbox.value.replace(/^\s*([-\w]*:\/+)?/, ""); // trim any leading space and scheme try { - var ioService = Components.classes["@mozilla.org/network/io-service;1"] - .getService(Components.interfaces.nsIIOService); - var uri = ioService.newURI("http://"+host, null, null); + var uri = Services.io.newURI("http://"+host, null, null); host = uri.host; } catch(ex) { - var promptService = Components.classes["@mozilla.org/embedcomp/prompt-service;1"] - .getService(Components.interfaces.nsIPromptService); var message = this._bundle.getString("invalidURI"); var title = this._bundle.getString("invalidURITitle"); - promptService.alert(window, title, message); + Services.prompt.alert(window, title, message); return; } var capabilityString = this._getCapabilityString(aCapability); // check whether the permission already exists, if not, add it - var exists = false; + let hostExists = false; + let capabilityExists = false; for (var i = 0; i < this._permissions.length; ++i) { if (this._permissions[i].rawHost == host) { - // Avoid calling the permission manager if the capability settings are - // the same. Otherwise allow the call to the permissions manager to - // update the listbox for us. - exists = this._permissions[i].capability == capabilityString; + hostExists = true; + capabilityExists = this._permissions[i].capability == capabilityString; + if (!capabilityExists) { + this._permissions[i].capability = capabilityString; + } break; } } - - if (!exists) { - host = (host.charAt(0) == ".") ? host.substring(1,host.length) : host; - var uri = ioService.newURI("http://" + host, null, null); - this._pm.add(uri, this._type, aCapability); + + let permissionParams = {host: host, type: this._type, capability: aCapability}; + if (!hostExists) { + this._permissionsToAdd.set(host, permissionParams); + this._addPermission(permissionParams); } + else if (!capabilityExists) { + this._permissionsToAdd.set(host, permissionParams); + this._handleCapabilityChange(); + } + textbox.value = ""; textbox.focus(); @@ -123,14 +129,57 @@ var gPermissionManager = { // enable "remove all" button as needed document.getElementById("removeAllPermissions").disabled = this._permissions.length == 0; }, - + + _removePermission: function(aPermission) + { + this._removePermissionFromList(aPermission.host); + + // If this permission was added during this session, let's remove + // it from the pending adds list to prevent calls to the + // permission manager. + let isNewPermission = this._permissionsToAdd.delete(aPermission.host); + + if (!isNewPermission) { + this._permissionsToDelete.set(aPermission.host, aPermission); + } + + }, + + _handleCapabilityChange: function () + { + // Re-do the sort, if the status changed from Block to Allow + // or vice versa, since if we're sorted on status, we may no + // longer be in order. + if (this._lastPermissionSortColumn.id == "statusCol") { + this._resortPermissions(); + } + this._tree.treeBoxObject.invalidate(); + }, + + _addPermission: function(aPermission) + { + this._addPermissionToList(aPermission); + ++this._view._rowCount; + this._tree.treeBoxObject.rowCountChanged(this._view.rowCount - 1, 1); + // Re-do the sort, since we inserted this new item at the end. + this._resortPermissions(); + }, + + _resortPermissions: function() + { + gTreeUtils.sort(this._tree, this._view, this._permissions, + this._permissionsComparator, + this._lastPermissionSortColumn, + this._lastPermissionSortAscending); + }, + onHostInput: function (aSiteField) { document.getElementById("btnSession").disabled = !aSiteField.value; document.getElementById("btnBlock").disabled = !aSiteField.value; document.getElementById("btnAllow").disabled = !aSiteField.value; }, - + onWindowKeyPress: function (aEvent) { if (aEvent.keyCode == KeyEvent.DOM_VK_ESCAPE) @@ -142,14 +191,14 @@ var gPermissionManager = { if (aEvent.keyCode == KeyEvent.DOM_VK_RETURN) document.getElementById("btnAllow").click(); }, - + onLoad: function () { this._bundle = document.getElementById("bundlePreferences"); var params = window.arguments[0]; this.init(params); }, - + init: function (aParams) { if (this._type) { @@ -159,14 +208,14 @@ var gPermissionManager = { this._type = aParams.permissionType; this._manageCapability = aParams.manageCapability; - + var permissionsText = document.getElementById("permissionsText"); while (permissionsText.hasChildNodes()) permissionsText.removeChild(permissionsText.firstChild); permissionsText.appendChild(document.createTextNode(aParams.introText)); document.title = aParams.windowTitle; - + document.getElementById("btnBlock").hidden = !aParams.blockVisible; document.getElementById("btnSession").hidden = !aParams.sessionVisible; document.getElementById("btnAllow").hidden = !aParams.allowVisible; @@ -182,23 +231,23 @@ var gPermissionManager = { var urlLabel = document.getElementById("urlLabel"); urlLabel.hidden = !urlFieldVisible; - var os = Components.classes["@mozilla.org/observer-service;1"] - .getService(Components.interfaces.nsIObserverService); - os.notifyObservers(null, NOTIFICATION_FLUSH_PERMISSIONS, this._type); - os.addObserver(this, "perm-changed", false); + Services.obs.notifyObservers(null, NOTIFICATION_FLUSH_PERMISSIONS, this._type); + Services.obs.addObserver(this, "perm-changed", false); this._loadPermissions(); - + urlField.focus(); }, - + uninit: function () { - var os = Components.classes["@mozilla.org/observer-service;1"] - .getService(Components.interfaces.nsIObserverService); - os.removeObserver(this, "perm-changed"); + if (!this._observerRemoved) { + Services.obs.removeObserver(this, "perm-changed"); + + this._observerRemoved = true; + } }, - + observe: function (aSubject, aTopic, aData) { if (aTopic == "perm-changed") { @@ -209,14 +258,7 @@ var gPermissionManager = { return; if (aData == "added") { - this._addPermissionToList(permission); - ++this._view._rowCount; - this._tree.treeBoxObject.rowCountChanged(this._view.rowCount - 1, 1); - // Re-do the sort, since we inserted this new item at the end. - gTreeUtils.sort(this._tree, this._view, this._permissions, - this._permissionsComparator, - this._lastPermissionSortColumn, - this._lastPermissionSortAscending); + this._addPermission(permission); } else if (aData == "changed") { for (var i = 0; i < this._permissions.length; ++i) { @@ -225,31 +267,14 @@ var gPermissionManager = { break; } } - // Re-do the sort, if the status changed from Block to Allow - // or vice versa, since if we're sorted on status, we may no - // longer be in order. - if (this._lastPermissionSortColumn.id == "statusCol") { - gTreeUtils.sort(this._tree, this._view, this._permissions, - this._permissionsComparator, - this._lastPermissionSortColumn, - this._lastPermissionSortAscending); - } - this._tree.treeBoxObject.invalidate(); + this._handleCapabilityChange(); } else if (aData == "deleted") { - for (var i = 0; i < this._permissions.length; i++) { - if (this._permissions[i].host == permission.host) { - this._permissions.splice(i, 1); - this._view._rowCount--; - this._tree.treeBoxObject.rowCountChanged(this._view.rowCount - 1, -1); - this._tree.treeBoxObject.invalidate(); - break; - } - } + this._removePermissionFromList(permission); } } }, - + onPermissionSelected: function () { var hasSelection = this._tree.view.selection.count > 0; @@ -257,7 +282,7 @@ var gPermissionManager = { document.getElementById("removePermission").disabled = !hasRows || !hasSelection; document.getElementById("removeAllPermissions").disabled = !hasRows; }, - + onPermissionDeleted: function () { if (!this._view.rowCount) @@ -266,12 +291,12 @@ var gPermissionManager = { gTreeUtils.deleteSelectedItems(this._tree, this._view, this._permissions, removedPermissions); for (var i = 0; i < removedPermissions.length; ++i) { var p = removedPermissions[i]; - this._pm.remove(p.host, p.type); - } + this._removePermission(p); + } document.getElementById("removePermission").disabled = !this._permissions.length; document.getElementById("removeAllPermissions").disabled = !this._permissions.length; }, - + onAllPermissionsDeleted: function () { if (!this._view.rowCount) @@ -280,12 +305,12 @@ var gPermissionManager = { gTreeUtils.deleteAll(this._tree, this._view, this._permissions, removedPermissions); for (var i = 0; i < removedPermissions.length; ++i) { var p = removedPermissions[i]; - this._pm.remove(p.host, p.type); - } + this._removePermission(p); + } document.getElementById("removePermission").disabled = true; document.getElementById("removeAllPermissions").disabled = true; }, - + onPermissionKeyPress: function (aEvent) { if (aEvent.keyCode == KeyEvent.DOM_VK_DELETE @@ -295,7 +320,7 @@ var gPermissionManager = { ) this.onPermissionDeleted(); }, - + _lastPermissionSortColumn: "", _lastPermissionSortAscending: false, _permissionsComparator : function (a, b) @@ -303,19 +328,38 @@ var gPermissionManager = { return a.toLowerCase().localeCompare(b.toLowerCase()); }, - + onPermissionSort: function (aColumn) { - this._lastPermissionSortAscending = gTreeUtils.sort(this._tree, - this._view, + this._lastPermissionSortAscending = gTreeUtils.sort(this._tree, + this._view, this._permissions, aColumn, this._permissionsComparator, - this._lastPermissionSortColumn, + this._lastPermissionSortColumn, this._lastPermissionSortAscending); this._lastPermissionSortColumn = aColumn; }, - + + onApplyChanges: function() + { + // Stop observing permission changes since we are about + // to write out the pending adds/deletes and don't need + // to update the UI + this.uninit(); + + for (let permissionParams of this._permissionsToAdd.values()) { + let uri = Services.io.newURI("http://" + permissionParams.host, null, null); + Services.perms.add(uri, permissionParams.type, permissionParams.capability); + } + + for (let p of this._permissionsToDelete.values()) { + Services.perms.remove(p.host, p.type); + } + + window.close(); + }, + _loadPermissions: function () { this._tree = document.getElementById("permissionsTree"); @@ -323,12 +367,12 @@ var gPermissionManager = { // load permissions into a table var count = 0; - var enumerator = this._pm.enumerator; + var enumerator = Services.perms.enumerator; while (enumerator.hasMoreElements()) { var nextPermission = enumerator.getNext().QueryInterface(Components.interfaces.nsIPermission); this._addPermissionToList(nextPermission); } - + this._view._rowCount = this._permissions.length; // sort and display the table @@ -338,7 +382,7 @@ var gPermissionManager = { // disable "remove all" button if there are none document.getElementById("removeAllPermissions").disabled = this._permissions.length == 0; }, - + _addPermissionToList: function (aPermission) { if (aPermission.type == this._type && @@ -352,9 +396,22 @@ var gPermissionManager = { aPermission.type, capabilityString); this._permissions.push(p); - } + } }, - + + _removePermissionFromList: function (aHost) + { + for (let i = 0; i < this._permissions.length; ++i) { + if (this._permissions[i].host == aHost) { + this._permissions.splice(i, 1); + this._view._rowCount--; + this._tree.treeBoxObject.rowCountChanged(this._view.rowCount - 1, -1); + this._tree.treeBoxObject.invalidate(); + break; + } + } + }, + setHost: function (aHost) { document.getElementById("url").value = aHost; diff --git a/browser/components/preferences/permissions.xul b/browser/components/preferences/permissions.xul index 22ec2f10581..e8c5ba56a04 100644 --- a/browser/components/preferences/permissions.xul +++ b/browser/components/preferences/permissions.xul @@ -61,8 +61,8 @@ - - + +