From f541ef97918f6b5faf442628c96d64c466949602 Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Thu, 28 Jun 2012 10:59:37 +0100 Subject: [PATCH 01/54] bug 764805 - fix handling of src:local style properties in the GDI backend. r=jdaggett --- gfx/thebes/gfxGDIFontList.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/gfx/thebes/gfxGDIFontList.cpp b/gfx/thebes/gfxGDIFontList.cpp index 3c0ac5d492d8..0c766d20b2e6 100644 --- a/gfx/thebes/gfxGDIFontList.cpp +++ b/gfx/thebes/gfxGDIFontList.cpp @@ -749,24 +749,27 @@ gfxGDIFontList::LookupLocalFont(const gfxProxyFontEntry *aProxyEntry, return nsnull; } - // create a new font entry with the proxy entry style characteristics - PRUint16 w = (aProxyEntry->mWeight == 0 ? 400 : aProxyEntry->mWeight); bool isCFF = false; // jtdfix -- need to determine this // use the face name from the lookup font entry, which will be the localized // face name which GDI mapping tables use (e.g. with the system locale set to // Dutch, a fullname of 'Arial Bold' will find a font entry with the face name // 'Arial Vet' which can be used as a key in GDI font lookups). - gfxFontEntry *fe = GDIFontEntry::CreateFontEntry(lookup->Name(), + GDIFontEntry *fe = GDIFontEntry::CreateFontEntry(lookup->Name(), gfxWindowsFontType(isCFF ? GFX_FONT_TYPE_PS_OPENTYPE : GFX_FONT_TYPE_TRUETYPE) /*type*/, - PRUint32(aProxyEntry->mItalic ? NS_FONT_STYLE_ITALIC : NS_FONT_STYLE_NORMAL), - w, aProxyEntry->mStretch, nsnull); + lookup->mItalic ? NS_FONT_STYLE_ITALIC : NS_FONT_STYLE_NORMAL, + lookup->mWeight, aProxyEntry->mStretch, nsnull); if (!fe) return nsnull; fe->mIsUserFont = true; fe->mIsLocalUserFont = true; + + // make the new font entry match the proxy entry style characteristics + fe->mWeight = (aProxyEntry->mWeight == 0 ? 400 : aProxyEntry->mWeight); + fe->mItalic = aProxyEntry->mItalic; + return fe; } From ab65b50cdb313c437e29eae19cbef8e8b8974948 Mon Sep 17 00:00:00 2001 From: Jonathan Kew Date: Thu, 28 Jun 2012 10:59:39 +0100 Subject: [PATCH 02/54] bug 764805 - reftest for style descriptors on src:local font face. r=jdaggett --- layout/reftests/font-face/local-1-ref.html | 3 +- layout/reftests/font-face/local-1.html | 6 ++- .../font-face/local-styled-1-ref.html | 33 +++++++++++++ layout/reftests/font-face/local-styled-1.html | 49 +++++++++++++++++++ layout/reftests/font-face/reftest.list | 3 ++ 5 files changed, 91 insertions(+), 3 deletions(-) create mode 100644 layout/reftests/font-face/local-styled-1-ref.html create mode 100644 layout/reftests/font-face/local-styled-1.html diff --git a/layout/reftests/font-face/local-1-ref.html b/layout/reftests/font-face/local-1-ref.html index bfe5ed45c252..854dee549534 100644 --- a/layout/reftests/font-face/local-1-ref.html +++ b/layout/reftests/font-face/local-1-ref.html @@ -7,7 +7,8 @@ diff --git a/layout/reftests/font-face/local-1.html b/layout/reftests/font-face/local-1.html index c0f7ec9358c7..44c719387204 100644 --- a/layout/reftests/font-face/local-1.html +++ b/layout/reftests/font-face/local-1.html @@ -34,7 +34,8 @@ font-family: "Local"; src: local(Nimbus Sans L), local(NimbusSansL-Regu), local(Helvetica), local(Bitstream Vera Sans), - local(Arial), local(Liberation Sans), local(SwissA); + local(Arial), local(Liberation Sans), local(SwissA), + local(Droid Sans), local(Roboto); font-weight: 100; } @font-face { @@ -42,7 +43,8 @@ src: local(Nimbus Sans L Bold), local(NimbusSansL-Bold), local(Helvetica Bold), local(Helvetica-Bold), local(Bitstream Vera Sans Bold), - local(Arial Bold), local(Liberation Sans Bold), local(SwissA Bold); + local(Arial Bold), local(Liberation Sans Bold), local(SwissA Bold), + local(Droid Sans Bold), local(Roboto Bold); font-weight: normal; } body { font-family: Local, serif } diff --git a/layout/reftests/font-face/local-styled-1-ref.html b/layout/reftests/font-face/local-styled-1-ref.html new file mode 100644 index 000000000000..ff017a49916d --- /dev/null +++ b/layout/reftests/font-face/local-styled-1-ref.html @@ -0,0 +1,33 @@ + + + + + + +
+This serif font should NOT be used below. +
+
+
+These three lines should all use the same font face. +
+
+This line should NOT be bold. +
+
+This line should NOT be italic. +
+ + diff --git a/layout/reftests/font-face/local-styled-1.html b/layout/reftests/font-face/local-styled-1.html new file mode 100644 index 000000000000..a14719a36aef --- /dev/null +++ b/layout/reftests/font-face/local-styled-1.html @@ -0,0 +1,49 @@ + + + + + + +
+This serif font should NOT be used below. +
+
+
+These three lines should all use the same font face. +
+
+This line should NOT be bold. +
+
+This line should NOT be italic. +
+ + diff --git a/layout/reftests/font-face/reftest.list b/layout/reftests/font-face/reftest.list index f79ee28dcd50..e6e483609b16 100644 --- a/layout/reftests/font-face/reftest.list +++ b/layout/reftests/font-face/reftest.list @@ -94,7 +94,10 @@ HTTP(..) == ex-unit-1-dynamic.html ex-unit-1-ref.html # random-if(!cocoaWidget) HTTP(..) == src-format-arabic.html src-format-arabic-aat-ref.html # random-if(cocoaWidget) HTTP(..) == src-format-arabic.html src-format-arabic-ot-ref.html +# bug 769194 - src:local() completely broken on android fails-if(Android) == local-1.html local-1-ref.html +fails-if(Android) == local-styled-1.html local-styled-1-ref.html + HTTP(..) == synthetic-weight-style.html synthetic-weight-style-ref.html HTTP(..) == synthetic-variations.html synthetic-variations-ref.html From 3ea8a0bac46ea26c89031d4cf7b2f98c7087bf72 Mon Sep 17 00:00:00 2001 From: Gian-Carlo Pascutto Date: Thu, 28 Jun 2012 14:57:54 +0200 Subject: [PATCH 03/54] Bug 762620 - Log the actual DailyCallback events. r=blassey --- widget/xpwidgets/nsIdleService.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/widget/xpwidgets/nsIdleService.cpp b/widget/xpwidgets/nsIdleService.cpp index 1b91f4cb5b42..edabceb5c6b7 100644 --- a/widget/xpwidgets/nsIdleService.cpp +++ b/widget/xpwidgets/nsIdleService.cpp @@ -178,6 +178,10 @@ nsIdleServiceDaily::~nsIdleServiceDaily() void nsIdleServiceDaily::DailyCallback(nsITimer* aTimer, void* aClosure) { +#ifdef ANDROID + __android_log_print(ANDROID_LOG_INFO, "IdleService", "DailyCallback running, registering Idle observer"); +#endif + nsIdleServiceDaily* me = static_cast(aClosure); // The one thing we do every day is to start waiting for the user to "have From 4d9a07165931f4da7ced90f73c58513793f6c294 Mon Sep 17 00:00:00 2001 From: Vladimir Vukicevic Date: Thu, 28 Jun 2012 09:42:32 -0400 Subject: [PATCH 04/54] b=766998; fennec profiling broken by webapp intent changes; r=wesj --- mobile/android/base/GeckoApp.java | 2 +- mobile/android/base/GeckoThread.java | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java index b8d6f287722f..c143dd8cb997 100644 --- a/mobile/android/base/GeckoApp.java +++ b/mobile/android/base/GeckoApp.java @@ -2152,7 +2152,7 @@ abstract public class GeckoApp return uri; final String action = intent.getAction(); - if (action.startsWith(ACTION_WEBAPP_PREFIX) || ACTION_BOOKMARK.equals(action)) { + if ((action != null && action.startsWith(ACTION_WEBAPP_PREFIX)) || ACTION_BOOKMARK.equals(action)) { uri = intent.getStringExtra("args"); if (uri != null && uri.startsWith("--url=")) { uri.replace("--url=", ""); diff --git a/mobile/android/base/GeckoThread.java b/mobile/android/base/GeckoThread.java index 7980c71ac0ea..e8d050d58229 100644 --- a/mobile/android/base/GeckoThread.java +++ b/mobile/android/base/GeckoThread.java @@ -73,9 +73,12 @@ public class GeckoThread extends Thread { // find the right intent type final String action = mIntent.getAction(); - String type = action.startsWith(GeckoApp.ACTION_WEBAPP_PREFIX) ? "-webapp" : - GeckoApp.ACTION_BOOKMARK.equals(action) ? "-bookmark" : - null; + String type = null; + + if (action != null && action.startsWith(GeckoApp.ACTION_WEBAPP_PREFIX)) + type = "-webapp"; + else if (GeckoApp.ACTION_BOOKMARK.equals(action)) + type = "-bookmark"; String args = mIntent.getStringExtra("args"); From 8da7931e652baefd091c24459c636de198019ddf Mon Sep 17 00:00:00 2001 From: Saurabh Anand Date: Thu, 28 Jun 2012 03:11:24 +0530 Subject: [PATCH 05/54] Bug 722988 - openLocationLastURL.jsm uses global Private Browsing state to make decisions; r=ehsan --- browser/base/content/openLocation.js | 4 ++- .../test/unit/test_openLocationLastURL.js | 29 ++++++++++------ browser/modules/openLocationLastURL.jsm | 33 +++++++++++++------ 3 files changed, 45 insertions(+), 21 deletions(-) diff --git a/browser/base/content/openLocation.js b/browser/base/content/openLocation.js index 931df95e69ea..f86e273df1cc 100644 --- a/browser/base/content/openLocation.js +++ b/browser/base/content/openLocation.js @@ -7,6 +7,7 @@ var browser; var dialog = {}; var pref = null; +let openLocationModule = {}; try { pref = Components.classes["@mozilla.org/preferences-service;1"] .getService(Components.interfaces.nsIPrefBranch); @@ -14,7 +15,8 @@ try { // not critical, remain silent } -Components.utils.import("resource:///modules/openLocationLastURL.jsm"); +Components.utils.import("resource:///modules/openLocationLastURL.jsm", openLocationModule); +let gOpenLocationLastURL = new openLocationModule.OpenLocationLastURL(window.opener); function onLoad() { diff --git a/browser/components/privatebrowsing/test/unit/test_openLocationLastURL.js b/browser/components/privatebrowsing/test/unit/test_openLocationLastURL.js index 6d4e9f20e20b..dbcbb6946b59 100644 --- a/browser/components/privatebrowsing/test/unit/test_openLocationLastURL.js +++ b/browser/components/privatebrowsing/test/unit/test_openLocationLastURL.js @@ -6,15 +6,19 @@ function run_test_on_service() { - Cu.import("resource:///modules/openLocationLastURL.jsm"); - + let openLocationModule = {}; + // This variable fakes the window required for getting the PB flag + let window = { gPrivateBrowsingUI: { privateWindow: false } }; + Cu.import("resource:///modules/openLocationLastURL.jsm", openLocationModule); + let gOpenLocationLastURL = new openLocationModule.OpenLocationLastURL(window); + function clearHistory() { // simulate clearing the private data Cc["@mozilla.org/observer-service;1"]. getService(Ci.nsIObserverService). notifyObservers(null, "browser:purge-session-history", ""); } - + let pb = Cc[PRIVATEBROWSING_CONTRACT_ID]. getService(Ci.nsIPrivateBrowsingService); let pref = Cc["@mozilla.org/preferences-service;1"]. @@ -24,6 +28,11 @@ function run_test_on_service() do_check_eq(typeof gOpenLocationLastURL, "object"); do_check_eq(gOpenLocationLastURL.value, ""); + function switchPrivateBrowsing(flag) { + pb.privateBrowsingEnabled = flag; + window.gPrivateBrowsingUI.privateWindow = flag; + } + const url1 = "mozilla.org"; const url2 = "mozilla.com"; @@ -40,26 +49,26 @@ function run_test_on_service() do_check_eq(gOpenLocationLastURL.value, ""); gOpenLocationLastURL.value = url2; - pb.privateBrowsingEnabled = true; + switchPrivateBrowsing(true); do_check_eq(gOpenLocationLastURL.value, ""); - - pb.privateBrowsingEnabled = false; + + switchPrivateBrowsing(false); do_check_eq(gOpenLocationLastURL.value, url2); - pb.privateBrowsingEnabled = true; + switchPrivateBrowsing(true); gOpenLocationLastURL.value = url1; do_check_eq(gOpenLocationLastURL.value, url1); - pb.privateBrowsingEnabled = false; + switchPrivateBrowsing(false); do_check_eq(gOpenLocationLastURL.value, url2); - pb.privateBrowsingEnabled = true; + switchPrivateBrowsing(true); gOpenLocationLastURL.value = url1; do_check_neq(gOpenLocationLastURL.value, ""); clearHistory(); do_check_eq(gOpenLocationLastURL.value, ""); - pb.privateBrowsingEnabled = false; + switchPrivateBrowsing(false); do_check_eq(gOpenLocationLastURL.value, ""); } diff --git a/browser/modules/openLocationLastURL.jsm b/browser/modules/openLocationLastURL.jsm index 924bbbfb48fa..e9fb89b803ee 100644 --- a/browser/modules/openLocationLastURL.jsm +++ b/browser/modules/openLocationLastURL.jsm @@ -4,13 +4,13 @@ const LAST_URL_PREF = "general.open_location.last_url"; const nsISupportsString = Components.interfaces.nsISupportsString; +const Ci = Components.interfaces; -var EXPORTED_SYMBOLS = [ "gOpenLocationLastURL" ]; +var EXPORTED_SYMBOLS = [ "OpenLocationLastURL" ]; -let pbSvc = Components.classes["@mozilla.org/privatebrowsing;1"] - .getService(Components.interfaces.nsIPrivateBrowsingService); let prefSvc = Components.classes["@mozilla.org/preferences-service;1"] .getService(Components.interfaces.nsIPrefBranch); +let gOpenLocationLastURLData = ""; let observer = { QueryInterface: function (aIID) { @@ -22,11 +22,12 @@ let observer = { }, observe: function (aSubject, aTopic, aData) { switch (aTopic) { - case "private-browsing": + case "last-pb-context-exited": gOpenLocationLastURLData = ""; break; case "browser:purge-session-history": - gOpenLocationLastURL.reset(); + prefSvc.clearUserPref(LAST_URL_PREF); + gOpenLocationLastURLData = ""; break; } } @@ -34,13 +35,25 @@ let observer = { let os = Components.classes["@mozilla.org/observer-service;1"] .getService(Components.interfaces.nsIObserverService); -os.addObserver(observer, "private-browsing", true); +os.addObserver(observer, "last-pb-context-exited", true); os.addObserver(observer, "browser:purge-session-history", true); -let gOpenLocationLastURLData = ""; -let gOpenLocationLastURL = { + +function OpenLocationLastURL(aWindow) { + this.window = aWindow; +} + +OpenLocationLastURL.prototype = { + isPrivate: function OpenLocationLastURL_isPrivate() { + // Assume not in private browsing mode, unless the browser window is + // in private mode. + if (!this.window || !("gPrivateBrowsingUI" in this.window)) + return false; + + return this.window.gPrivateBrowsingUI.privateWindow; + }, get value() { - if (pbSvc.privateBrowsingEnabled) + if (this.isPrivate()) return gOpenLocationLastURLData; else { try { @@ -54,7 +67,7 @@ let gOpenLocationLastURL = { set value(val) { if (typeof val != "string") val = ""; - if (pbSvc.privateBrowsingEnabled) + if (this.isPrivate()) gOpenLocationLastURLData = val; else { let str = Components.classes["@mozilla.org/supports-string;1"] From fce23dd7f27d7411cf2812cae41e90c6ac2f019c Mon Sep 17 00:00:00 2001 From: Blake Kaplan Date: Thu, 28 Jun 2012 15:40:05 +0200 Subject: [PATCH 06/54] Bug 769248 - Don't overwrite a good BSSID with a bad one. r=gal --- dom/wifi/WifiWorker.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/dom/wifi/WifiWorker.js b/dom/wifi/WifiWorker.js index c16d36176b64..605e708c08d9 100644 --- a/dom/wifi/WifiWorker.js +++ b/dom/wifi/WifiWorker.js @@ -718,9 +718,11 @@ var WifiManager = (function() { fields.state = supplicantStatesMap[fields.state]; // The BSSID field is only valid in the ASSOCIATING and ASSOCIATED - // states. - if (fields.state === "ASSOCIATING" || fields.state == "ASSOCIATED") + // states, except when we "reauth", except this seems to depend on the + // driver, so simply check to make sure that we don't have a null BSSID. + if (fields.BSSID !== "00:00:00:00:00:00") manager.connectionInfo.bssid = fields.BSSID; + notifyStateChange(fields); return true; } From 44659b1a8f0e2117f59650141a47f3368c2e5334 Mon Sep 17 00:00:00 2001 From: Blake Kaplan Date: Thu, 28 Jun 2012 16:14:11 +0200 Subject: [PATCH 07/54] Bug 769265 - Fix these calls. r=bent --- content/base/src/nsFrameMessageManager.cpp | 2 +- dom/workers/WorkerPrivate.cpp | 2 +- dom/workers/WorkerScope.cpp | 2 +- js/xpconnect/loader/mozJSComponentLoader.cpp | 2 +- js/xpconnect/shell/xpcshell.cpp | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/content/base/src/nsFrameMessageManager.cpp b/content/base/src/nsFrameMessageManager.cpp index bddaf537f028..d34199b8ebfc 100644 --- a/content/base/src/nsFrameMessageManager.cpp +++ b/content/base/src/nsFrameMessageManager.cpp @@ -257,7 +257,7 @@ NS_IMETHODIMP nsFrameMessageManager::Dump(const nsAString& aStr) { #ifdef ANDROID - __android_log_print(ANDROID_LOG_INFO, "Gecko", NS_ConvertUTF16toUTF8(aStr).get()); + __android_log_print(ANDROID_LOG_INFO, "Gecko", "%s", NS_ConvertUTF16toUTF8(aStr).get()); #endif fputs(NS_ConvertUTF16toUTF8(aStr).get(), stdout); fflush(stdout); diff --git a/dom/workers/WorkerPrivate.cpp b/dom/workers/WorkerPrivate.cpp index 8ebb1a86e7ea..41585321c4b3 100644 --- a/dom/workers/WorkerPrivate.cpp +++ b/dom/workers/WorkerPrivate.cpp @@ -1233,7 +1233,7 @@ public: if (!logged) { NS_ConvertUTF16toUTF8 msg(aMessage); #ifdef ANDROID - __android_log_print(ANDROID_LOG_INFO, "Gecko", msg.get()); + __android_log_print(ANDROID_LOG_INFO, "Gecko", "%s", msg.get()); #endif fputs(msg.get(), stderr); fflush(stderr); diff --git a/dom/workers/WorkerScope.cpp b/dom/workers/WorkerScope.cpp index de1f4e4238dc..5c35bda9d96e 100644 --- a/dom/workers/WorkerScope.cpp +++ b/dom/workers/WorkerScope.cpp @@ -542,7 +542,7 @@ private: } #ifdef ANDROID - __android_log_print(ANDROID_LOG_INFO, "Gecko", buffer.ptr()); + __android_log_print(ANDROID_LOG_INFO, "Gecko", "%s", buffer.ptr()); #endif fputs(buffer.ptr(), stdout); fflush(stdout); diff --git a/js/xpconnect/loader/mozJSComponentLoader.cpp b/js/xpconnect/loader/mozJSComponentLoader.cpp index 86ec5296c44a..8ff2f0d6fa2d 100644 --- a/js/xpconnect/loader/mozJSComponentLoader.cpp +++ b/js/xpconnect/loader/mozJSComponentLoader.cpp @@ -176,7 +176,7 @@ Dump(JSContext *cx, unsigned argc, jsval *vp) NS_ConvertUTF16toUTF8 utf8str(reinterpret_cast(chars)); #ifdef ANDROID - __android_log_print(ANDROID_LOG_INFO, "Gecko", utf8str.get()); + __android_log_print(ANDROID_LOG_INFO, "Gecko", "%s", utf8str.get()); #endif fputs(utf8str.get(), stdout); fflush(stdout); diff --git a/js/xpconnect/shell/xpcshell.cpp b/js/xpconnect/shell/xpcshell.cpp index 9757f415f7f9..b0873c5b2823 100644 --- a/js/xpconnect/shell/xpcshell.cpp +++ b/js/xpconnect/shell/xpcshell.cpp @@ -423,7 +423,7 @@ Dump(JSContext *cx, unsigned argc, jsval *vp) return false; #ifdef ANDROID - __android_log_print(ANDROID_LOG_INFO, "Gecko", bytes.ptr()); + __android_log_print(ANDROID_LOG_INFO, "Gecko", "%s", bytes.ptr()); #endif fputs(bytes.ptr(), gOutFile); fflush(gOutFile); From af83f120733b6abd18f93cbfa19ece85413b687c Mon Sep 17 00:00:00 2001 From: Benoit Girard Date: Wed, 20 Jun 2012 19:22:02 -0400 Subject: [PATCH 08/54] Bug 707308 - Support dynamic stack labels for profile. r=jmuizelaar --HG-- extra : rebase_source : 6960978d26a056ab2d581e23b024e3a2f5bc3e60 --- tools/profiler/TableTicker.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/profiler/TableTicker.cpp b/tools/profiler/TableTicker.cpp index d2bb98758049..d9e885c45c78 100644 --- a/tools/profiler/TableTicker.cpp +++ b/tools/profiler/TableTicker.cpp @@ -298,6 +298,13 @@ public: b.DefineProperty(sample, "frames", frames); b.ArrayPush(samples, sample); break; + case 'r': + { + if (sample) { + b.DefineProperty(sample, "responsiveness", entry.mTagFloat); + } + } + break; case 'c': case 'l': { From e74fa60a3056bc97ef6c42eb6b8aef097ab32e4c Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 22:56:55 -0400 Subject: [PATCH 09/54] Bug 722868 - Part 1: Add the deleteTemporaryPrivateFileWhenPossible API; r=bzbarsky --- .../exthandler/nsExternalHelperAppService.cpp | 22 ++++++++++++++----- .../exthandler/nsExternalHelperAppService.h | 5 +++++ .../nsIExternalHelperAppService.idl | 7 +++++- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index c78569dedaa6..5ab6698c0ad7 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -923,7 +923,10 @@ NS_IMETHODIMP nsExternalHelperAppService::GetApplicationDescription(const nsACSt // Methods related to deleting temporary files on exit ////////////////////////////////////////////////////////////////////////////////////////////////////// -NS_IMETHODIMP nsExternalHelperAppService::DeleteTemporaryFileOnExit(nsIFile * aTemporaryFile) +/* static */ +nsresult +nsExternalHelperAppService::DeleteTemporaryFileHelper(nsIFile * aTemporaryFile, + nsCOMArray &aFileList) { bool isFile = false; @@ -931,14 +934,23 @@ NS_IMETHODIMP nsExternalHelperAppService::DeleteTemporaryFileOnExit(nsIFile * aT aTemporaryFile->IsFile(&isFile); if (!isFile) return NS_OK; - if (mInPrivateBrowsing) - mTemporaryPrivateFilesList.AppendObject(aTemporaryFile); - else - mTemporaryFilesList.AppendObject(aTemporaryFile); + aFileList.AppendObject(localFile); return NS_OK; } +NS_IMETHODIMP +nsExternalHelperAppService::DeleteTemporaryFileOnExit(nsIFile* aTemporaryFile) +{ + return DeleteTemporaryFileHelper(aTemporaryFile, mTemporaryFilesList); +} + +NS_IMETHODIMP +nsExternalHelperAppService::DeleteTemporaryPrivateFileWhenPossible(nsIFile* aTemporaryFile) +{ + return DeleteTemporaryFileHelper(aTemporaryFile, mTemporaryPrivateFilesList); +} + void nsExternalHelperAppService::FixFilePermissions(nsIFile* aFile) { // This space intentionally left blank diff --git a/uriloader/exthandler/nsExternalHelperAppService.h b/uriloader/exthandler/nsExternalHelperAppService.h index 59505ba43b30..6e2d83ff8817 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.h +++ b/uriloader/exthandler/nsExternalHelperAppService.h @@ -172,6 +172,11 @@ protected: * Helper function for ExpungeTemporaryFiles and ExpungeTemporaryPrivateFiles */ static void ExpungeTemporaryFilesHelper(nsCOMArray &fileList); + /** + * Helper function for DeleteTemporaryFileOnExit and DeleteTemporaryPrivateFileWhenPossible + */ + static nsresult DeleteTemporaryFileHelper(nsIFile* aTemporaryFile, + nsCOMArray &aFileList); /** * Functions related to the tempory file cleanup service provided by * nsExternalHelperAppService diff --git a/uriloader/exthandler/nsIExternalHelperAppService.idl b/uriloader/exthandler/nsIExternalHelperAppService.idl index e5ee8b7549e0..c0066ec31a36 100644 --- a/uriloader/exthandler/nsIExternalHelperAppService.idl +++ b/uriloader/exthandler/nsIExternalHelperAppService.idl @@ -53,7 +53,7 @@ interface nsIExternalHelperAppService : nsISupports * This is a private interface shared between external app handlers and the platform specific * external helper app service */ -[scriptable, uuid(d0b5d7d3-9565-403d-9fb5-e5089c4567c6)] +[scriptable, uuid(6613e2e7-feab-4e3a-bb1f-b03200d544ec)] interface nsPIExternalAppLauncher : nsISupports { /** @@ -62,6 +62,11 @@ interface nsPIExternalAppLauncher : nsISupports * @param aTemporaryFile A temporary file we should delete on exit. */ void deleteTemporaryFileOnExit(in nsIFile aTemporaryFile); + /** + * Delete a temporary file created inside private browsing mode when + * the private browsing mode has ended. + */ + void deleteTemporaryPrivateFileWhenPossible(in nsIFile aTemporaryFile); }; /** From 3dfe40b2024b60a5883bac2703b9d8e1cbe4c42e Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:01:35 -0400 Subject: [PATCH 10/54] Bug 722868 - Part 2: Make the download manager aware of the deleteTemporaryPrivateFileWhenPossible API; r=gavin --- toolkit/components/downloads/nsDownloadManager.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/toolkit/components/downloads/nsDownloadManager.cpp b/toolkit/components/downloads/nsDownloadManager.cpp index f550acfca3e5..7c5cbec752cf 100644 --- a/toolkit/components/downloads/nsDownloadManager.cpp +++ b/toolkit/components/downloads/nsDownloadManager.cpp @@ -2879,8 +2879,13 @@ nsDownload::OpenWithApplication() // Even if we are unable to get this service we return the result // of LaunchWithFile() which makes more sense. - if (appLauncher) - (void)appLauncher->DeleteTemporaryFileOnExit(target); + if (appLauncher) { + if (nsDownloadManager::gDownloadManagerService->mInPrivateBrowsing) { + (void)appLauncher->DeleteTemporaryPrivateFileWhenPossible(target); + } else { + (void)appLauncher->DeleteTemporaryFileOnExit(target); + } + } } return retVal; From 5420a6007465d448325a8e8f206355bd2c3191a5 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:04:49 -0400 Subject: [PATCH 11/54] Bug 722868 - Part 3: Make insExternalAppHandler::OpenWithApplication aware of the deleteTemporaryPrivateFileWhenPossible API; r=bzbarsky --- uriloader/exthandler/nsExternalHelperAppService.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index 5ab6698c0ad7..ca037370dee6 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -2208,9 +2208,12 @@ nsresult nsExternalAppHandler::OpenWithApplication() } // Always schedule files to be deleted at the end of the private browsing // mode, regardless of the value of the pref. - else if (deleteTempFileOnExit || mExtProtSvc->InPrivateBrowsing()) { + else if (deleteTempFileOnExit) { mExtProtSvc->DeleteTemporaryFileOnExit(mFinalFileDestination); } + else if (mExtProtSvc->InPrivateBrowsing()) { + mExtProtSvc->DeleteTemporaryPrivateFileWhenPossible(mFinalFileDestination); + } } return rv; From 72c7201a2d45dc9d715f31175d1ba71444667e6a Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:14:16 -0400 Subject: [PATCH 12/54] Bug 722868 - Part 4: Make the view source window aware of the deleteTemporaryPrivateFileWhenPossible API; r=Mossop --- .../viewsource/content/viewSourceUtils.js | 46 +++++++++++++++---- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/toolkit/components/viewsource/content/viewSourceUtils.js b/toolkit/components/viewsource/content/viewSourceUtils.js index eb2c1fef2844..709f0693bb03 100644 --- a/toolkit/components/viewsource/content/viewSourceUtils.js +++ b/toolkit/components/viewsource/content/viewSourceUtils.js @@ -119,10 +119,28 @@ var gViewSourceUtils = { webBrowserPersist.progressListener = this.viewSourceProgressListener; webBrowserPersist.saveURI(uri, null, null, null, null, file); - // register the file to be deleted on app exit - Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] - .getService(Components.interfaces.nsPIExternalAppLauncher) - .deleteTemporaryFileOnExit(file); + let fromPrivateWindow = false; + if (aDocument) { + try { + fromPrivateWindow = + aDocument.defaultView + .QueryInterface(Components.interfaces.nsIInterfaceRequestor) + .getInterface(Components.interfaces.nsIWebNavigation) + .QueryInterface(Components.interfaces.nsILoadContext) + .usePrivateBrowsing; + } catch (e) { + } + } + + let helperService = Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] + .getService(Components.interfaces.nsPIExternalAppLauncher); + if (fromPrivateWindow) { + // register the file to be deleted when possible + helperService.deleteTemporaryPrivateFileWhenPossible(file); + } else { + // register the file to be deleted on app exit + helperService.deleteTemporaryFileOnExit(file); + } } else { // we'll use nsIWebPageDescriptor to get the source because it may // not have to refetch the file from the server @@ -264,10 +282,22 @@ var gViewSourceUtils = { coStream.close(); foStream.close(); - // register the file to be deleted on app exit - Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] - .getService(Components.interfaces.nsPIExternalAppLauncher) - .deleteTemporaryFileOnExit(this.file); + let fromPrivateWindow = + this.data.doc.defaultView + .QueryInterface(Components.interfaces.nsIInterfaceRequestor) + .getInterface(Components.interfaces.nsIWebNavigation) + .QueryInterface(Components.interfaces.nsILoadContext) + .usePrivateBrowsing; + + let helperService = Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] + .getService(Components.interfaces.nsPIExternalAppLauncher); + if (fromPrivateWindow) { + // register the file to be deleted when possible + helperService.deleteTemporaryPrivateFileWhenPossible(this.file); + } else { + // register the file to be deleted on app exit + helperService.deleteTemporaryFileOnExit(this.file); + } } var editorArgs = gViewSourceUtils.buildEditorArgs(this.file.path, From 1e90e8fe386745a8d90f1af57d2de9849ece8764 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:31:35 -0400 Subject: [PATCH 13/54] Bug 722868 - Part 5: Use the channel's private browsing flag to determine how to handle the temporary file in nsExternalAppHandler::OpenWithApplication; r=bzbarsky --- .../exthandler/nsExternalHelperAppService.cpp | 17 +++++++++++++++-- .../exthandler/nsExternalHelperAppService.h | 6 ------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index ca037370dee6..bbc9622a4ccf 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -108,6 +108,7 @@ #include "nsIDocShellTreeOwner.h" #include "nsIDocShellTreeItem.h" #include "ExternalHelperAppChild.h" +#include "nsILoadContext.h" #ifdef MOZ_WIDGET_ANDROID #include "AndroidBridge.h" @@ -2192,9 +2193,21 @@ nsresult nsExternalAppHandler::OpenWithApplication() false); #endif + // See whether the channel has been opened in private browsing mode + bool inPrivateBrowsing = false; + NS_ASSERTION(mRequest, "This should never be called with a null request"); + nsCOMPtr channel = do_QueryInterface(mRequest); + if (channel) { + nsCOMPtr ctx; + NS_QueryNotificationCallbacks(channel, ctx); + if (ctx) { + inPrivateBrowsing = ctx->UsePrivateBrowsing(); + } + } + // make the tmp file readonly so users won't edit it and lose the changes // only if we're going to delete the file - if (deleteTempFileOnExit || mExtProtSvc->InPrivateBrowsing()) + if (deleteTempFileOnExit || inPrivateBrowsing) mFinalFileDestination->SetPermissions(0400); rv = mMimeInfo->LaunchWithFile(mFinalFileDestination); @@ -2211,7 +2224,7 @@ nsresult nsExternalAppHandler::OpenWithApplication() else if (deleteTempFileOnExit) { mExtProtSvc->DeleteTemporaryFileOnExit(mFinalFileDestination); } - else if (mExtProtSvc->InPrivateBrowsing()) { + else if (inPrivateBrowsing) { mExtProtSvc->DeleteTemporaryPrivateFileWhenPossible(mFinalFileDestination); } } diff --git a/uriloader/exthandler/nsExternalHelperAppService.h b/uriloader/exthandler/nsExternalHelperAppService.h index 6e2d83ff8817..56e5b83cc9e1 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.h +++ b/uriloader/exthandler/nsExternalHelperAppService.h @@ -111,12 +111,6 @@ public: virtual NS_HIDDEN_(nsresult) OSProtocolHandlerExists(const char *aScheme, bool *aExists) = 0; - /** - * Simple accessor to let nsExternalAppHandler know if we are currently - * inside the private browsing mode. - */ - bool InPrivateBrowsing() const { return mInPrivateBrowsing; } - protected: /** * Searches the "extra" array of MIMEInfo objects for an object From f7f65b8cd68083c79a70c6dbd5f18b07e5d806dc Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:35:22 -0400 Subject: [PATCH 14/54] Bug 722868 - Part 6: Delete the temporary private files when the last private browsing window is closed; r=bzbarsky --- uriloader/exthandler/nsExternalHelperAppService.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index bbc9622a4ccf..7dd34b927f1b 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -532,6 +532,8 @@ nsresult nsExternalHelperAppService::Init() nsresult rv = obs->AddObserver(this, "profile-before-change", true); NS_ENSURE_SUCCESS(rv, rv); + rv = obs->AddObserver(this, "last-pb-context-exited", true); + NS_ENSURE_SUCCESS(rv, rv); return obs->AddObserver(this, NS_PRIVATE_BROWSING_SWITCH_TOPIC, true); } @@ -1065,6 +1067,8 @@ nsExternalHelperAppService::Observe(nsISupports *aSubject, const char *aTopic, c { if (!strcmp(aTopic, "profile-before-change")) { ExpungeTemporaryFiles(); + } else if (!strcmp(aTopic, "last-pb-context-exited")) { + ExpungeTemporaryPrivateFiles(); } else if (!strcmp(aTopic, NS_PRIVATE_BROWSING_SWITCH_TOPIC)) { if (NS_LITERAL_STRING(NS_PRIVATE_BROWSING_ENTER).Equals(someData)) mInPrivateBrowsing = true; From 5890e37db6fbc4297b086405ed495db23b4e2131 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:37:31 -0400 Subject: [PATCH 15/54] Bug 722868 - Part 7: Remove the usage of the global private browsing in the external app helper service since it is no longer needed; r=bzbarsky --- .../exthandler/nsExternalHelperAppService.cpp | 22 ++----------------- .../exthandler/nsExternalHelperAppService.h | 4 ---- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index 7dd34b927f1b..fbcced1cd9ec 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -100,8 +100,6 @@ #include "plbase64.h" #include "prmem.h" -#include "nsIPrivateBrowsingService.h" - #include "ContentChild.h" #include "nsXULAppAPI.h" #include "nsPIDOMWindow.h" @@ -505,18 +503,11 @@ NS_IMPL_ISUPPORTS6( nsIObserver, nsISupportsWeakReference) -nsExternalHelperAppService::nsExternalHelperAppService() : - mInPrivateBrowsing(false) +nsExternalHelperAppService::nsExternalHelperAppService() { } nsresult nsExternalHelperAppService::Init() { - nsCOMPtr pbs = - do_GetService(NS_PRIVATE_BROWSING_SERVICE_CONTRACTID); - if (pbs) { - pbs->GetPrivateBrowsingEnabled(&mInPrivateBrowsing); - } - // Add an observer for profile change nsCOMPtr obs = mozilla::services::GetObserverService(); if (!obs) @@ -532,9 +523,7 @@ nsresult nsExternalHelperAppService::Init() nsresult rv = obs->AddObserver(this, "profile-before-change", true); NS_ENSURE_SUCCESS(rv, rv); - rv = obs->AddObserver(this, "last-pb-context-exited", true); - NS_ENSURE_SUCCESS(rv, rv); - return obs->AddObserver(this, NS_PRIVATE_BROWSING_SWITCH_TOPIC, true); + return obs->AddObserver(this, "last-pb-context-exited", true); } nsExternalHelperAppService::~nsExternalHelperAppService() @@ -1069,13 +1058,6 @@ nsExternalHelperAppService::Observe(nsISupports *aSubject, const char *aTopic, c ExpungeTemporaryFiles(); } else if (!strcmp(aTopic, "last-pb-context-exited")) { ExpungeTemporaryPrivateFiles(); - } else if (!strcmp(aTopic, NS_PRIVATE_BROWSING_SWITCH_TOPIC)) { - if (NS_LITERAL_STRING(NS_PRIVATE_BROWSING_ENTER).Equals(someData)) - mInPrivateBrowsing = true; - else if (NS_LITERAL_STRING(NS_PRIVATE_BROWSING_LEAVE).Equals(someData)) { - mInPrivateBrowsing = false; - ExpungeTemporaryPrivateFiles(); - } } return NS_OK; } diff --git a/uriloader/exthandler/nsExternalHelperAppService.h b/uriloader/exthandler/nsExternalHelperAppService.h index 56e5b83cc9e1..ce2e39c33a0e 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.h +++ b/uriloader/exthandler/nsExternalHelperAppService.h @@ -191,10 +191,6 @@ protected: * added during the private browsing mode) */ nsCOMArray mTemporaryPrivateFilesList; - /** - * Whether we are in private browsing mode - */ - bool mInPrivateBrowsing; }; /** From 9f814b505f1740a5084c91941646e8ea13d6ef3e Mon Sep 17 00:00:00 2001 From: Ed Morley Date: Thu, 28 Jun 2012 19:00:30 +0100 Subject: [PATCH 16/54] Backout a6175c97f365, 3ceeeaf0519f, bde34cebdcdc, 66dfa9606626, 86762d8c4de9, 717f908c990a, aa83d71fe7ee (bug 722868) for compilation failures --- .../downloads/nsDownloadManager.cpp | 9 +-- .../viewsource/content/viewSourceUtils.js | 46 +++---------- .../exthandler/nsExternalHelperAppService.cpp | 64 ++++++++----------- .../exthandler/nsExternalHelperAppService.h | 15 +++-- .../nsIExternalHelperAppService.idl | 7 +- 5 files changed, 46 insertions(+), 95 deletions(-) diff --git a/toolkit/components/downloads/nsDownloadManager.cpp b/toolkit/components/downloads/nsDownloadManager.cpp index 7c5cbec752cf..f550acfca3e5 100644 --- a/toolkit/components/downloads/nsDownloadManager.cpp +++ b/toolkit/components/downloads/nsDownloadManager.cpp @@ -2879,13 +2879,8 @@ nsDownload::OpenWithApplication() // Even if we are unable to get this service we return the result // of LaunchWithFile() which makes more sense. - if (appLauncher) { - if (nsDownloadManager::gDownloadManagerService->mInPrivateBrowsing) { - (void)appLauncher->DeleteTemporaryPrivateFileWhenPossible(target); - } else { - (void)appLauncher->DeleteTemporaryFileOnExit(target); - } - } + if (appLauncher) + (void)appLauncher->DeleteTemporaryFileOnExit(target); } return retVal; diff --git a/toolkit/components/viewsource/content/viewSourceUtils.js b/toolkit/components/viewsource/content/viewSourceUtils.js index 709f0693bb03..eb2c1fef2844 100644 --- a/toolkit/components/viewsource/content/viewSourceUtils.js +++ b/toolkit/components/viewsource/content/viewSourceUtils.js @@ -119,28 +119,10 @@ var gViewSourceUtils = { webBrowserPersist.progressListener = this.viewSourceProgressListener; webBrowserPersist.saveURI(uri, null, null, null, null, file); - let fromPrivateWindow = false; - if (aDocument) { - try { - fromPrivateWindow = - aDocument.defaultView - .QueryInterface(Components.interfaces.nsIInterfaceRequestor) - .getInterface(Components.interfaces.nsIWebNavigation) - .QueryInterface(Components.interfaces.nsILoadContext) - .usePrivateBrowsing; - } catch (e) { - } - } - - let helperService = Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] - .getService(Components.interfaces.nsPIExternalAppLauncher); - if (fromPrivateWindow) { - // register the file to be deleted when possible - helperService.deleteTemporaryPrivateFileWhenPossible(file); - } else { - // register the file to be deleted on app exit - helperService.deleteTemporaryFileOnExit(file); - } + // register the file to be deleted on app exit + Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] + .getService(Components.interfaces.nsPIExternalAppLauncher) + .deleteTemporaryFileOnExit(file); } else { // we'll use nsIWebPageDescriptor to get the source because it may // not have to refetch the file from the server @@ -282,22 +264,10 @@ var gViewSourceUtils = { coStream.close(); foStream.close(); - let fromPrivateWindow = - this.data.doc.defaultView - .QueryInterface(Components.interfaces.nsIInterfaceRequestor) - .getInterface(Components.interfaces.nsIWebNavigation) - .QueryInterface(Components.interfaces.nsILoadContext) - .usePrivateBrowsing; - - let helperService = Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] - .getService(Components.interfaces.nsPIExternalAppLauncher); - if (fromPrivateWindow) { - // register the file to be deleted when possible - helperService.deleteTemporaryPrivateFileWhenPossible(this.file); - } else { - // register the file to be deleted on app exit - helperService.deleteTemporaryFileOnExit(this.file); - } + // register the file to be deleted on app exit + Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] + .getService(Components.interfaces.nsPIExternalAppLauncher) + .deleteTemporaryFileOnExit(this.file); } var editorArgs = gViewSourceUtils.buildEditorArgs(this.file.path, diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index fbcced1cd9ec..c78569dedaa6 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -100,13 +100,14 @@ #include "plbase64.h" #include "prmem.h" +#include "nsIPrivateBrowsingService.h" + #include "ContentChild.h" #include "nsXULAppAPI.h" #include "nsPIDOMWindow.h" #include "nsIDocShellTreeOwner.h" #include "nsIDocShellTreeItem.h" #include "ExternalHelperAppChild.h" -#include "nsILoadContext.h" #ifdef MOZ_WIDGET_ANDROID #include "AndroidBridge.h" @@ -503,11 +504,18 @@ NS_IMPL_ISUPPORTS6( nsIObserver, nsISupportsWeakReference) -nsExternalHelperAppService::nsExternalHelperAppService() +nsExternalHelperAppService::nsExternalHelperAppService() : + mInPrivateBrowsing(false) { } nsresult nsExternalHelperAppService::Init() { + nsCOMPtr pbs = + do_GetService(NS_PRIVATE_BROWSING_SERVICE_CONTRACTID); + if (pbs) { + pbs->GetPrivateBrowsingEnabled(&mInPrivateBrowsing); + } + // Add an observer for profile change nsCOMPtr obs = mozilla::services::GetObserverService(); if (!obs) @@ -523,7 +531,7 @@ nsresult nsExternalHelperAppService::Init() nsresult rv = obs->AddObserver(this, "profile-before-change", true); NS_ENSURE_SUCCESS(rv, rv); - return obs->AddObserver(this, "last-pb-context-exited", true); + return obs->AddObserver(this, NS_PRIVATE_BROWSING_SWITCH_TOPIC, true); } nsExternalHelperAppService::~nsExternalHelperAppService() @@ -915,10 +923,7 @@ NS_IMETHODIMP nsExternalHelperAppService::GetApplicationDescription(const nsACSt // Methods related to deleting temporary files on exit ////////////////////////////////////////////////////////////////////////////////////////////////////// -/* static */ -nsresult -nsExternalHelperAppService::DeleteTemporaryFileHelper(nsIFile * aTemporaryFile, - nsCOMArray &aFileList) +NS_IMETHODIMP nsExternalHelperAppService::DeleteTemporaryFileOnExit(nsIFile * aTemporaryFile) { bool isFile = false; @@ -926,23 +931,14 @@ nsExternalHelperAppService::DeleteTemporaryFileHelper(nsIFile * aTemporaryFile, aTemporaryFile->IsFile(&isFile); if (!isFile) return NS_OK; - aFileList.AppendObject(localFile); + if (mInPrivateBrowsing) + mTemporaryPrivateFilesList.AppendObject(aTemporaryFile); + else + mTemporaryFilesList.AppendObject(aTemporaryFile); return NS_OK; } -NS_IMETHODIMP -nsExternalHelperAppService::DeleteTemporaryFileOnExit(nsIFile* aTemporaryFile) -{ - return DeleteTemporaryFileHelper(aTemporaryFile, mTemporaryFilesList); -} - -NS_IMETHODIMP -nsExternalHelperAppService::DeleteTemporaryPrivateFileWhenPossible(nsIFile* aTemporaryFile) -{ - return DeleteTemporaryFileHelper(aTemporaryFile, mTemporaryPrivateFilesList); -} - void nsExternalHelperAppService::FixFilePermissions(nsIFile* aFile) { // This space intentionally left blank @@ -1056,8 +1052,13 @@ nsExternalHelperAppService::Observe(nsISupports *aSubject, const char *aTopic, c { if (!strcmp(aTopic, "profile-before-change")) { ExpungeTemporaryFiles(); - } else if (!strcmp(aTopic, "last-pb-context-exited")) { - ExpungeTemporaryPrivateFiles(); + } else if (!strcmp(aTopic, NS_PRIVATE_BROWSING_SWITCH_TOPIC)) { + if (NS_LITERAL_STRING(NS_PRIVATE_BROWSING_ENTER).Equals(someData)) + mInPrivateBrowsing = true; + else if (NS_LITERAL_STRING(NS_PRIVATE_BROWSING_LEAVE).Equals(someData)) { + mInPrivateBrowsing = false; + ExpungeTemporaryPrivateFiles(); + } } return NS_OK; } @@ -2179,21 +2180,9 @@ nsresult nsExternalAppHandler::OpenWithApplication() false); #endif - // See whether the channel has been opened in private browsing mode - bool inPrivateBrowsing = false; - NS_ASSERTION(mRequest, "This should never be called with a null request"); - nsCOMPtr channel = do_QueryInterface(mRequest); - if (channel) { - nsCOMPtr ctx; - NS_QueryNotificationCallbacks(channel, ctx); - if (ctx) { - inPrivateBrowsing = ctx->UsePrivateBrowsing(); - } - } - // make the tmp file readonly so users won't edit it and lose the changes // only if we're going to delete the file - if (deleteTempFileOnExit || inPrivateBrowsing) + if (deleteTempFileOnExit || mExtProtSvc->InPrivateBrowsing()) mFinalFileDestination->SetPermissions(0400); rv = mMimeInfo->LaunchWithFile(mFinalFileDestination); @@ -2207,12 +2196,9 @@ nsresult nsExternalAppHandler::OpenWithApplication() } // Always schedule files to be deleted at the end of the private browsing // mode, regardless of the value of the pref. - else if (deleteTempFileOnExit) { + else if (deleteTempFileOnExit || mExtProtSvc->InPrivateBrowsing()) { mExtProtSvc->DeleteTemporaryFileOnExit(mFinalFileDestination); } - else if (inPrivateBrowsing) { - mExtProtSvc->DeleteTemporaryPrivateFileWhenPossible(mFinalFileDestination); - } } return rv; diff --git a/uriloader/exthandler/nsExternalHelperAppService.h b/uriloader/exthandler/nsExternalHelperAppService.h index ce2e39c33a0e..59505ba43b30 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.h +++ b/uriloader/exthandler/nsExternalHelperAppService.h @@ -111,6 +111,12 @@ public: virtual NS_HIDDEN_(nsresult) OSProtocolHandlerExists(const char *aScheme, bool *aExists) = 0; + /** + * Simple accessor to let nsExternalAppHandler know if we are currently + * inside the private browsing mode. + */ + bool InPrivateBrowsing() const { return mInPrivateBrowsing; } + protected: /** * Searches the "extra" array of MIMEInfo objects for an object @@ -166,11 +172,6 @@ protected: * Helper function for ExpungeTemporaryFiles and ExpungeTemporaryPrivateFiles */ static void ExpungeTemporaryFilesHelper(nsCOMArray &fileList); - /** - * Helper function for DeleteTemporaryFileOnExit and DeleteTemporaryPrivateFileWhenPossible - */ - static nsresult DeleteTemporaryFileHelper(nsIFile* aTemporaryFile, - nsCOMArray &aFileList); /** * Functions related to the tempory file cleanup service provided by * nsExternalHelperAppService @@ -191,6 +192,10 @@ protected: * added during the private browsing mode) */ nsCOMArray mTemporaryPrivateFilesList; + /** + * Whether we are in private browsing mode + */ + bool mInPrivateBrowsing; }; /** diff --git a/uriloader/exthandler/nsIExternalHelperAppService.idl b/uriloader/exthandler/nsIExternalHelperAppService.idl index c0066ec31a36..e5ee8b7549e0 100644 --- a/uriloader/exthandler/nsIExternalHelperAppService.idl +++ b/uriloader/exthandler/nsIExternalHelperAppService.idl @@ -53,7 +53,7 @@ interface nsIExternalHelperAppService : nsISupports * This is a private interface shared between external app handlers and the platform specific * external helper app service */ -[scriptable, uuid(6613e2e7-feab-4e3a-bb1f-b03200d544ec)] +[scriptable, uuid(d0b5d7d3-9565-403d-9fb5-e5089c4567c6)] interface nsPIExternalAppLauncher : nsISupports { /** @@ -62,11 +62,6 @@ interface nsPIExternalAppLauncher : nsISupports * @param aTemporaryFile A temporary file we should delete on exit. */ void deleteTemporaryFileOnExit(in nsIFile aTemporaryFile); - /** - * Delete a temporary file created inside private browsing mode when - * the private browsing mode has ended. - */ - void deleteTemporaryPrivateFileWhenPossible(in nsIFile aTemporaryFile); }; /** From bae66faeef43a8027ccac0f8fee087d90363d123 Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Thu, 28 Jun 2012 11:03:53 -0700 Subject: [PATCH 17/54] Bug 758427 - Ignore KeyEvents with keyCodes greater than getMaxKeyCode(), such as Gingerbread Galaxy Note's bogus stylus events. r=blassey --- mobile/android/base/GeckoInputConnection.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/mobile/android/base/GeckoInputConnection.java b/mobile/android/base/GeckoInputConnection.java index 9d6771147ad6..df72cd7460c4 100644 --- a/mobile/android/base/GeckoInputConnection.java +++ b/mobile/android/base/GeckoInputConnection.java @@ -876,6 +876,9 @@ public class GeckoInputConnection + isPreIme + ")"); } + if (keyCode > KeyEvent.getMaxKeyCode()) + return false; + clampSelection(); switch (keyCode) { @@ -935,6 +938,9 @@ public class GeckoInputConnection + isPreIme + ")"); } + if (keyCode > KeyEvent.getMaxKeyCode()) + return false; + switch (keyCode) { case KeyEvent.KEYCODE_BACK: case KeyEvent.KEYCODE_SEARCH: From 8c4f7239bf61721701d122d82c6086210934176b Mon Sep 17 00:00:00 2001 From: Asaf Romano Date: Thu, 28 Jun 2012 21:14:45 +0300 Subject: [PATCH 18/54] Bug 761494 - Cannot open all Bookmarks in tabs by Middle clicking a folder in Sidebar. r=mak --- browser/components/places/content/treeView.js | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/browser/components/places/content/treeView.js b/browser/components/places/content/treeView.js index 23e853d5d544..b6a95777216c 100644 --- a/browser/components/places/content/treeView.js +++ b/browser/components/places/content/treeView.js @@ -2,6 +2,13 @@ * 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/XPCOMUtils.jsm'); + +const PTV_interfaces = [Ci.nsITreeView, + Ci.nsINavHistoryResultObserver, + Ci.nsINavHistoryResultTreeViewer, + Ci.nsISupportsWeakReference]; + function PlacesTreeView(aFlatList, aOnOpenFlatContainer, aController) { this._tree = null; this._result = null; @@ -39,15 +46,17 @@ PlacesTreeView.prototype = { return this.__dateService; }, - QueryInterface: function PTV_QueryInterface(aIID) { - if (aIID.equals(Ci.nsITreeView) || - aIID.equals(Ci.nsINavHistoryResultObserver) || - aIID.equals(Ci.nsINavHistoryResultTreeViewer) || - aIID.equals(Ci.nsISupportsWeakReference) || - aIID.equals(Ci.nsISupports)) - return this; - throw Cr.NS_ERROR_NO_INTERFACE; - }, + QueryInterface: XPCOMUtils.generateQI(PTV_interfaces), + + // Bug 761494: + // ---------- + // Some addons use methods from nsINavHistoryResultObserver and + // nsINavHistoryResultTreeViewer, without QIing to these intefaces first. + // That's not a problem when the view is retrieved through the + // .view getter (which returns the wrappedJSObject of this object), + // it raises an issue when the view retrieved through the treeBoxObject.view + // getter. Thus, to avoid breaking addons, the interfaces are prefetched. + classInfo: XPCOMUtils.generateCI({ interfaces: PTV_interfaces }), /** * This is called once both the result and the tree are set. From 543ec7adc00fa8c446f967c26a6cfe10ab54a494 Mon Sep 17 00:00:00 2001 From: Geoff Brown Date: Thu, 28 Jun 2012 11:20:15 -0700 Subject: [PATCH 19/54] Bug 756704 - Robocop: avoid hangs when events missed; r=kats --- .../robocop/FennecNativeActions.java.in | 38 ++++++++++++++++--- mobile/android/base/tests/BaseTest.java.in | 10 ++--- 2 files changed, 37 insertions(+), 11 deletions(-) diff --git a/build/mobile/robocop/FennecNativeActions.java.in b/build/mobile/robocop/FennecNativeActions.java.in index 7656d139db4c..cf5a68701a3b 100644 --- a/build/mobile/robocop/FennecNativeActions.java.in +++ b/build/mobile/robocop/FennecNativeActions.java.in @@ -36,6 +36,7 @@ public class FennecNativeActions implements Actions { private Solo mSolo; private Instrumentation mInstr; private Activity mGeckoApp; + private Assert mAsserter; // Objects for reflexive access of fennec classes. private ClassLoader mClassLoader; @@ -50,10 +51,11 @@ public class FennecNativeActions implements Actions { private Method mSetDrawListener; private static final String LOGTAG = "FennecNativeActions"; - public FennecNativeActions(Activity activity, Solo robocop, Instrumentation instrumentation) { + public FennecNativeActions(Activity activity, Solo robocop, Instrumentation instrumentation, Assert asserter) { mSolo = robocop; mInstr = instrumentation; mGeckoApp = activity; + mAsserter = asserter; // Set up reflexive access of java classes and methods. try { mClassLoader = activity.getClassLoader(); @@ -117,6 +119,7 @@ public class FennecNativeActions implements Actions { private final String mGeckoEvent; private final Object[] mRegistrationParams; private boolean mEventReceived; + private static final int MAX_WAIT_MS = 90000; GeckoEventExpecter(String geckoEvent, Object[] registrationParams) { mGeckoEvent = geckoEvent; @@ -124,13 +127,21 @@ public class FennecNativeActions implements Actions { } public synchronized void blockForEvent() { + long startTime = SystemClock.uptimeMillis(); + long endTime = 0; while (! mEventReceived) { try { - this.wait(); + this.wait(MAX_WAIT_MS); } catch (InterruptedException ie) { FennecNativeDriver.log(LogLevel.ERROR, ie); break; } + endTime = SystemClock.uptimeMillis(); + if (!mEventReceived && (endTime - startTime >= MAX_WAIT_MS)) { + mAsserter.ok(false, "GeckoEventExpecter", + "blockForEvent timeout: "+mGeckoEvent); + return; + } } FennecNativeDriver.log(FennecNativeDriver.LogLevel.DEBUG, "unblocked on expecter for " + mGeckoEvent); @@ -222,6 +233,7 @@ public class FennecNativeActions implements Actions { class PaintExpecter implements RepeatedEventExpecter { private Object mLayerClient; private boolean mPaintDone; + private static final int MAX_WAIT_MS = 90000; PaintExpecter() throws IllegalAccessException, InvocationTargetException { mLayerClient = mGetLayerClient.invoke(mGeckoApp); @@ -236,13 +248,20 @@ public class FennecNativeActions implements Actions { } public synchronized void blockForEvent() { + long startTime = SystemClock.uptimeMillis(); + long endTime = 0; while (!mPaintDone) { try { - this.wait(); + this.wait(MAX_WAIT_MS); } catch (InterruptedException ie) { FennecNativeDriver.log(LogLevel.ERROR, ie); break; } + endTime = SystemClock.uptimeMillis(); + if (!mPaintDone && (endTime - startTime >= MAX_WAIT_MS)) { + mAsserter.ok(false, "PaintExpecter", "blockForEvent timeout"); + return; + } } try { mSetDrawListener.invoke(mLayerClient, (Object)null); @@ -260,16 +279,23 @@ public class FennecNativeActions implements Actions { throw new IllegalArgumentException("millis must be > 0"); } // wait for at least one event + long startTime = SystemClock.uptimeMillis(); + long endTime = 0; while (!mPaintDone) { try { - this.wait(); + this.wait(MAX_WAIT_MS); } catch (InterruptedException ie) { FennecNativeDriver.log(LogLevel.ERROR, ie); break; } + endTime = SystemClock.uptimeMillis(); + if (!mPaintDone && (endTime - startTime >= MAX_WAIT_MS)) { + mAsserter.ok(false, "PaintExpecter", "blockUtilClear timeout"); + return; + } } // now wait for a period of millis where we don't get an event - long startTime = SystemClock.uptimeMillis(); + startTime = SystemClock.uptimeMillis(); while (true) { try { this.wait(millis); @@ -277,7 +303,7 @@ public class FennecNativeActions implements Actions { FennecNativeDriver.log(LogLevel.ERROR, ie); break; } - long endTime = SystemClock.uptimeMillis(); + endTime = SystemClock.uptimeMillis(); if (endTime - startTime >= millis) { // success break; diff --git a/mobile/android/base/tests/BaseTest.java.in b/mobile/android/base/tests/BaseTest.java.in index 2a414aa224d2..0e85a75e6bd4 100644 --- a/mobile/android/base/tests/BaseTest.java.in +++ b/mobile/android/base/tests/BaseTest.java.in @@ -68,11 +68,6 @@ abstract class BaseTest extends ActivityInstrumentationTestCase2 { setActivityIntent(i); mActivity = getActivity(); - // Set up Robotium.solo and Driver objects - mSolo = new Solo(getInstrumentation()); - mDriver = new FennecNativeDriver(mActivity, mSolo); - mActions = new FennecNativeActions(mActivity, mSolo, getInstrumentation()); - mLogFile = (String)config.get("logfile"); mBaseUrl = ((String)config.get("host")).replaceAll("(/$)", ""); mRawBaseUrl = ((String)config.get("rawhost")).replaceAll("(/$)", ""); @@ -85,6 +80,11 @@ abstract class BaseTest extends ActivityInstrumentationTestCase2 { } mAsserter.setLogFile(mLogFile); mAsserter.setTestName(this.getClass().getName()); + + // Set up Robotium.solo and Driver objects + mSolo = new Solo(getInstrumentation()); + mDriver = new FennecNativeDriver(mActivity, mSolo); + mActions = new FennecNativeActions(mActivity, mSolo, getInstrumentation(), mAsserter); } @Override From 456b3d687f2ae4e5cfc05dfc87ff6359ebc17f93 Mon Sep 17 00:00:00 2001 From: Brian Nicholson Date: Thu, 28 Jun 2012 11:15:17 -0700 Subject: [PATCH 20/54] Bug 762968 - Part 1: Ignore bogus stylus events in onKeyDown(). r=cpeterson --HG-- extra : rebase_source : 75267b1bffe981fe5e60477018e9c86677742cb8 --- mobile/android/base/AwesomeBar.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/mobile/android/base/AwesomeBar.java b/mobile/android/base/AwesomeBar.java index 0a7cdb2daec3..9ff75f999e52 100644 --- a/mobile/android/base/AwesomeBar.java +++ b/mobile/android/base/AwesomeBar.java @@ -353,6 +353,11 @@ public class AwesomeBar extends GeckoActivity implements GeckoEventListener { @Override public boolean onKeyDown(int keyCode, KeyEvent event) { + // Galaxy Note sends key events for the stylus that are outside of the + // valid keyCode range (see bug 758427) + if (keyCode > KeyEvent.getMaxKeyCode()) + return true; + // This method is called only if the key event was not handled // by any of the views, which usually means the edit box lost focus if (keyCode == KeyEvent.KEYCODE_BACK || From 38baf2167f34ea51dcd5b5c495c7352cfd778b32 Mon Sep 17 00:00:00 2001 From: Brian Nicholson Date: Thu, 28 Jun 2012 11:21:24 -0700 Subject: [PATCH 21/54] Bug 762968 - Part 2: Replace onInterceptTouchEvent() with individual listeners. r=wesj --HG-- extra : rebase_source : 96242a2de53c8df97426e7b9f31acbe01f37aa6f --- mobile/android/base/AwesomeBar.java | 9 +++++++++ mobile/android/base/AwesomeBarTabs.java | 24 ++++++++++++------------ 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/mobile/android/base/AwesomeBar.java b/mobile/android/base/AwesomeBar.java index 9ff75f999e52..cc1686f66086 100644 --- a/mobile/android/base/AwesomeBar.java +++ b/mobile/android/base/AwesomeBar.java @@ -220,6 +220,15 @@ public class AwesomeBar extends GeckoActivity implements GeckoEventListener { } }); + mText.setOnFocusChangeListener(new View.OnFocusChangeListener() { + public void onFocusChange(View v, boolean hasFocus) { + if (!hasFocus) { + InputMethodManager imm = (InputMethodManager) getSystemService(Context.INPUT_METHOD_SERVICE); + imm.hideSoftInputFromWindow(v.getWindowToken(), 0); + } + } + }); + registerForContextMenu(mAwesomeTabs.findViewById(R.id.all_pages_list)); registerForContextMenu(mAwesomeTabs.findViewById(R.id.bookmarks_list)); registerForContextMenu(mAwesomeTabs.findViewById(R.id.history_list)); diff --git a/mobile/android/base/AwesomeBarTabs.java b/mobile/android/base/AwesomeBarTabs.java index 5cc80c9ffd2f..5e6799f7c5b2 100644 --- a/mobile/android/base/AwesomeBarTabs.java +++ b/mobile/android/base/AwesomeBarTabs.java @@ -68,6 +68,7 @@ public class AwesomeBarTabs extends TabHost { private boolean mInflated; private LayoutInflater mInflater; private OnUrlOpenListener mUrlOpenListener; + private View.OnTouchListener mListTouchListener; private ContentResolver mContentResolver; private ContentObserver mContentObserver; private SearchEngine mSuggestEngine; @@ -886,6 +887,14 @@ public class AwesomeBarTabs extends TabHost { // to the TabHost. setup(); + mListTouchListener = new View.OnTouchListener() { + public boolean onTouch(View view, MotionEvent event) { + if (event.getAction() == MotionEvent.ACTION_DOWN) + hideSoftInput(view); + return false; + } + }; + addAllPagesTab(); addBookmarksTab(); addHistoryTab(); @@ -974,6 +983,7 @@ public class AwesomeBarTabs extends TabHost { }); allPagesList.setAdapter(mAllPagesCursorAdapter); + allPagesList.setOnTouchListener(mListTouchListener); } private void addBookmarksTab() { @@ -984,6 +994,7 @@ public class AwesomeBarTabs extends TabHost { R.id.bookmarks_list); ListView bookmarksList = (ListView) findViewById(R.id.bookmarks_list); + bookmarksList.setOnTouchListener(mListTouchListener); // Only load bookmark list when tab is actually used. // See OnTabChangeListener above. @@ -997,6 +1008,7 @@ public class AwesomeBarTabs extends TabHost { R.id.history_list); ListView historyList = (ListView) findViewById(R.id.history_list); + historyList.setOnTouchListener(mListTouchListener); // Only load history list when tab is actually used. // See OnTabChangeListener above. @@ -1224,16 +1236,4 @@ public class AwesomeBarTabs extends TabHost { public boolean isInReadingList() { return mInReadingList; } - - @Override - public boolean onInterceptTouchEvent(MotionEvent ev) { - // we should only have to hide the soft keyboard once - when the user - // initially touches the screen - if (ev.getAction() == MotionEvent.ACTION_DOWN) - hideSoftInput(this); - - // the android docs make no sense, but returning false will cause this and other - // motion events to be sent to the view the user tapped on - return false; - } } From d81b3c6fa4d78926afaf7fe398634e9d5f24c303 Mon Sep 17 00:00:00 2001 From: Asaf Romano Date: Thu, 28 Jun 2012 21:24:44 +0300 Subject: [PATCH 22/54] Bug 762799 - Cannot open bookmark from sidebar with keyboard (with Enter key). r=mak --- browser/components/places/content/sidebarUtils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/browser/components/places/content/sidebarUtils.js b/browser/components/places/content/sidebarUtils.js index d08fadd67004..659afe0eeb10 100644 --- a/browser/components/places/content/sidebarUtils.js +++ b/browser/components/places/content/sidebarUtils.js @@ -66,11 +66,11 @@ var SidebarUtils = { handleTreeKeyPress: function SU_handleTreeKeyPress(aEvent) { // XXX Bug 627901: Post Fx4, this method should take a tree parameter. - let node = aEvent.target.selectedNode; + let tree = aEvent.target; + let node = tree.selectedNode; if (node) { - let view = PlacesUIUtils.getViewForNode(node); if (aEvent.keyCode == KeyEvent.DOM_VK_RETURN) - PlacesUIUtils.openNodeWithEvent(node, aEvent, view); + PlacesUIUtils.openNodeWithEvent(node, aEvent, tree); } }, From 2e4f27310ac0da3427ff1f091b0a50cacb22188c Mon Sep 17 00:00:00 2001 From: Vladan Djeric Date: Thu, 28 Jun 2012 14:57:52 -0400 Subject: [PATCH 23/54] Bug 763138: Telemetry should indicate whether a debugger is attached. r=bsmedberg,smichaud --- toolkit/components/telemetry/TelemetryPing.js | 11 +++++ xpcom/base/nsDebugImpl.cpp | 41 +++++++++++++++++++ xpcom/base/nsIDebug2.idl | 8 +++- 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/toolkit/components/telemetry/TelemetryPing.js b/toolkit/components/telemetry/TelemetryPing.js index b3e67b66b724..33cf5d509044 100644 --- a/toolkit/components/telemetry/TelemetryPing.js +++ b/toolkit/components/telemetry/TelemetryPing.js @@ -65,6 +65,8 @@ const IDLE_TIMEOUT_SECONDS = 5 * 60; var gLastMemoryPoll = null; +let gWasDebuggerAttached = false; + function getLocale() { return Cc["@mozilla.org/chrome/chrome-registry;1"]. getService(Ci.nsIXULChromeRegistry). @@ -121,6 +123,12 @@ function getSimpleMeasurements() { ret.startupInterrupted = new Number(Services.startup.interrupted); + // Update debuggerAttached flag + let debugService = Cc["@mozilla.org/xpcom/debug;1"].getService(Ci.nsIDebug2); + let isDebuggerAttached = debugService.isDebuggerAttached; + gWasDebuggerAttached = gWasDebuggerAttached || isDebuggerAttached; + ret.debuggerAttached = new Number(gWasDebuggerAttached); + ret.js = Cc["@mozilla.org/js/xpc/XPConnect;1"] .getService(Ci.nsIJSEngineTelemetryStats) .telemetryValue; @@ -776,6 +784,9 @@ TelemetryPing.prototype = { case "sessionstore-windows-restored": Services.obs.removeObserver(this, "sessionstore-windows-restored"); this._hasWindowRestoredObserver = false; + // Check whether debugger was attached during startup + let debugService = Cc["@mozilla.org/xpcom/debug;1"].getService(Ci.nsIDebug2); + gWasDebuggerAttached = debugService.isDebuggerAttached; // fall through case "test-gather-startup": this.gatherStartupInformation(); diff --git a/xpcom/base/nsDebugImpl.cpp b/xpcom/base/nsDebugImpl.cpp index d99f390e0c3f..c3a14dfa58fd 100644 --- a/xpcom/base/nsDebugImpl.cpp +++ b/xpcom/base/nsDebugImpl.cpp @@ -43,6 +43,13 @@ #include "nsString.h" #endif +#if defined(XP_MACOSX) +#include +#include +#include +#include +#endif + #include "mozilla/mozalloc_abort.h" static void @@ -136,6 +143,40 @@ nsDebugImpl::GetAssertionCount(PRInt32* aResult) return NS_OK; } +NS_IMETHODIMP +nsDebugImpl::GetIsDebuggerAttached(bool* aResult) +{ + *aResult = false; + +#if defined(XP_WIN) + *aResult = ::IsDebuggerPresent(); +#elif defined(XP_MACOSX) + // Specify the info we're looking for + int mib[4]; + mib[0] = CTL_KERN; + mib[1] = KERN_PROC; + mib[2] = KERN_PROC_PID; + mib[3] = getpid(); + size_t mibSize = sizeof(mib) / sizeof(int); + + struct kinfo_proc info; + size_t infoSize = sizeof(info); + memset(&info, 0, infoSize); + + if (sysctl(mib, mibSize, &info, &infoSize, NULL, 0)) { + // if the call fails, default to false + *aResult = false; + return NS_OK; + } + + if (info.kp_proc.p_flag & P_TRACED) { + *aResult = true; + } +#endif + + return NS_OK; +} + /* static */ void nsDebugImpl::SetMultiprocessMode(const char *aDesc) { diff --git a/xpcom/base/nsIDebug2.idl b/xpcom/base/nsIDebug2.idl index 63266290aae2..57e17d834a43 100644 --- a/xpcom/base/nsIDebug2.idl +++ b/xpcom/base/nsIDebug2.idl @@ -7,7 +7,7 @@ #include "nsIDebug.idl" -[scriptable, uuid(9c9307ed-480a-4f2a-8f29-21378c03bcbc)] +[scriptable, uuid(6cb17fec-cdf7-4f7c-b267-37a0acaa9cf1)] interface nsIDebug2 : nsIDebug { /** @@ -21,4 +21,10 @@ interface nsIDebug2 : nsIDebug * The number of assertions since process start. */ readonly attribute long assertionCount; + + /** + * Whether a debugger is currently attached. + * Supports Windows + Mac + */ + readonly attribute bool isDebuggerAttached; }; From 2ba15a514fb9bdae71db803754c87f073ecdd2a4 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 22:56:55 -0400 Subject: [PATCH 24/54] Bug 722868 - Part 1: Add the deleteTemporaryPrivateFileWhenPossible API; r=bzbarsky --- .../exthandler/nsExternalHelperAppService.cpp | 22 ++++++++++++++----- .../exthandler/nsExternalHelperAppService.h | 5 +++++ .../nsIExternalHelperAppService.idl | 7 +++++- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index c78569dedaa6..c2c67b00a9b7 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -923,7 +923,10 @@ NS_IMETHODIMP nsExternalHelperAppService::GetApplicationDescription(const nsACSt // Methods related to deleting temporary files on exit ////////////////////////////////////////////////////////////////////////////////////////////////////// -NS_IMETHODIMP nsExternalHelperAppService::DeleteTemporaryFileOnExit(nsIFile * aTemporaryFile) +/* static */ +nsresult +nsExternalHelperAppService::DeleteTemporaryFileHelper(nsIFile * aTemporaryFile, + nsCOMArray &aFileList) { bool isFile = false; @@ -931,14 +934,23 @@ NS_IMETHODIMP nsExternalHelperAppService::DeleteTemporaryFileOnExit(nsIFile * aT aTemporaryFile->IsFile(&isFile); if (!isFile) return NS_OK; - if (mInPrivateBrowsing) - mTemporaryPrivateFilesList.AppendObject(aTemporaryFile); - else - mTemporaryFilesList.AppendObject(aTemporaryFile); + aFileList.AppendObject(aTemporaryFile); return NS_OK; } +NS_IMETHODIMP +nsExternalHelperAppService::DeleteTemporaryFileOnExit(nsIFile* aTemporaryFile) +{ + return DeleteTemporaryFileHelper(aTemporaryFile, mTemporaryFilesList); +} + +NS_IMETHODIMP +nsExternalHelperAppService::DeleteTemporaryPrivateFileWhenPossible(nsIFile* aTemporaryFile) +{ + return DeleteTemporaryFileHelper(aTemporaryFile, mTemporaryPrivateFilesList); +} + void nsExternalHelperAppService::FixFilePermissions(nsIFile* aFile) { // This space intentionally left blank diff --git a/uriloader/exthandler/nsExternalHelperAppService.h b/uriloader/exthandler/nsExternalHelperAppService.h index 59505ba43b30..3813479ccd39 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.h +++ b/uriloader/exthandler/nsExternalHelperAppService.h @@ -172,6 +172,11 @@ protected: * Helper function for ExpungeTemporaryFiles and ExpungeTemporaryPrivateFiles */ static void ExpungeTemporaryFilesHelper(nsCOMArray &fileList); + /** + * Helper function for DeleteTemporaryFileOnExit and DeleteTemporaryPrivateFileWhenPossible + */ + static nsresult DeleteTemporaryFileHelper(nsIFile* aTemporaryFile, + nsCOMArray &aFileList); /** * Functions related to the tempory file cleanup service provided by * nsExternalHelperAppService diff --git a/uriloader/exthandler/nsIExternalHelperAppService.idl b/uriloader/exthandler/nsIExternalHelperAppService.idl index e5ee8b7549e0..c0066ec31a36 100644 --- a/uriloader/exthandler/nsIExternalHelperAppService.idl +++ b/uriloader/exthandler/nsIExternalHelperAppService.idl @@ -53,7 +53,7 @@ interface nsIExternalHelperAppService : nsISupports * This is a private interface shared between external app handlers and the platform specific * external helper app service */ -[scriptable, uuid(d0b5d7d3-9565-403d-9fb5-e5089c4567c6)] +[scriptable, uuid(6613e2e7-feab-4e3a-bb1f-b03200d544ec)] interface nsPIExternalAppLauncher : nsISupports { /** @@ -62,6 +62,11 @@ interface nsPIExternalAppLauncher : nsISupports * @param aTemporaryFile A temporary file we should delete on exit. */ void deleteTemporaryFileOnExit(in nsIFile aTemporaryFile); + /** + * Delete a temporary file created inside private browsing mode when + * the private browsing mode has ended. + */ + void deleteTemporaryPrivateFileWhenPossible(in nsIFile aTemporaryFile); }; /** From b03ad8e3e773dbc8c51f8f66c1330dd83b72f868 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:01:35 -0400 Subject: [PATCH 25/54] Bug 722868 - Part 2: Make the download manager aware of the deleteTemporaryPrivateFileWhenPossible API; r=gavin --- toolkit/components/downloads/nsDownloadManager.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/toolkit/components/downloads/nsDownloadManager.cpp b/toolkit/components/downloads/nsDownloadManager.cpp index f550acfca3e5..7c5cbec752cf 100644 --- a/toolkit/components/downloads/nsDownloadManager.cpp +++ b/toolkit/components/downloads/nsDownloadManager.cpp @@ -2879,8 +2879,13 @@ nsDownload::OpenWithApplication() // Even if we are unable to get this service we return the result // of LaunchWithFile() which makes more sense. - if (appLauncher) - (void)appLauncher->DeleteTemporaryFileOnExit(target); + if (appLauncher) { + if (nsDownloadManager::gDownloadManagerService->mInPrivateBrowsing) { + (void)appLauncher->DeleteTemporaryPrivateFileWhenPossible(target); + } else { + (void)appLauncher->DeleteTemporaryFileOnExit(target); + } + } } return retVal; From 8fa8c9e94c4f11f0d1ceddbf9d20b91e3e081a96 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:04:49 -0400 Subject: [PATCH 26/54] Bug 722868 - Part 3: Make insExternalAppHandler::OpenWithApplication aware of the deleteTemporaryPrivateFileWhenPossible API; r=bzbarsky --- uriloader/exthandler/nsExternalHelperAppService.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index c2c67b00a9b7..e5330c0beda2 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -2208,9 +2208,12 @@ nsresult nsExternalAppHandler::OpenWithApplication() } // Always schedule files to be deleted at the end of the private browsing // mode, regardless of the value of the pref. - else if (deleteTempFileOnExit || mExtProtSvc->InPrivateBrowsing()) { + else if (deleteTempFileOnExit) { mExtProtSvc->DeleteTemporaryFileOnExit(mFinalFileDestination); } + else if (mExtProtSvc->InPrivateBrowsing()) { + mExtProtSvc->DeleteTemporaryPrivateFileWhenPossible(mFinalFileDestination); + } } return rv; From 2d1d49791df2eb0f99951f988cc4b562a1780837 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:14:16 -0400 Subject: [PATCH 27/54] Bug 722868 - Part 4: Make the view source window aware of the deleteTemporaryPrivateFileWhenPossible API; r=Mossop --- .../viewsource/content/viewSourceUtils.js | 46 +++++++++++++++---- 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/toolkit/components/viewsource/content/viewSourceUtils.js b/toolkit/components/viewsource/content/viewSourceUtils.js index eb2c1fef2844..709f0693bb03 100644 --- a/toolkit/components/viewsource/content/viewSourceUtils.js +++ b/toolkit/components/viewsource/content/viewSourceUtils.js @@ -119,10 +119,28 @@ var gViewSourceUtils = { webBrowserPersist.progressListener = this.viewSourceProgressListener; webBrowserPersist.saveURI(uri, null, null, null, null, file); - // register the file to be deleted on app exit - Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] - .getService(Components.interfaces.nsPIExternalAppLauncher) - .deleteTemporaryFileOnExit(file); + let fromPrivateWindow = false; + if (aDocument) { + try { + fromPrivateWindow = + aDocument.defaultView + .QueryInterface(Components.interfaces.nsIInterfaceRequestor) + .getInterface(Components.interfaces.nsIWebNavigation) + .QueryInterface(Components.interfaces.nsILoadContext) + .usePrivateBrowsing; + } catch (e) { + } + } + + let helperService = Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] + .getService(Components.interfaces.nsPIExternalAppLauncher); + if (fromPrivateWindow) { + // register the file to be deleted when possible + helperService.deleteTemporaryPrivateFileWhenPossible(file); + } else { + // register the file to be deleted on app exit + helperService.deleteTemporaryFileOnExit(file); + } } else { // we'll use nsIWebPageDescriptor to get the source because it may // not have to refetch the file from the server @@ -264,10 +282,22 @@ var gViewSourceUtils = { coStream.close(); foStream.close(); - // register the file to be deleted on app exit - Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] - .getService(Components.interfaces.nsPIExternalAppLauncher) - .deleteTemporaryFileOnExit(this.file); + let fromPrivateWindow = + this.data.doc.defaultView + .QueryInterface(Components.interfaces.nsIInterfaceRequestor) + .getInterface(Components.interfaces.nsIWebNavigation) + .QueryInterface(Components.interfaces.nsILoadContext) + .usePrivateBrowsing; + + let helperService = Components.classes["@mozilla.org/uriloader/external-helper-app-service;1"] + .getService(Components.interfaces.nsPIExternalAppLauncher); + if (fromPrivateWindow) { + // register the file to be deleted when possible + helperService.deleteTemporaryPrivateFileWhenPossible(this.file); + } else { + // register the file to be deleted on app exit + helperService.deleteTemporaryFileOnExit(this.file); + } } var editorArgs = gViewSourceUtils.buildEditorArgs(this.file.path, From bbda19a62fa736ca263f25c63dbff8ae7499d10c Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:31:35 -0400 Subject: [PATCH 28/54] Bug 722868 - Part 5: Use the channel's private browsing flag to determine how to handle the temporary file in nsExternalAppHandler::OpenWithApplication; r=bzbarsky --- .../exthandler/nsExternalHelperAppService.cpp | 17 +++++++++++++++-- .../exthandler/nsExternalHelperAppService.h | 6 ------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index e5330c0beda2..9aad64042993 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -108,6 +108,7 @@ #include "nsIDocShellTreeOwner.h" #include "nsIDocShellTreeItem.h" #include "ExternalHelperAppChild.h" +#include "nsILoadContext.h" #ifdef MOZ_WIDGET_ANDROID #include "AndroidBridge.h" @@ -2192,9 +2193,21 @@ nsresult nsExternalAppHandler::OpenWithApplication() false); #endif + // See whether the channel has been opened in private browsing mode + bool inPrivateBrowsing = false; + NS_ASSERTION(mRequest, "This should never be called with a null request"); + nsCOMPtr channel = do_QueryInterface(mRequest); + if (channel) { + nsCOMPtr ctx; + NS_QueryNotificationCallbacks(channel, ctx); + if (ctx) { + inPrivateBrowsing = ctx->UsePrivateBrowsing(); + } + } + // make the tmp file readonly so users won't edit it and lose the changes // only if we're going to delete the file - if (deleteTempFileOnExit || mExtProtSvc->InPrivateBrowsing()) + if (deleteTempFileOnExit || inPrivateBrowsing) mFinalFileDestination->SetPermissions(0400); rv = mMimeInfo->LaunchWithFile(mFinalFileDestination); @@ -2211,7 +2224,7 @@ nsresult nsExternalAppHandler::OpenWithApplication() else if (deleteTempFileOnExit) { mExtProtSvc->DeleteTemporaryFileOnExit(mFinalFileDestination); } - else if (mExtProtSvc->InPrivateBrowsing()) { + else if (inPrivateBrowsing) { mExtProtSvc->DeleteTemporaryPrivateFileWhenPossible(mFinalFileDestination); } } diff --git a/uriloader/exthandler/nsExternalHelperAppService.h b/uriloader/exthandler/nsExternalHelperAppService.h index 3813479ccd39..c677a72bfa22 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.h +++ b/uriloader/exthandler/nsExternalHelperAppService.h @@ -111,12 +111,6 @@ public: virtual NS_HIDDEN_(nsresult) OSProtocolHandlerExists(const char *aScheme, bool *aExists) = 0; - /** - * Simple accessor to let nsExternalAppHandler know if we are currently - * inside the private browsing mode. - */ - bool InPrivateBrowsing() const { return mInPrivateBrowsing; } - protected: /** * Searches the "extra" array of MIMEInfo objects for an object From 406ac7f90e289e04babc609d23c172772a1c7333 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:35:22 -0400 Subject: [PATCH 29/54] Bug 722868 - Part 6: Delete the temporary private files when the last private browsing window is closed; r=bzbarsky --- uriloader/exthandler/nsExternalHelperAppService.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index 9aad64042993..0fb9bcc1b0e4 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -532,6 +532,8 @@ nsresult nsExternalHelperAppService::Init() nsresult rv = obs->AddObserver(this, "profile-before-change", true); NS_ENSURE_SUCCESS(rv, rv); + rv = obs->AddObserver(this, "last-pb-context-exited", true); + NS_ENSURE_SUCCESS(rv, rv); return obs->AddObserver(this, NS_PRIVATE_BROWSING_SWITCH_TOPIC, true); } @@ -1065,6 +1067,8 @@ nsExternalHelperAppService::Observe(nsISupports *aSubject, const char *aTopic, c { if (!strcmp(aTopic, "profile-before-change")) { ExpungeTemporaryFiles(); + } else if (!strcmp(aTopic, "last-pb-context-exited")) { + ExpungeTemporaryPrivateFiles(); } else if (!strcmp(aTopic, NS_PRIVATE_BROWSING_SWITCH_TOPIC)) { if (NS_LITERAL_STRING(NS_PRIVATE_BROWSING_ENTER).Equals(someData)) mInPrivateBrowsing = true; From 619dd8e8e8a8ed610407b49f933bfd9c62c6e3ec Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Tue, 24 Apr 2012 23:37:31 -0400 Subject: [PATCH 30/54] Bug 722868 - Part 7: Remove the usage of the global private browsing in the external app helper service since it is no longer needed; r=bzbarsky --- .../exthandler/nsExternalHelperAppService.cpp | 22 ++----------------- .../exthandler/nsExternalHelperAppService.h | 4 ---- 2 files changed, 2 insertions(+), 24 deletions(-) diff --git a/uriloader/exthandler/nsExternalHelperAppService.cpp b/uriloader/exthandler/nsExternalHelperAppService.cpp index 0fb9bcc1b0e4..b0acafa07c84 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.cpp +++ b/uriloader/exthandler/nsExternalHelperAppService.cpp @@ -100,8 +100,6 @@ #include "plbase64.h" #include "prmem.h" -#include "nsIPrivateBrowsingService.h" - #include "ContentChild.h" #include "nsXULAppAPI.h" #include "nsPIDOMWindow.h" @@ -505,18 +503,11 @@ NS_IMPL_ISUPPORTS6( nsIObserver, nsISupportsWeakReference) -nsExternalHelperAppService::nsExternalHelperAppService() : - mInPrivateBrowsing(false) +nsExternalHelperAppService::nsExternalHelperAppService() { } nsresult nsExternalHelperAppService::Init() { - nsCOMPtr pbs = - do_GetService(NS_PRIVATE_BROWSING_SERVICE_CONTRACTID); - if (pbs) { - pbs->GetPrivateBrowsingEnabled(&mInPrivateBrowsing); - } - // Add an observer for profile change nsCOMPtr obs = mozilla::services::GetObserverService(); if (!obs) @@ -532,9 +523,7 @@ nsresult nsExternalHelperAppService::Init() nsresult rv = obs->AddObserver(this, "profile-before-change", true); NS_ENSURE_SUCCESS(rv, rv); - rv = obs->AddObserver(this, "last-pb-context-exited", true); - NS_ENSURE_SUCCESS(rv, rv); - return obs->AddObserver(this, NS_PRIVATE_BROWSING_SWITCH_TOPIC, true); + return obs->AddObserver(this, "last-pb-context-exited", true); } nsExternalHelperAppService::~nsExternalHelperAppService() @@ -1069,13 +1058,6 @@ nsExternalHelperAppService::Observe(nsISupports *aSubject, const char *aTopic, c ExpungeTemporaryFiles(); } else if (!strcmp(aTopic, "last-pb-context-exited")) { ExpungeTemporaryPrivateFiles(); - } else if (!strcmp(aTopic, NS_PRIVATE_BROWSING_SWITCH_TOPIC)) { - if (NS_LITERAL_STRING(NS_PRIVATE_BROWSING_ENTER).Equals(someData)) - mInPrivateBrowsing = true; - else if (NS_LITERAL_STRING(NS_PRIVATE_BROWSING_LEAVE).Equals(someData)) { - mInPrivateBrowsing = false; - ExpungeTemporaryPrivateFiles(); - } } return NS_OK; } diff --git a/uriloader/exthandler/nsExternalHelperAppService.h b/uriloader/exthandler/nsExternalHelperAppService.h index c677a72bfa22..2af4044a8368 100644 --- a/uriloader/exthandler/nsExternalHelperAppService.h +++ b/uriloader/exthandler/nsExternalHelperAppService.h @@ -191,10 +191,6 @@ protected: * added during the private browsing mode) */ nsCOMArray mTemporaryPrivateFilesList; - /** - * Whether we are in private browsing mode - */ - bool mInPrivateBrowsing; }; /** From b1677bb975a2c585aabf6ba924927df3b530c1d7 Mon Sep 17 00:00:00 2001 From: Saurabh Anand Date: Fri, 29 Jun 2012 02:52:41 +0530 Subject: [PATCH 31/54] Bug 722984 - nsBrowserGlue uses global private browsing service to make decisions; r=ehsan --- browser/components/nsBrowserGlue.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/browser/components/nsBrowserGlue.js b/browser/components/nsBrowserGlue.js index 6478cc38ed10..931211843025 100644 --- a/browser/components/nsBrowserGlue.js +++ b/browser/components/nsBrowserGlue.js @@ -476,10 +476,13 @@ BrowserGlue.prototype = { var windowcount = 0; var pagecount = 0; var browserEnum = Services.wm.getEnumerator("navigator:browser"); + let allWindowsPrivate = true; while (browserEnum.hasMoreElements()) { windowcount++; var browser = browserEnum.getNext(); + if (("gPrivateBrowsingUI" in browser) && !browser.gPrivateBrowsingUI.privateWindow) + allWindowsPrivate = false; var tabbrowser = browser.document.getElementById("content"); if (tabbrowser) pagecount += tabbrowser.browsers.length - tabbrowser._numPinnedTabs; @@ -523,10 +526,8 @@ BrowserGlue.prototype = { return; } - var inPrivateBrowsing = Cc["@mozilla.org/privatebrowsing;1"]. - getService(Ci.nsIPrivateBrowsingService). - privateBrowsingEnabled; - if (inPrivateBrowsing) + // Never show a prompt inside private browsing mode + if (allWindowsPrivate) return; if (!showPrompt) @@ -1573,6 +1574,8 @@ ContentPermissionPrompt.prototype = { var message; var secondaryActions = []; + var requestingWindow = request.window.top; + var chromeWin = getChromeWindow(requestingWindow).wrappedJSObject; // Different message/options if it is a local file if (requestingURI.schemeIs("file")) { @@ -1583,11 +1586,7 @@ ContentPermissionPrompt.prototype = { [requestingURI.host], 1); // Don't offer to "always/never share" in PB mode - var inPrivateBrowsing = Cc["@mozilla.org/privatebrowsing;1"]. - getService(Ci.nsIPrivateBrowsingService). - privateBrowsingEnabled; - - if (!inPrivateBrowsing) { + if (("gPrivateBrowsingUI" in chromeWin) && !chromeWin.gPrivateBrowsingUI.privateWindow) { secondaryActions.push({ label: browserBundle.GetStringFromName("geolocation.alwaysShareLocation"), accessKey: browserBundle.GetStringFromName("geolocation.alwaysShareLocation.accesskey"), @@ -1607,8 +1606,6 @@ ContentPermissionPrompt.prototype = { } } - var requestingWindow = request.window.top; - var chromeWin = getChromeWindow(requestingWindow).wrappedJSObject; var browser = chromeWin.gBrowser.getBrowserForDocument(requestingWindow.document); chromeWin.PopupNotifications.show(browser, "geolocation", message, "geo-notification-icon", From 5fd32a8647dc99db7b8968a4a8f7c4146d4ef224 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Jun 2012 23:47:55 +0200 Subject: [PATCH 32/54] Bug 754202 - Fix content->chrome postMessage mochitest. r=bz This isn't something we actually support, and were using enablePrivilege to do it before. When we switch this to SpecialPowers wrapping, the sender origin becomes a chrome URL (not very interesting) and the source window becomes null (which we do explicitly in nsGlobalWindow for chrome callers). So those tests stop being useful. --- dom/tests/mochitest/whatwg/postMessage_chrome_helper.html | 4 +--- dom/tests/mochitest/whatwg/test_postMessage_chrome.html | 4 ---- 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/dom/tests/mochitest/whatwg/postMessage_chrome_helper.html b/dom/tests/mochitest/whatwg/postMessage_chrome_helper.html index a93cc5b46b89..86a57ed0f6af 100644 --- a/dom/tests/mochitest/whatwg/postMessage_chrome_helper.html +++ b/dom/tests/mochitest/whatwg/postMessage_chrome_helper.html @@ -37,9 +37,7 @@ function respond(msg) { - // ...so get privileges and test that this works with privileges - netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect"); - window.parent.postMessage(msg, "*"); + SpecialPowers.wrap(window).parent.postMessage(msg, "*"); } window.addEventListener("message", receiveMessage, false); diff --git a/dom/tests/mochitest/whatwg/test_postMessage_chrome.html b/dom/tests/mochitest/whatwg/test_postMessage_chrome.html index 2ba2a6622883..e50ef6f0947f 100644 --- a/dom/tests/mochitest/whatwg/test_postMessage_chrome.html +++ b/dom/tests/mochitest/whatwg/test_postMessage_chrome.html @@ -93,10 +93,6 @@ function chromePathIsSet(evt) function receiveContent(evt) { is(evt.isTrusted, true, "should have sent a trusted event"); - is(evt.origin, "http://example.org", "content response event has wrong URI"); - is(evt.source, window.frames.contentDomain, - "wrong source for same-domain message!"); - finish(); } From 6def798e8fb1c4e967e956e767954ea2293ee0cc Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Jun 2012 23:47:55 +0200 Subject: [PATCH 33/54] Bug 754202 - Pull object principals directly off the compartment and assert that behavior doesn't change. r=bz --- caps/include/nsScriptSecurityManager.h | 7 ++--- caps/src/nsScriptSecurityManager.cpp | 40 +++++++++++++++----------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/caps/include/nsScriptSecurityManager.h b/caps/include/nsScriptSecurityManager.h index 5196ed561f7c..f7b94fabcd06 100644 --- a/caps/include/nsScriptSecurityManager.h +++ b/caps/include/nsScriptSecurityManager.h @@ -403,12 +403,11 @@ private: // Returns null if a principal cannot be found; generally callers // should error out at that point. - static nsIPrincipal* - doGetObjectPrincipal(JSObject *obj + static nsIPrincipal* doGetObjectPrincipal(JSObject *obj); #ifdef DEBUG - , bool aAllowShortCircuit = true + static nsIPrincipal* + old_doGetObjectPrincipal(JSObject *obj, bool aAllowShortCircuit = true); #endif - ); // Returns null if a principal cannot be found. Note that rv can be NS_OK // when this happens -- this means that there was no JS running. diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp index 71224abda462..8e79a7366d86 100644 --- a/caps/src/nsScriptSecurityManager.cpp +++ b/caps/src/nsScriptSecurityManager.cpp @@ -2398,19 +2398,33 @@ nsScriptSecurityManager::GetObjectPrincipal(JSContext *aCx, JSObject *aObj, // static nsIPrincipal* -nsScriptSecurityManager::doGetObjectPrincipal(JSObject *aObj +nsScriptSecurityManager::doGetObjectPrincipal(JSObject *aObj) +{ + JSCompartment *compartment = js::GetObjectCompartment(aObj); + JSPrincipals *principals = JS_GetCompartmentPrincipals(compartment); + nsIPrincipal *principal = nsJSPrincipals::get(principals); + + // We leave the old code in for a little while to make sure that pulling + // object principals directly off the compartment always gives an equivalent + // result (from a security perspective). #ifdef DEBUG - , bool aAllowShortCircuit + nsIPrincipal *old = old_doGetObjectPrincipal(aObj); + MOZ_ASSERT(NS_SUCCEEDED(CheckSameOriginPrincipal(principal, old))); #endif - ) + + return principal; +} + +#ifdef DEBUG +// static +nsIPrincipal* +nsScriptSecurityManager::old_doGetObjectPrincipal(JSObject *aObj, + bool aAllowShortCircuit) { NS_ASSERTION(aObj, "Bad call to doGetObjectPrincipal()!"); nsIPrincipal* result = nsnull; -#ifdef DEBUG JSObject* origObj = aObj; -#endif - js::Class *jsClass = js::GetObjectClass(aObj); // A common case seen in this code is that we enter this function @@ -2444,12 +2458,7 @@ nsScriptSecurityManager::doGetObjectPrincipal(JSObject *aObj if (IS_WRAPPER_CLASS(jsClass)) { result = sXPConnect->GetPrincipal(aObj, -#ifdef DEBUG - aAllowShortCircuit -#else - true -#endif - ); + aAllowShortCircuit); if (result) { break; } @@ -2465,7 +2474,6 @@ nsScriptSecurityManager::doGetObjectPrincipal(JSObject *aObj priv = nsnull; } -#ifdef DEBUG if (aAllowShortCircuit) { nsCOMPtr xpcWrapper = do_QueryInterface(priv); @@ -2475,7 +2483,6 @@ nsScriptSecurityManager::doGetObjectPrincipal(JSObject *aObj "Uh, an nsIXPConnectWrappedNative with the " "wrong JSClass or getObjectOps hooks!"); } -#endif nsCOMPtr objPrin = do_QueryInterface(priv); @@ -2497,9 +2504,8 @@ nsScriptSecurityManager::doGetObjectPrincipal(JSObject *aObj jsClass = js::GetObjectClass(aObj); } while (1); -#ifdef DEBUG if (aAllowShortCircuit) { - nsIPrincipal *principal = doGetObjectPrincipal(origObj, false); + nsIPrincipal *principal = old_doGetObjectPrincipal(origObj, false); // Because of inner window reuse, we can have objects with one principal // living in a scope with a different (but same-origin) principal. So @@ -2507,10 +2513,10 @@ nsScriptSecurityManager::doGetObjectPrincipal(JSObject *aObj NS_ASSERTION(NS_SUCCEEDED(CheckSameOriginPrincipal(result, principal)), "Principal mismatch. Not good"); } -#endif return result; } +#endif /* DEBUG */ ///////////////// Capabilities API ///////////////////// NS_IMETHODIMP From 285b2d0a7eba5f609c5e66334d4dbbd3525436d3 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Jun 2012 23:47:55 +0200 Subject: [PATCH 34/54] Bug 754202 - Pull subject principals directly off the compartment. r=mrbkap It would be nice to check these principals against the principals acquired using the old mechanism. Unfortunately, they often differ. Because CAPS uses JS stack frames, any time we enter a compartment and do an operation (even throwing an Access-Denied exception) without running any JS code, we'll end up with a different principal. Our security story is pretty darn tied to compartments at this point, so let's just pull the trigger. --- caps/src/nsScriptSecurityManager.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp index 8e79a7366d86..85b787bd63c3 100644 --- a/caps/src/nsScriptSecurityManager.cpp +++ b/caps/src/nsScriptSecurityManager.cpp @@ -2380,9 +2380,15 @@ nsIPrincipal* nsScriptSecurityManager::GetSubjectPrincipal(JSContext *cx, nsresult* rv) { - NS_PRECONDITION(rv, "Null out param"); - JSStackFrame *fp; - return GetPrincipalAndFrame(cx, &fp, rv); + *rv = NS_OK; + JSCompartment *compartment = js::GetContextCompartment(cx); + + // The context should always be in a compartment, either one it has entered + // or the one associated with its global. + MOZ_ASSERT(!!compartment); + + JSPrincipals *principals = JS_GetCompartmentPrincipals(compartment); + return nsJSPrincipals::get(principals); } NS_IMETHODIMP From ebec529f3b8affc85f0aaf35d8b1aa4a98466fa3 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Jun 2012 23:47:55 +0200 Subject: [PATCH 35/54] Bug 754202 - Disallow calling EvaluateString{,WithValue} with a principal that doesn't match the global. r=mrbkap --- dom/base/nsJSEnvironment.cpp | 70 ++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/dom/base/nsJSEnvironment.cpp b/dom/base/nsJSEnvironment.cpp index d2663c4d43cc..acdced806e57 100644 --- a/dom/base/nsJSEnvironment.cpp +++ b/dom/base/nsJSEnvironment.cpp @@ -1199,27 +1199,29 @@ nsJSContext::EvaluateStringWithValue(const nsAString& aScript, xpc_UnmarkGrayObject(aScopeObject); nsAutoMicroTask mt; - // Safety first: get an object representing the script's principals, i.e., - // the entities who signed this script, or the fully-qualified-domain-name - // or "codebase" from which it was loaded. - nsCOMPtr principal = aPrincipal; - nsresult rv; - if (!aPrincipal) { - nsIScriptGlobalObject *global = GetGlobalObject(); - if (!global) - return NS_ERROR_FAILURE; - nsCOMPtr objPrincipal = - do_QueryInterface(global, &rv); - if (NS_FAILED(rv)) - return NS_ERROR_FAILURE; - principal = objPrincipal->GetPrincipal(); - if (!principal) - return NS_ERROR_FAILURE; - } + // Ignore the principal that was passed in, and just assert that it matches + // the one we pull off the global. + nsCOMPtr principal; + nsCOMPtr objPrincipal = do_QueryInterface(GetGlobalObject()); + if (!objPrincipal) + return NS_ERROR_FAILURE; + principal = objPrincipal->GetPrincipal(); + if (!principal) + return NS_ERROR_FAILURE; +#ifdef DEBUG + bool equal = false; + principal->Equals(aPrincipal, &equal); + MOZ_ASSERT(equal); + nsIPrincipal *scopeObjectPrincipal = + nsJSPrincipals::get(JS_GetCompartmentPrincipals(js::GetObjectCompartment(aScopeObject))); + equal = false; + principal->Equals(scopeObjectPrincipal, &equal); + MOZ_ASSERT(equal); +#endif bool ok = false; - rv = sSecurityManager->CanExecuteScripts(mContext, principal, &ok); + nsresult rv = sSecurityManager->CanExecuteScripts(mContext, principal, &ok); if (NS_FAILED(rv)) { return NS_ERROR_FAILURE; } @@ -1401,19 +1403,25 @@ nsJSContext::EvaluateString(const nsAString& aScript, xpc_UnmarkGrayObject(aScopeObject); - // Safety first: get an object representing the script's principals, i.e., - // the entities who signed this script, or the fully-qualified-domain-name - // or "codebase" from which it was loaded. - nsCOMPtr principal = aPrincipal; - if (!aPrincipal) { - nsCOMPtr objPrincipal = - do_QueryInterface(GetGlobalObject()); - if (!objPrincipal) - return NS_ERROR_FAILURE; - principal = objPrincipal->GetPrincipal(); - if (!principal) - return NS_ERROR_FAILURE; - } + // Ignore the principal that was passed in, and just assert that it matches + // the one we pull off the global. + nsCOMPtr principal; + nsCOMPtr objPrincipal = do_QueryInterface(GetGlobalObject()); + if (!objPrincipal) + return NS_ERROR_FAILURE; + principal = objPrincipal->GetPrincipal(); + if (!principal) + return NS_ERROR_FAILURE; +#ifdef DEBUG + bool equal = false; + principal->Equals(aPrincipal, &equal); + MOZ_ASSERT(equal); + nsIPrincipal *scopeObjectPrincipal = + nsJSPrincipals::get(JS_GetCompartmentPrincipals(js::GetObjectCompartment(aScopeObject))); + equal = false; + principal->Equals(scopeObjectPrincipal, &equal); + MOZ_ASSERT(equal); +#endif bool ok = false; From 83245872f0e8a052601d580b7224c88a9f89271b Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Jun 2012 23:47:55 +0200 Subject: [PATCH 36/54] Bug 754202 - Remove context pushing/popping API. r=mrbkap Each one of these uses grabs the principal off of an object for pushing, but also enters the compartment of that object. So we shouldn't need this anymore. Can I get a 'hell yeah'? --- caps/idl/nsIScriptSecurityManager.idl | 25 +------ caps/src/nsScriptSecurityManager.cpp | 70 +------------------- dom/base/nsJSEnvironment.cpp | 35 ---------- dom/src/json/nsJSON.cpp | 19 +----- ipc/testshell/XPCShellEnvironment.cpp | 14 ---- js/src/jsapi.h | 12 ---- js/src/jsclone.cpp | 35 +--------- js/xpconnect/shell/xpcshell.cpp | 14 ---- js/xpconnect/src/XPCWrappedJSClass.cpp | 35 ---------- js/xpconnect/wrappers/CrossOriginWrapper.cpp | 20 ------ 10 files changed, 5 insertions(+), 274 deletions(-) diff --git a/caps/idl/nsIScriptSecurityManager.idl b/caps/idl/nsIScriptSecurityManager.idl index 1eb56a10685a..1195ad5d7dc1 100644 --- a/caps/idl/nsIScriptSecurityManager.idl +++ b/caps/idl/nsIScriptSecurityManager.idl @@ -9,7 +9,7 @@ interface nsIURI; interface nsIChannel; -[scriptable, uuid(3708aa92-e2d9-4fd1-9e46-edfa3eb5ebf5)] +[scriptable, uuid(cdb27711-492b-4973-938b-de81ac124658)] interface nsIScriptSecurityManager : nsIXPCSecurityManager { ///////////////// Security Checks ////////////////// @@ -260,29 +260,6 @@ interface nsIScriptSecurityManager : nsIXPCSecurityManager [noscript,notxpcom] nsIPrincipal getCxSubjectPrincipal(in JSContextPtr cx); [noscript,notxpcom] nsIPrincipal getCxSubjectPrincipalAndFrame(in JSContextPtr cx, out JSStackFramePtr fp); - - /** - * If no scripted code is running "above" (or called from) fp, then - * instead of looking at cx->globalObject, we will return |principal|. - * This function only affects |cx|. If someone pushes another context onto - * the context stack, then it supersedes this call. - * NOTE: If |fp| is non-null popContextPrincipal must be called before fp - * has finished executing. - * - * @param cx The context to clamp. - * @param fp The frame pointer to clamp at. May be 'null'. - * @param principal The principal to clamp to. - */ - [noscript] void pushContextPrincipal(in JSContextPtr cx, - in JSStackFramePtr fp, - in nsIPrincipal principal); - - /** - * Removes a clamp set by pushContextPrincipal from cx. This must be - * called in a stack-like fashion (e.g., given two contexts |a| and |b|, - * it is not legal to do: push(a) push(b) pop(a)). - */ - [noscript] void popContextPrincipal(in JSContextPtr cx); }; %{C++ diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp index 85b787bd63c3..a49abb172e9e 100644 --- a/caps/src/nsScriptSecurityManager.cpp +++ b/caps/src/nsScriptSecurityManager.cpp @@ -163,44 +163,6 @@ GetScriptContext(JSContext *cx) return GetScriptContextFromJSContext(cx); } -// Callbacks for the JS engine to use to push/pop context principals. -static JSBool -PushPrincipalCallback(JSContext *cx, JSPrincipals *principals) -{ - // We should already be in the compartment of the given principal. - MOZ_ASSERT(principals == - JS_GetCompartmentPrincipals((js::GetContextCompartment(cx)))); - - // Get the security manager. - nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager(); - if (!ssm) { - return true; - } - - // Push the principal. - JSStackFrame *fp = NULL; - nsresult rv = ssm->PushContextPrincipal(cx, JS_FrameIterator(cx, &fp), - nsJSPrincipals::get(principals)); - if (NS_FAILED(rv)) { - JS_ReportOutOfMemory(cx); - return false; - } - - return true; -} - -static JSBool -PopPrincipalCallback(JSContext *cx) -{ - nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager(); - if (ssm) { - ssm->PopContextPrincipal(cx); - } - - return true; -} - - inline void SetPendingException(JSContext *cx, const char *aMsg) { JSAutoRequest ar(cx); @@ -405,34 +367,6 @@ nsScriptSecurityManager::GetCxSubjectPrincipalAndFrame(JSContext *cx, JSStackFra return principal; } -NS_IMETHODIMP -nsScriptSecurityManager::PushContextPrincipal(JSContext *cx, - JSStackFrame *fp, - nsIPrincipal *principal) -{ - NS_ASSERTION(principal, "Must pass a non-null principal"); - - ContextPrincipal *cp = new ContextPrincipal(mContextPrincipals, cx, fp, - principal); - if (!cp) - return NS_ERROR_OUT_OF_MEMORY; - - mContextPrincipals = cp; - return NS_OK; -} - -NS_IMETHODIMP -nsScriptSecurityManager::PopContextPrincipal(JSContext *cx) -{ - NS_ASSERTION(mContextPrincipals->mCx == cx, "Mismatched push/pop"); - - ContextPrincipal *next = mContextPrincipals->mNext; - delete mContextPrincipals; - mContextPrincipals = next; - - return NS_OK; -} - //////////////////// // Policy Storage // //////////////////// @@ -3143,9 +3077,7 @@ nsresult nsScriptSecurityManager::Init() CheckObjectAccess, nsJSPrincipals::Subsume, ObjectPrincipalFinder, - ContentSecurityPolicyPermitsJSAction, - PushPrincipalCallback, - PopPrincipalCallback + ContentSecurityPolicyPermitsJSAction }; MOZ_ASSERT(!JS_GetSecurityCallbacks(sRuntime)); diff --git a/dom/base/nsJSEnvironment.cpp b/dom/base/nsJSEnvironment.cpp index acdced806e57..bfeef71d219c 100644 --- a/dom/base/nsJSEnvironment.cpp +++ b/dom/base/nsJSEnvironment.cpp @@ -1238,9 +1238,6 @@ nsJSContext::EvaluateStringWithValue(const nsAString& aScript, jsval val; - rv = sSecurityManager->PushContextPrincipal(mContext, nsnull, principal); - NS_ENSURE_SUCCESS(rv, rv); - nsJSContext::TerminationFuncHolder holder(this); // SecurityManager said "ok", but don't compile if aVersion is unknown. @@ -1296,8 +1293,6 @@ nsJSContext::EvaluateStringWithValue(const nsAString& aScript, } } - sSecurityManager->PopContextPrincipal(mContext); - // Pop here, after JS_ValueToString and any other possible evaluation. if (NS_FAILED(stack->Pop(nsnull))) rv = NS_ERROR_FAILURE; @@ -1446,9 +1441,6 @@ nsJSContext::EvaluateString(const nsAString& aScript, jsval val = JSVAL_VOID; jsval* vp = aRetValue ? &val : NULL; - rv = sSecurityManager->PushContextPrincipal(mContext, nsnull, principal); - NS_ENSURE_SUCCESS(rv, rv); - nsJSContext::TerminationFuncHolder holder(this); ++mExecuteDepth; @@ -1500,8 +1492,6 @@ nsJSContext::EvaluateString(const nsAString& aScript, --mExecuteDepth; - sSecurityManager->PopContextPrincipal(mContext); - // Pop here, after JS_ValueToString and any other possible evaluation. if (NS_FAILED(stack->Pop(nsnull))) rv = NS_ERROR_FAILURE; @@ -1599,15 +1589,6 @@ nsJSContext::ExecuteScript(JSScript* aScriptObject, return NS_ERROR_FAILURE; } - nsCOMPtr principal; - rv = sSecurityManager->GetObjectPrincipal(mContext, - JS_GetGlobalFromScript(aScriptObject), - getter_AddRefs(principal)); - NS_ENSURE_SUCCESS(rv, rv); - - rv = sSecurityManager->PushContextPrincipal(mContext, nsnull, principal); - NS_ENSURE_SUCCESS(rv, rv); - nsJSContext::TerminationFuncHolder holder(this); XPCAutoRequest ar(mContext); ++mExecuteDepth; @@ -1634,8 +1615,6 @@ nsJSContext::ExecuteScript(JSScript* aScriptObject, --mExecuteDepth; - sSecurityManager->PopContextPrincipal(mContext); - // Pop here, after JS_ValueToString and any other possible evaluation. if (NS_FAILED(stack->Pop(nsnull))) rv = NS_ERROR_FAILURE; @@ -1876,24 +1855,12 @@ nsJSContext::CallEventHandler(nsISupports* aTarget, JSObject* aScope, jsval *argv = nsnull; JSObject *funobj = aHandler; - nsCOMPtr principal; - rv = sSecurityManager->GetObjectPrincipal(mContext, funobj, - getter_AddRefs(principal)); - NS_ENSURE_SUCCESS(rv, rv); - - JSStackFrame *currentfp = nsnull; - rv = sSecurityManager->PushContextPrincipal(mContext, - JS_FrameIterator(mContext, ¤tfp), - principal); - NS_ENSURE_SUCCESS(rv, rv); - jsval funval = OBJECT_TO_JSVAL(funobj); JSAutoEnterCompartment ac; js::ForceFrame ff(mContext, funobj); if (!ac.enter(mContext, funobj) || !ff.enter() || !JS_WrapObject(mContext, &target)) { ReportPendingException(); - sSecurityManager->PopContextPrincipal(mContext); return NS_ERROR_FAILURE; } @@ -1936,8 +1903,6 @@ nsJSContext::CallEventHandler(nsISupports* aTarget, JSObject* aScope, // nested calls through XPConnect. if (NS_FAILED(rv)) ReportPendingException(); - - sSecurityManager->PopContextPrincipal(mContext); } pusher.Pop(); diff --git a/dom/src/json/nsJSON.cpp b/dom/src/json/nsJSON.cpp index 75221971876c..feef436d730d 100644 --- a/dom/src/json/nsJSON.cpp +++ b/dom/src/json/nsJSON.cpp @@ -177,32 +177,15 @@ nsJSON::EncodeFromJSVal(JS::Value *value, JSContext *cx, nsAString &result) JSAutoRequest ar(cx); JSAutoEnterCompartment ac; - nsIScriptSecurityManager *ssm = nsnull; if (value->isObject()) { JSObject *obj = &value->toObject(); if (!ac.enter(cx, obj)) { return NS_ERROR_FAILURE; } - - nsCOMPtr principal; - ssm = nsContentUtils::GetSecurityManager(); - nsresult rv = ssm->GetObjectPrincipal(cx, obj, getter_AddRefs(principal)); - NS_ENSURE_SUCCESS(rv, rv); - - JSStackFrame *fp = nsnull; - rv = ssm->PushContextPrincipal(cx, JS_FrameIterator(cx, &fp), principal); - NS_ENSURE_SUCCESS(rv, rv); } nsJSONWriter writer; - JSBool ok = JS_Stringify(cx, value, NULL, JSVAL_NULL, - WriteCallback, &writer); - - if (ssm) { - ssm->PopContextPrincipal(cx); - } - - if (!ok) { + if (!JS_Stringify(cx, value, NULL, JSVAL_NULL, WriteCallback, &writer)) { return NS_ERROR_XPC_BAD_CONVERT_JS; } diff --git a/ipc/testshell/XPCShellEnvironment.cpp b/ipc/testshell/XPCShellEnvironment.cpp index a922d7fe9253..2a52c57fd6e7 100644 --- a/ipc/testshell/XPCShellEnvironment.cpp +++ b/ipc/testshell/XPCShellEnvironment.cpp @@ -880,20 +880,6 @@ FullTrustSecMan::GetCxSubjectPrincipalAndFrame(JSContext *cx, return mSystemPrincipal; } -NS_IMETHODIMP -FullTrustSecMan::PushContextPrincipal(JSContext *cx, - JSStackFrame *fp, - nsIPrincipal *principal) -{ - return NS_OK; -} - -NS_IMETHODIMP -FullTrustSecMan::PopContextPrincipal(JSContext *cx) -{ - return NS_OK; -} - NS_IMETHODIMP_(nsrefcnt) XPCShellDirProvider::AddRef() { diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 88b5fc75b667..caef68cef70f 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -1664,16 +1664,6 @@ typedef JSPrincipals * typedef JSBool (* JSCSPEvalChecker)(JSContext *cx); -/* - * Security callbacks for pushing and popping context principals. These are only - * temporarily necessary and will hopefully be gone again in a matter of weeks. - */ -typedef JSBool -(* JSPushContextPrincipalOp)(JSContext *cx, JSPrincipals *principals); - -typedef JSBool -(* JSPopContextPrincipalOp)(JSContext *cx); - /* * Callback used to ask the embedding for the cross compartment wrapper handler * that implements the desired prolicy for this kind of object in the @@ -4343,8 +4333,6 @@ struct JSSecurityCallbacks { JSSubsumePrincipalsOp subsumePrincipals; JSObjectPrincipalsFinder findObjectPrincipals; JSCSPEvalChecker contentSecurityPolicyAllows; - JSPushContextPrincipalOp pushContextPrincipal; - JSPopContextPrincipalOp popContextPrincipal; }; extern JS_PUBLIC_API(void) diff --git a/js/src/jsclone.cpp b/js/src/jsclone.cpp index c773696ed4b2..eece5f9729cf 100644 --- a/js/src/jsclone.cpp +++ b/js/src/jsclone.cpp @@ -472,37 +472,6 @@ JSStructuredCloneWriter::startObject(JSObject *obj) return out.writePair(obj->isArray() ? SCTAG_ARRAY_OBJECT : SCTAG_OBJECT_OBJECT, 0); } -class AutoEnterCompartmentAndPushPrincipal : public JSAutoEnterCompartment -{ - public: - bool enter(JSContext *cx, JSObject *target) { - // First, enter the compartment. - if (!JSAutoEnterCompartment::enter(cx, target)) - return false; - - // We only need to push a principal if we changed compartments. - if (state != STATE_OTHER_COMPARTMENT) - return true; - - // Push. - const JSSecurityCallbacks *cb = cx->runtime->securityCallbacks; - if (cb->pushContextPrincipal) - return cb->pushContextPrincipal(cx, target->principals(cx)); - return true; - } - - ~AutoEnterCompartmentAndPushPrincipal() { - // Pop the principal if necessary. - if (state == STATE_OTHER_COMPARTMENT) { - AutoCompartment *ac = getAutoCompartment(); - const JSSecurityCallbacks *cb = ac->context->runtime->securityCallbacks; - if (cb->popContextPrincipal) - cb->popContextPrincipal(ac->context); - } - } -}; - - bool JSStructuredCloneWriter::startWrite(const Value &v) { @@ -529,7 +498,7 @@ JSStructuredCloneWriter::startWrite(const Value &v) // If we unwrapped above, we'll need to enter the underlying compartment. // Let the AutoEnterCompartment do the right thing for us. - AutoEnterCompartmentAndPushPrincipal ac; + JSAutoEnterCompartment ac; if (!ac.enter(context(), obj)) return false; @@ -574,7 +543,7 @@ JSStructuredCloneWriter::write(const Value &v) RootedObject obj(context(), &objs.back().toObject()); // The objects in |obj| can live in other compartments. - AutoEnterCompartmentAndPushPrincipal ac; + JSAutoEnterCompartment ac; if (!ac.enter(context(), obj)) return false; diff --git a/js/xpconnect/shell/xpcshell.cpp b/js/xpconnect/shell/xpcshell.cpp index b0873c5b2823..b75f065e7887 100644 --- a/js/xpconnect/shell/xpcshell.cpp +++ b/js/xpconnect/shell/xpcshell.cpp @@ -1381,20 +1381,6 @@ FullTrustSecMan::GetSubjectPrincipal(nsIPrincipal **_retval) return *_retval ? NS_OK : NS_ERROR_FAILURE; } -/* [noscript] void pushContextPrincipal (in JSContextPtr cx, in JSStackFramePtr fp, in nsIPrincipal principal); */ -NS_IMETHODIMP -FullTrustSecMan::PushContextPrincipal(JSContext * cx, JSStackFrame * fp, nsIPrincipal *principal) -{ - return NS_OK; -} - -/* [noscript] void popContextPrincipal (in JSContextPtr cx); */ -NS_IMETHODIMP -FullTrustSecMan::PopContextPrincipal(JSContext * cx) -{ - return NS_OK; -} - /* [noscript] nsIPrincipal getSystemPrincipal (); */ NS_IMETHODIMP FullTrustSecMan::GetSystemPrincipal(nsIPrincipal **_retval) diff --git a/js/xpconnect/src/XPCWrappedJSClass.cpp b/js/xpconnect/src/XPCWrappedJSClass.cpp index cee742de449c..4193f66835aa 100644 --- a/js/xpconnect/src/XPCWrappedJSClass.cpp +++ b/js/xpconnect/src/XPCWrappedJSClass.cpp @@ -1112,17 +1112,6 @@ nsXPCWrappedJSClass::CheckForException(XPCCallContext & ccx, return NS_ERROR_FAILURE; } -class ContextPrincipalGuard -{ - nsIScriptSecurityManager *ssm; - XPCCallContext &ccx; - public: - ContextPrincipalGuard(XPCCallContext &ccx) - : ssm(nsnull), ccx(ccx) {} - void principalPushed(nsIScriptSecurityManager *ssm) { this->ssm = ssm; } - ~ContextPrincipalGuard() { if (ssm) ssm->PopContextPrincipal(ccx); } -}; - NS_IMETHODIMP nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS* wrapper, uint16_t methodIndex, const XPTMethodDescriptor* info, @@ -1166,7 +1155,6 @@ nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS* wrapper, uint16_t methodIndex, JS::AutoValueVector args(cx); AutoScriptEvaluate scriptEval(cx); - ContextPrincipalGuard principalGuard(ccx); // XXX ASSUMES that retval is last arg. The xpidl compiler ensures this. uint8_t paramCount = info->num_args; @@ -1180,29 +1168,6 @@ nsXPCWrappedJSClass::CallMethod(nsXPCWrappedJS* wrapper, uint16_t methodIndex, xpcc->SetException(nsnull); XPCJSRuntime::Get()->SetPendingException(nsnull); - // This scoping is necessary due to the gotos here. Ugh. - { - // TODO Remove me in favor of security wrappers. - nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager(); - if (ssm) { - nsIPrincipal *objPrincipal = - xpc::AccessCheck::getPrincipal(js::GetObjectCompartment(obj)); - if (objPrincipal) { - JSStackFrame* fp = nsnull; - nsresult rv = - ssm->PushContextPrincipal(ccx, JS_FrameIterator(ccx, &fp), - objPrincipal); - if (NS_FAILED(rv)) { - JS_ReportOutOfMemory(ccx); - retval = NS_ERROR_OUT_OF_MEMORY; - goto pre_call_clean_up; - } - - principalGuard.principalPushed(ssm); - } - } - } - // We use js_Invoke so that the gcthings we use as args will be rooted by // the engine as we do conversions and prepare to do the function call. diff --git a/js/xpconnect/wrappers/CrossOriginWrapper.cpp b/js/xpconnect/wrappers/CrossOriginWrapper.cpp index 9e57fd5d21e1..4faeecb6c4fe 100644 --- a/js/xpconnect/wrappers/CrossOriginWrapper.cpp +++ b/js/xpconnect/wrappers/CrossOriginWrapper.cpp @@ -74,32 +74,12 @@ bool NoWaiverWrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, bool *bp) { *bp = true; // always allowed - nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager(); - if (!ssm) { - return true; - } - - // Note: By the time enter is called here, CrossCompartmentWrapper has - // already pushed the fake stack frame onto cx. Because of this, the frame - // that we're clamping is the one that we want (the one in our compartment). - JSStackFrame *fp = NULL; - nsIPrincipal *principal = GetCompartmentPrincipal(js::GetObjectCompartment(wrappedObject(wrapper))); - nsresult rv = ssm->PushContextPrincipal(cx, JS_FrameIterator(cx, &fp), principal); - if (NS_FAILED(rv)) { - NS_WARNING("Not allowing call because we're out of memory"); - JS_ReportOutOfMemory(cx); - return false; - } return true; } void NoWaiverWrapper::leave(JSContext *cx, JSObject *wrapper) { - nsIScriptSecurityManager *ssm = XPCWrapper::GetSecurityManager(); - if (ssm) { - ssm->PopContextPrincipal(cx); - } } } From 338e1a2a10ff3ba3e06c0d28947b4c0e265915aa Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Jun 2012 23:47:55 +0200 Subject: [PATCH 37/54] Bug 754202 - Remove mContextPrincipal usage from within nsScriptSecurityManager. r=mrbkap --- caps/include/nsScriptSecurityManager.h | 12 ------ caps/src/nsScriptSecurityManager.cpp | 59 ++------------------------ 2 files changed, 4 insertions(+), 67 deletions(-) diff --git a/caps/include/nsScriptSecurityManager.h b/caps/include/nsScriptSecurityManager.h index f7b94fabcd06..84948f351b87 100644 --- a/caps/include/nsScriptSecurityManager.h +++ b/caps/include/nsScriptSecurityManager.h @@ -553,17 +553,6 @@ private: PrintPolicyDB(); #endif - struct ContextPrincipal { - ContextPrincipal(ContextPrincipal *next, JSContext *cx, - JSStackFrame *fp, nsIPrincipal *principal) - : mNext(next), mCx(cx), mFp(fp), mPrincipal(principal) {} - - ContextPrincipal *mNext; - JSContext *mCx; - JSStackFrame *mFp; - nsCOMPtr mPrincipal; - }; - // JS strings we need to clean up on shutdown static jsid sEnabledID; @@ -576,7 +565,6 @@ private: nsCOMPtr mSystemPrincipal; nsCOMPtr mSystemCertificate; - ContextPrincipal *mContextPrincipals; nsInterfaceHashtable mPrincipals; bool mPrefInitialized; bool mIsJavaScriptEnabled; diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp index a49abb172e9e..71f06cca3c1b 100644 --- a/caps/src/nsScriptSecurityManager.cpp +++ b/caps/src/nsScriptSecurityManager.cpp @@ -2238,24 +2238,10 @@ nsScriptSecurityManager::GetPrincipalAndFrame(JSContext *cx, if (cx) { - JSStackFrame *target = nsnull; - nsIPrincipal *targetPrincipal = nsnull; - for (ContextPrincipal *cp = mContextPrincipals; cp; cp = cp->mNext) - { - if (cp->mCx == cx) - { - target = cp->mFp; - targetPrincipal = cp->mPrincipal; - break; - } - } - // Get principals from innermost JavaScript frame. JSStackFrame *fp = nsnull; // tell JS_FrameIterator to start at innermost for (fp = JS_FrameIterator(cx, &fp); fp; fp = JS_FrameIterator(cx, &fp)) { - if (fp == target) - break; nsIPrincipal* result = GetFramePrincipal(cx, fp, rv); if (result) { @@ -2265,25 +2251,6 @@ nsScriptSecurityManager::GetPrincipalAndFrame(JSContext *cx, } } - // If targetPrincipal is non-null, then it means that someone wants to - // clamp the principals on this context to this principal. Note that - // fp might not equal target here (fp might be null) because someone - // could have set aside the frame chain in the meantime. - if (targetPrincipal) - { - if (fp && fp == target) - { - *frameResult = fp; - } - else - { - JSStackFrame *inner = nsnull; - *frameResult = JS_FrameIterator(cx, &inner); - } - - return targetPrincipal; - } - nsIScriptContextPrincipal* scp = GetScriptContextPrincipalFromJSContext(cx); if (scp) @@ -2468,27 +2435,11 @@ nsScriptSecurityManager::IsCapabilityEnabled(const char *capability, JSContext *cx = GetCurrentJSContext(); fp = cx ? JS_FrameIterator(cx, &fp) : nsnull; - JSStackFrame *target = nsnull; - nsIPrincipal *targetPrincipal = nsnull; - for (ContextPrincipal *cp = mContextPrincipals; cp; cp = cp->mNext) - { - if (cp->mCx == cx) - { - target = cp->mFp; - targetPrincipal = cp->mPrincipal; - break; - } - } - if (!fp) { - // No script code on stack. If we had a principal pushed for this - // context and fp is null, then we use that principal. Otherwise, we - // don't have enough information and have to allow execution. - - *result = (targetPrincipal && !target) - ? (targetPrincipal == mSystemPrincipal) - : true; + // No script code on stack. We don't have enough information and have + // to allow execution. + *result = true; return NS_OK; } @@ -2532,7 +2483,7 @@ nsScriptSecurityManager::IsCapabilityEnabled(const char *capability, // the JS engine via JS_EvaluateScript or similar APIs. if (JS_IsGlobalFrame(cx, fp)) break; - } while (fp != target && (fp = JS_FrameIterator(cx, &fp)) != nsnull); + } while ((fp = JS_FrameIterator(cx, &fp)) != nsnull); if (!previousPrincipal) { @@ -3016,7 +2967,6 @@ nsScriptSecurityManager::nsScriptSecurityManager(void) : mOriginToPolicyMap(nsnull), mDefaultPolicy(nsnull), mCapabilities(nsnull), - mContextPrincipals(nsnull), mPrefInitialized(false), mIsJavaScriptEnabled(false), mIsWritingPrefs(false), @@ -3096,7 +3046,6 @@ jsid nsScriptSecurityManager::sEnabledID = JSID_VOID; nsScriptSecurityManager::~nsScriptSecurityManager(void) { Preferences::RemoveObservers(this, kObservedPrefs); - NS_ASSERTION(!mContextPrincipals, "Leaking mContextPrincipals"); delete mOriginToPolicyMap; if(mDefaultPolicy) mDefaultPolicy->Drop(); From bcea5c7acf9e65c9d9c19c311db8501c4aebbbfa Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Jun 2012 23:47:55 +0200 Subject: [PATCH 38/54] Bug 754202 - Remove NoWaiverWrapper. r=mrbkap No more principal pushing! --- js/xpconnect/wrappers/CrossOriginWrapper.cpp | 22 +------------------- js/xpconnect/wrappers/CrossOriginWrapper.h | 13 +----------- js/xpconnect/wrappers/WrapperFactory.cpp | 9 +------- 3 files changed, 3 insertions(+), 41 deletions(-) diff --git a/js/xpconnect/wrappers/CrossOriginWrapper.cpp b/js/xpconnect/wrappers/CrossOriginWrapper.cpp index 4faeecb6c4fe..c260410eedc8 100644 --- a/js/xpconnect/wrappers/CrossOriginWrapper.cpp +++ b/js/xpconnect/wrappers/CrossOriginWrapper.cpp @@ -15,15 +15,7 @@ namespace xpc { -NoWaiverWrapper::NoWaiverWrapper(unsigned flags) : js::CrossCompartmentWrapper(flags) -{ -} - -NoWaiverWrapper::~NoWaiverWrapper() -{ -} - -CrossOriginWrapper::CrossOriginWrapper(unsigned flags) : NoWaiverWrapper(flags) +CrossOriginWrapper::CrossOriginWrapper(unsigned flags) : js::CrossCompartmentWrapper(flags) { } @@ -70,16 +62,4 @@ CrossOriginWrapper::construct(JSContext *cx, JSObject *wrapper, WrapperFactory::WaiveXrayAndWrap(cx, rval); } -bool -NoWaiverWrapper::enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, bool *bp) -{ - *bp = true; // always allowed - return true; -} - -void -NoWaiverWrapper::leave(JSContext *cx, JSObject *wrapper) -{ -} - } diff --git a/js/xpconnect/wrappers/CrossOriginWrapper.h b/js/xpconnect/wrappers/CrossOriginWrapper.h index 1e6209488660..2cfcaeefb874 100644 --- a/js/xpconnect/wrappers/CrossOriginWrapper.h +++ b/js/xpconnect/wrappers/CrossOriginWrapper.h @@ -15,18 +15,7 @@ namespace xpc { -class NoWaiverWrapper : public js::CrossCompartmentWrapper { - public: - NoWaiverWrapper(unsigned flags); - virtual ~NoWaiverWrapper(); - - virtual bool enter(JSContext *cx, JSObject *wrapper, jsid id, Action act, bool *bp) MOZ_OVERRIDE; - virtual void leave(JSContext *cx, JSObject *wrapper) MOZ_OVERRIDE; - - static NoWaiverWrapper singleton; -}; - -class CrossOriginWrapper : public NoWaiverWrapper { +class CrossOriginWrapper : public js::CrossCompartmentWrapper { public: CrossOriginWrapper(unsigned flags); virtual ~CrossOriginWrapper(); diff --git a/js/xpconnect/wrappers/WrapperFactory.cpp b/js/xpconnect/wrappers/WrapperFactory.cpp index 9fd002536c59..d4b4a2014e51 100644 --- a/js/xpconnect/wrappers/WrapperFactory.cpp +++ b/js/xpconnect/wrappers/WrapperFactory.cpp @@ -31,13 +31,6 @@ namespace xpc { // we know to not apply an X-ray wrapper. DirectWrapper WaiveXrayWrapperWrapper(WrapperFactory::WAIVE_XRAY_WRAPPER_FLAG); -// Objects that haven't been explicitly waived, but have been exposed -// to chrome don't want a CrossOriginWrapper, since that deeply-waives -// but need the transparent behavior of a CrossOriginWrapper. The -// NoWaiverWrapper is like a CrossOriginWrapper that can also hand out -// XrayWrappers as return values. -NoWaiverWrapper NoWaiverWrapper::singleton(0); - // When objects for which we waived the X-ray wrapper cross into // chrome, we wrap them into a special cross-compartment wrapper // that transitively extends the waiver to all properties we get @@ -341,7 +334,7 @@ WrapperFactory::Rewrap(JSContext *cx, JSObject *obj, JSObject *wrappedProto, JSO usingXray = true; wrapper = &Xray::singleton; } else { - wrapper = &NoWaiverWrapper::singleton; + wrapper = &CrossCompartmentWrapper::singleton; } } } From 791b8a0a39d60b39f7d6b13f2bc462b06b75c7fd Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Jun 2012 23:47:55 +0200 Subject: [PATCH 39/54] Bug 754202 - Check principal in IsCapabilityEnabled when there's no code on the stack. r=mrbkap --- caps/src/nsScriptSecurityManager.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/caps/src/nsScriptSecurityManager.cpp b/caps/src/nsScriptSecurityManager.cpp index 71f06cca3c1b..3640fbb02e3a 100644 --- a/caps/src/nsScriptSecurityManager.cpp +++ b/caps/src/nsScriptSecurityManager.cpp @@ -2437,10 +2437,11 @@ nsScriptSecurityManager::IsCapabilityEnabled(const char *capability, if (!fp) { - // No script code on stack. We don't have enough information and have - // to allow execution. - *result = true; - + // No script code on stack. Allow access if and only if the subject + // principal is system. + nsresult ignored; + nsIPrincipal *subjectPrin = doGetSubjectPrincipal(&ignored); + *result = (!subjectPrin || subjectPrin == mSystemPrincipal); return NS_OK; } From 6b573f8ee14054173af7234fc5b28c78eaca94f0 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Jun 2012 23:47:56 +0200 Subject: [PATCH 40/54] Bug 754202 - Scope the push of the safe js context such that it doesn't include the call to XHR::Send. r=mrbkap,bent --- dom/workers/XMLHttpRequest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/workers/XMLHttpRequest.cpp b/dom/workers/XMLHttpRequest.cpp index f7ae2569316b..032b70767d5e 100644 --- a/dom/workers/XMLHttpRequest.cpp +++ b/dom/workers/XMLHttpRequest.cpp @@ -1147,9 +1147,9 @@ public: MainThreadRun() { nsCOMPtr variant; - RuntimeService::AutoSafeJSContext cx; if (mBody.data()) { + RuntimeService::AutoSafeJSContext cx; nsIXPConnect* xpc = nsContentUtils::XPConnect(); NS_ASSERTION(xpc, "This should never be null!"); From 446f38447d6f57af9479da790ced50a97adbccb5 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Jun 2012 23:47:56 +0200 Subject: [PATCH 41/54] Bug 754202 - Use the safe JSContext rather than the current JSContext in IndexedDBDatabaseParent::HandleDatabaseEvent. r=bent It's not used for anything substantial, and it can be (or always is?) called with no JS on the stack. --- dom/indexedDB/ipc/IndexedDBParent.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dom/indexedDB/ipc/IndexedDBParent.cpp b/dom/indexedDB/ipc/IndexedDBParent.cpp index 0779700c4b32..39178342b371 100644 --- a/dom/indexedDB/ipc/IndexedDBParent.cpp +++ b/dom/indexedDB/ipc/IndexedDBParent.cpp @@ -377,7 +377,7 @@ IndexedDBDatabaseParent::HandleDatabaseEvent(nsIDOMEvent* aEvent, nsresult rv; if (aType.EqualsLiteral(VERSIONCHANGE_EVT_STR)) { - JSContext* cx = nsContentUtils::GetCurrentJSContext(); + JSContext* cx = nsContentUtils::GetSafeJSContext(); NS_ENSURE_TRUE(cx, NS_ERROR_FAILURE); nsCOMPtr changeEvent = do_QueryInterface(aEvent); From 680c2f89d48c4539c8063facccb016c617e4b6ce Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 28 Jun 2012 23:47:56 +0200 Subject: [PATCH 42/54] Bug 754202 - Only push the context of the event target if the listener is scripted. r=smaug --- content/events/src/nsEventListenerManager.cpp | 50 +++++++++++++------ content/events/src/nsEventListenerManager.h | 13 +++-- content/xbl/src/nsXBLPrototypeHandler.cpp | 6 +++ 3 files changed, 52 insertions(+), 17 deletions(-) diff --git a/content/events/src/nsEventListenerManager.cpp b/content/events/src/nsEventListenerManager.cpp index b6e8e8354faf..8e7267c16256 100644 --- a/content/events/src/nsEventListenerManager.cpp +++ b/content/events/src/nsEventListenerManager.cpp @@ -213,14 +213,20 @@ nsEventListenerManager::AddEventListener(nsIDOMEventListener *aListener, ls->mListener = aListener; ls->mEventType = aType; ls->mTypeAtom = aTypeAtom; - ls->mWrappedJS = false; ls->mFlags = aFlags; ls->mHandlerIsString = false; - nsCOMPtr wjs = do_QueryInterface(aListener); - if (wjs) { - ls->mWrappedJS = true; + // Detect the type of event listener. + nsCOMPtr wjs; + if (aFlags & NS_PRIV_EVENT_FLAG_SCRIPT) { + ls->mListenerType = eJSEventListener; + } else if ((wjs = do_QueryInterface(aListener))) { + ls->mListenerType = eWrappedJSListener; + } else { + ls->mListenerType = eNativeListener; } + + if (aFlags & NS_EVENT_FLAG_SYSTEM_EVENT) { mMayHaveSystemGroupListeners = true; } @@ -458,7 +464,8 @@ nsEventListenerManager::FindJSEventListener(PRUint32 aEventType, for (PRUint32 i = 0; i < count; ++i) { ls = &mListeners.ElementAt(i); if (EVENT_TYPE_EQUALS(ls, aEventType, aTypeAtom) && - ls->mFlags & NS_PRIV_EVENT_FLAG_SCRIPT) { + (ls->mListenerType == eJSEventListener)) + { return ls; } } @@ -796,7 +803,7 @@ nsEventListenerManager::HandleEventSubType(nsListenerStruct* aListenerStruct, // If this is a script handler and we haven't yet // compiled the event handler itself - if ((aListenerStruct->mFlags & NS_PRIV_EVENT_FLAG_SCRIPT) && + if ((aListenerStruct->mListenerType == eJSEventListener) && aListenerStruct->mHandlerIsString) { nsIJSEventListener *jslistener = aListenerStruct->GetJSListener(); result = CompileEventHandlerInternal(aListenerStruct, @@ -863,13 +870,28 @@ nsEventListenerManager::HandleEventInternal(nsPresContext* aPresContext, break; } } + + // Push the appropriate context. Note that we explicitly don't push a + // context in the case that the listener is non-scripted, in which case + // it's the native code's responsibility to push a context if it ever + // enters JS. Ideally we'd do things this way for all scripted callbacks, + // but that would involve a lot of changes and context pushing is going + // away soon anyhow. + // + // NB: Since we're looping here, the no-RePush() case needs to actually be + // a Pop(), otherwise we might end up with whatever was pushed in a + // previous iteration. + if (ls->mListenerType == eNativeListener) { + aPusher->Pop(); + } else if (!aPusher->RePush(aCurrentTarget)) { + continue; + } + nsRefPtr kungFuDeathGrip = ls->mListener; - if (aPusher->RePush(aCurrentTarget)) { - if (NS_FAILED(HandleEventSubType(ls, ls->mListener, *aDOMEvent, - aCurrentTarget, aFlags, - aPusher))) { - aEvent->flags |= NS_EVENT_FLAG_EXCEPTION_THROWN; - } + if (NS_FAILED(HandleEventSubType(ls, ls->mListener, *aDOMEvent, + aCurrentTarget, aFlags, + aPusher))) { + aEvent->flags |= NS_EVENT_FLAG_EXCEPTION_THROWN; } } } @@ -992,7 +1014,7 @@ nsEventListenerManager::GetListenerInfo(nsCOMArray* aList) bool allowsUntrusted = !!(ls.mFlags & NS_PRIV_EVENT_UNTRUSTED_PERMITTED); // If this is a script handler and we haven't yet // compiled the event handler itself go ahead and compile it - if ((ls.mFlags & NS_PRIV_EVENT_FLAG_SCRIPT) && ls.mHandlerIsString) { + if ((ls.mListenerType == eJSEventListener) && ls.mHandlerIsString) { CompileEventHandlerInternal(const_cast(&ls), true, nsnull); } @@ -1107,7 +1129,7 @@ nsEventListenerManager::UnmarkGrayJSListeners() if (jsl) { xpc_UnmarkGrayObject(jsl->GetHandler()); xpc_UnmarkGrayObject(jsl->GetEventScope()); - } else if (ls.mWrappedJS) { + } else if (ls.mListenerType == eWrappedJSListener) { xpc_TryUnmarkWrappedGrayObject(ls.mListener); } } diff --git a/content/events/src/nsEventListenerManager.h b/content/events/src/nsEventListenerManager.h index be47c3d94197..5f12605d3a22 100644 --- a/content/events/src/nsEventListenerManager.h +++ b/content/events/src/nsEventListenerManager.h @@ -31,23 +31,30 @@ class nsCxPusher; class nsIEventListenerInfo; class nsIDocument; +typedef enum +{ + eNativeListener = 0, + eJSEventListener, + eWrappedJSListener +} nsListenerType; + struct nsListenerStruct { nsRefPtr mListener; PRUint32 mEventType; nsCOMPtr mTypeAtom; PRUint16 mFlags; + PRUint8 mListenerType; bool mHandlerIsString; - bool mWrappedJS; nsIJSEventListener* GetJSListener() const { - return (mFlags & NS_PRIV_EVENT_FLAG_SCRIPT) ? + return (mListenerType == eJSEventListener) ? static_cast(mListener.get()) : nsnull; } ~nsListenerStruct() { - if ((mFlags & NS_PRIV_EVENT_FLAG_SCRIPT) && mListener) { + if ((mListenerType == eJSEventListener) && mListener) { static_cast(mListener.get())->Disconnect(); } } diff --git a/content/xbl/src/nsXBLPrototypeHandler.cpp b/content/xbl/src/nsXBLPrototypeHandler.cpp index 1062e2898e48..5bd7c672214c 100644 --- a/content/xbl/src/nsXBLPrototypeHandler.cpp +++ b/content/xbl/src/nsXBLPrototypeHandler.cpp @@ -280,6 +280,12 @@ nsXBLPrototypeHandler::ExecuteHandler(nsIDOMEventTarget* aTarget, scriptTarget = aTarget; } + // We're about to create a new nsJSEventListener, which means that we're + // responsible for pushing the context of the event target. See the similar + // comment in nsEventManagerListener.cpp. + nsCxPusher pusher; + NS_ENSURE_STATE(pusher.Push(aTarget)); + rv = EnsureEventHandler(boundGlobal, boundContext, onEventAtom, handler); NS_ENSURE_SUCCESS(rv, rv); From 5eafa030527be55c2ef613ae3af23d785e59e5e1 Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Thu, 28 Jun 2012 18:06:32 -0400 Subject: [PATCH 43/54] bug 766312 - part 1 - pipeline cancelation could be "undone" r=honzab --- netwerk/protocol/http/nsHttpConnection.cpp | 7 ++++++- netwerk/protocol/http/nsHttpConnection.h | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/netwerk/protocol/http/nsHttpConnection.cpp b/netwerk/protocol/http/nsHttpConnection.cpp index c7ceb841b0f0..111a1e66ad5e 100644 --- a/netwerk/protocol/http/nsHttpConnection.cpp +++ b/netwerk/protocol/http/nsHttpConnection.cpp @@ -49,6 +49,7 @@ nsHttpConnection::nsHttpConnection() , mTotalBytesWritten(0) , mKeepAlive(true) // assume to keep-alive by default , mKeepAliveMask(true) + , mDontReuse(false) , mSupportsPipelining(false) // assume low-grade server , mIsReused(false) , mCompletedProxyConnect(false) @@ -516,6 +517,7 @@ nsHttpConnection::DontReuse() { mKeepAliveMask = false; mKeepAlive = false; + mDontReuse = true; mIdleTimeout = 0; if (mSpdySession) mSpdySession->DontReuse(); @@ -532,12 +534,15 @@ nsHttpConnection::SupportsPipelining() this, mTransaction->PipelineDepth(), mRemainingConnectionUses)); return false; } - return mSupportsPipelining && IsKeepAlive(); + return mSupportsPipelining && IsKeepAlive() && !mDontReuse; } bool nsHttpConnection::CanReuse() { + if (mDontReuse) + return false; + if ((mTransaction ? mTransaction->PipelineDepth() : 0) >= mRemainingConnectionUses) { return false; diff --git a/netwerk/protocol/http/nsHttpConnection.h b/netwerk/protocol/http/nsHttpConnection.h index 14386416010a..4e6dd9b1005a 100644 --- a/netwerk/protocol/http/nsHttpConnection.h +++ b/netwerk/protocol/http/nsHttpConnection.h @@ -217,6 +217,7 @@ private: bool mKeepAlive; bool mKeepAliveMask; + bool mDontReuse; bool mSupportsPipelining; bool mIsReused; bool mCompletedProxyConnect; From f741c62912de8ea9fa2fcc580d698ca695da5380 Mon Sep 17 00:00:00 2001 From: Patrick McManus Date: Thu, 28 Jun 2012 18:24:02 -0400 Subject: [PATCH 44/54] bug 767159 remove sync dns from socks proxy r=biesi --- netwerk/socket/nsSOCKSIOLayer.cpp | 89 ++++++++++++++++++++++++------- 1 file changed, 71 insertions(+), 18 deletions(-) diff --git a/netwerk/socket/nsSOCKSIOLayer.cpp b/netwerk/socket/nsSOCKSIOLayer.cpp index 4e71c758324c..892a6cebadeb 100644 --- a/netwerk/socket/nsSOCKSIOLayer.cpp +++ b/netwerk/socket/nsSOCKSIOLayer.cpp @@ -15,6 +15,9 @@ #include "nsISocketProvider.h" #include "nsSOCKSIOLayer.h" #include "nsNetCID.h" +#include "nsIDNSListener.h" +#include "nsICancelable.h" +#include "nsThreadUtils.h" static PRDescIdentity nsSOCKSIOLayerIdentity; static PRIOMethods nsSOCKSIOLayerMethods; @@ -31,9 +34,12 @@ static PRLogModuleInfo *gSOCKSLog; #endif class nsSOCKSSocketInfo : public nsISOCKSSocketInfo + , public nsIDNSListener { enum State { SOCKS_INITIAL, + SOCKS_DNS_IN_PROGRESS, + SOCKS_DNS_COMPLETE, SOCKS_CONNECTING_TO_PROXY, SOCKS4_WRITE_CONNECT_REQUEST, SOCKS4_READ_CONNECT_RESPONSE, @@ -57,6 +63,7 @@ public: NS_DECL_ISUPPORTS NS_DECL_NSISOCKSSOCKETINFO + NS_DECL_NSIDNSLISTENER void Init(PRInt32 version, const char *proxyHost, @@ -71,6 +78,7 @@ public: private: void HandshakeFinished(PRErrorCode err = 0); + PRStatus StartDNS(PRFileDesc *fd); PRStatus ConnectToProxy(PRFileDesc *fd); PRStatus ContinueConnectingToProxy(PRFileDesc *fd, PRInt16 oflags); PRStatus WriteV4ConnectRequest(); @@ -106,7 +114,10 @@ private: PRUint32 mDataLength; PRUint32 mReadOffset; PRUint32 mAmountToRead; - nsCOMPtr mDnsRec; + nsCOMPtr mDnsRec; + nsCOMPtr mLookup; + nsresult mLookupStatus; + PRFileDesc *mFD; nsCString mDestinationHost; nsCString mProxyHost; @@ -146,7 +157,7 @@ nsSOCKSSocketInfo::Init(PRInt32 version, const char *proxyHost, PRInt32 proxyPor mFlags = flags; } -NS_IMPL_THREADSAFE_ISUPPORTS1(nsSOCKSSocketInfo, nsISOCKSSocketInfo) +NS_IMPL_THREADSAFE_ISUPPORTS2(nsSOCKSSocketInfo, nsISOCKSSocketInfo, nsIDNSListener) NS_IMETHODIMP nsSOCKSSocketInfo::GetExternalProxyAddr(PRNetAddr * *aExternalProxyAddr) @@ -214,6 +225,50 @@ nsSOCKSSocketInfo::HandshakeFinished(PRErrorCode err) mDataLength = 0; mReadOffset = 0; mAmountToRead = 0; + if (mLookup) { + mLookup->Cancel(NS_ERROR_FAILURE); + mLookup = nsnull; + } +} + +PRStatus +nsSOCKSSocketInfo::StartDNS(PRFileDesc *fd) +{ + NS_ABORT_IF_FALSE(!mDnsRec && mState == SOCKS_INITIAL, + "Must be in initial state to make DNS Lookup"); + + nsCOMPtr dns = do_GetService(NS_DNSSERVICE_CONTRACTID); + if (!dns) + return PR_FAILURE; + + mFD = fd; + nsresult rv = dns->AsyncResolve(mProxyHost, 0, this, + NS_GetCurrentThread(), + getter_AddRefs(mLookup)); + + if (NS_FAILED(rv)) { + LOGERROR(("socks: DNS lookup for SOCKS proxy %s failed", + mProxyHost.get())); + return PR_FAILURE; + } + mState = SOCKS_DNS_IN_PROGRESS; + PR_SetError(PR_IN_PROGRESS_ERROR, 0); + return PR_FAILURE; +} + +NS_IMETHODIMP +nsSOCKSSocketInfo::OnLookupComplete(nsICancelable *aRequest, + nsIDNSRecord *aRecord, + nsresult aStatus) +{ + NS_ABORT_IF_FALSE(aRequest == mLookup, "wrong DNS query"); + mLookup = nsnull; + mLookupStatus = aStatus; + mDnsRec = aRecord; + mState = SOCKS_DNS_COMPLETE; + ConnectToProxy(mFD); + mFD = nsnull; + return NS_OK; } PRStatus @@ -222,21 +277,12 @@ nsSOCKSSocketInfo::ConnectToProxy(PRFileDesc *fd) PRStatus status; nsresult rv; - NS_ABORT_IF_FALSE(mState == SOCKS_INITIAL, - "Must be in initial state to make connection!"); + NS_ABORT_IF_FALSE(mState == SOCKS_DNS_COMPLETE, + "Must have DNS to make connection!"); - // If we haven't performed the DNS lookup, do that now. - if (!mDnsRec) { - nsCOMPtr dns = do_GetService(NS_DNSSERVICE_CONTRACTID); - if (!dns) - return PR_FAILURE; - - rv = dns->Resolve(mProxyHost, 0, getter_AddRefs(mDnsRec)); - if (NS_FAILED(rv)) { - LOGERROR(("socks: DNS lookup for SOCKS proxy %s failed", - mProxyHost.get())); - return PR_FAILURE; - } + if (NS_FAILED(mLookupStatus)) { + PR_SetError(PR_BAD_ADDRESS_ERROR, 0); + return PR_FAILURE; } PRInt32 addresses = 0; @@ -291,7 +337,7 @@ nsSOCKSSocketInfo::ContinueConnectingToProxy(PRFileDesc *fd, PRInt16 oflags) PRErrorCode c = PR_GetError(); if (c != PR_WOULD_BLOCK_ERROR && c != PR_IN_PROGRESS_ERROR) { // A connection failure occured, try another address - mState = SOCKS_INITIAL; + mState = SOCKS_DNS_COMPLETE; return ConnectToProxy(fd); } @@ -634,6 +680,11 @@ nsSOCKSSocketInfo::DoHandshake(PRFileDesc *fd, PRInt16 oflags) switch (mState) { case SOCKS_INITIAL: + return StartDNS(fd); + case SOCKS_DNS_IN_PROGRESS: + PR_SetError(PR_IN_PROGRESS_ERROR, 0); + return PR_FAILURE; + case SOCKS_DNS_COMPLETE: return ConnectToProxy(fd); case SOCKS_CONNECTING_TO_PROXY: return ContinueConnectingToProxy(fd, oflags); @@ -696,6 +747,8 @@ PRInt16 nsSOCKSSocketInfo::GetPollFlags() const { switch (mState) { + case SOCKS_DNS_IN_PROGRESS: + case SOCKS_DNS_COMPLETE: case SOCKS_CONNECTING_TO_PROXY: return PR_POLL_EXCEPT | PR_POLL_WRITE; case SOCKS4_WRITE_CONNECT_REQUEST: @@ -1139,7 +1192,7 @@ nsSOCKSIOLayerAddToSocket(PRInt32 family, return NS_ERROR_FAILURE; } - *info = infoObject; + *info = static_cast(infoObject); NS_ADDREF(*info); return NS_OK; } From 3511c2190f15367c293f2fd395c0ac614ad6fb90 Mon Sep 17 00:00:00 2001 From: Jonathan Watt Date: Thu, 28 Jun 2012 23:59:05 +0100 Subject: [PATCH 45/54] Bug 769103 - Get SVG masks working for HTML elements under CSS transforms. r=roc. --- .../mask-transformed-html-01.xhtml | 42 +++++++++++++++++++ .../mask-transformed-html-02.xhtml | 42 +++++++++++++++++++ .../reftests/svg/svg-integration/reftest.list | 4 ++ layout/svg/base/src/nsSVGMaskFrame.cpp | 20 +++++---- 4 files changed, 101 insertions(+), 7 deletions(-) create mode 100644 layout/reftests/svg/svg-integration/mask-transformed-html-01.xhtml create mode 100644 layout/reftests/svg/svg-integration/mask-transformed-html-02.xhtml diff --git a/layout/reftests/svg/svg-integration/mask-transformed-html-01.xhtml b/layout/reftests/svg/svg-integration/mask-transformed-html-01.xhtml new file mode 100644 index 000000000000..25d0ad22685b --- /dev/null +++ b/layout/reftests/svg/svg-integration/mask-transformed-html-01.xhtml @@ -0,0 +1,42 @@ + + + + Test SVG masking of transformed HTML elements + + + + + + + + + + + + + + + +
+
+ + + diff --git a/layout/reftests/svg/svg-integration/mask-transformed-html-02.xhtml b/layout/reftests/svg/svg-integration/mask-transformed-html-02.xhtml new file mode 100644 index 000000000000..c614291a684a --- /dev/null +++ b/layout/reftests/svg/svg-integration/mask-transformed-html-02.xhtml @@ -0,0 +1,42 @@ + + + + Test SVG masking of transformed HTML elements + + + + + + + + + + + + + + + +
+
+ + + diff --git a/layout/reftests/svg/svg-integration/reftest.list b/layout/reftests/svg/svg-integration/reftest.list index ca65ac1f4452..b0a262fe221c 100644 --- a/layout/reftests/svg/svg-integration/reftest.list +++ b/layout/reftests/svg/svg-integration/reftest.list @@ -25,3 +25,7 @@ == mask-html-01-extref-02.xhtml mask-html-01-ref.svg == mask-html-zoomed-01.xhtml mask-html-01-ref.svg == mask-html-xbl-bound-01.html mask-html-01-ref.svg +== mask-transformed-html-01.xhtml ../pass.svg +== mask-transformed-html-02.xhtml ../pass.svg + + diff --git a/layout/svg/base/src/nsSVGMaskFrame.cpp b/layout/svg/base/src/nsSVGMaskFrame.cpp index 4775852c956a..d9016408264e 100644 --- a/layout/svg/base/src/nsSVGMaskFrame.cpp +++ b/layout/svg/base/src/nsSVGMaskFrame.cpp @@ -53,18 +53,14 @@ nsSVGMaskFrame::ComputeMaskAlpha(nsRenderingContext *aContext, gfxContext *gfx = aContext->ThebesContext(); + // Get the clip extents in device space: gfx->Save(); nsSVGUtils::SetClipRect(gfx, aMatrix, maskArea); + gfx->IdentityMatrix(); gfxRect clipExtents = gfx->GetClipExtents(); clipExtents.RoundOut(); gfx->Restore(); -#ifdef DEBUG_tor - fprintf(stderr, "clip extent: %f,%f %fx%f\n", - clipExtents.X(), clipExtents.Y(), - clipExtents.Width(), clipExtents.Height()); -#endif - bool resultOverflows; gfxIntSize surfaceSize = nsSVGUtils::ConvertToSurfaceSize(gfxSize(clipExtents.Width(), @@ -82,10 +78,19 @@ nsSVGMaskFrame::ComputeMaskAlpha(nsRenderingContext *aContext, new gfxImageSurface(surfaceSize, gfxASurface::ImageFormatARGB32); if (!image || image->CairoStatus()) return nsnull; - image->SetDeviceOffset(-clipExtents.TopLeft()); + + // We would like to use gfxImageSurface::SetDeviceOffset() to position + // 'image'. However, we need to set the same matrix on the temporary context + // and pattern that we create below as is currently set on 'gfx'. + // Unfortunately, any device offset set by SetDeviceOffset() is affected by + // the transform passed to the SetMatrix() calls, so to avoid that we account + // for the device offset in the transform rather than use SetDeviceOffset(). + gfxMatrix matrix = + gfx->CurrentMatrix() * gfxMatrix().Translate(-clipExtents.TopLeft()); nsRenderingContext tmpCtx; tmpCtx.Init(this->PresContext()->DeviceContext(), image); + tmpCtx.ThebesContext()->SetMatrix(matrix); mMaskParent = aParent; if (mMaskParentMatrix) { @@ -132,6 +137,7 @@ nsSVGMaskFrame::ComputeMaskAlpha(nsRenderingContext *aContext, } gfxPattern *retval = new gfxPattern(image); + retval->SetMatrix(matrix); NS_IF_ADDREF(retval); return retval; } From c5c17d3f4a382384e76ac30618edf8aa91e1ace5 Mon Sep 17 00:00:00 2001 From: David Mandelin Date: Mon, 25 Jun 2012 18:04:01 -0700 Subject: [PATCH 46/54] Bug 746036: stop querying for the current script in GetNameFromBytecode, r=bhackett --- js/src/jsinterp.cpp | 13 ++++++------ js/src/jsinterpinlines.h | 5 +++-- js/src/jsopcodeinlines.h | 3 +-- js/src/jspropertycache.cpp | 10 ++++----- js/src/methodjit/PolyIC.cpp | 2 +- js/src/methodjit/StubCalls.cpp | 4 ++-- js/src/vm/Stack-inl.h | 39 ---------------------------------- js/src/vm/Stack.h | 1 - 8 files changed, 17 insertions(+), 60 deletions(-) diff --git a/js/src/jsinterp.cpp b/js/src/jsinterp.cpp index e8c632feced3..83f0ec82c432 100644 --- a/js/src/jsinterp.cpp +++ b/js/src/jsinterp.cpp @@ -913,17 +913,16 @@ inline InterpreterFrames::~InterpreterFrames() #if defined(DEBUG) && !defined(JS_THREADSAFE) && !defined(JSGC_ROOT_ANALYSIS) void -js::AssertValidPropertyCacheHit(JSContext *cx, - JSObject *start_, JSObject *found, - PropertyCacheEntry *entry) +js::AssertValidPropertyCacheHit(JSContext *cx, JSObject *start_, + JSObject *found, PropertyCacheEntry *entry) { jsbytecode *pc; - cx->stack.currentScript(&pc); + JSScript *script = cx->stack.currentScript(&pc); uint64_t sample = cx->runtime->gcNumber; PropertyCacheEntry savedEntry = *entry; - RootedPropertyName name(cx, GetNameFromBytecode(cx, pc, JSOp(*pc))); + RootedPropertyName name(cx, GetNameFromBytecode(cx, script, pc, JSOp(*pc))); RootedObject start(cx, start_); JSObject *obj, *pobj; @@ -2330,7 +2329,7 @@ BEGIN_CASE(JSOP_LENGTH) BEGIN_CASE(JSOP_CALLPROP) { RootedValue rval(cx); - if (!GetPropertyOperation(cx, regs.pc, regs.sp[-1], rval.address())) + if (!GetPropertyOperation(cx, script, regs.pc, regs.sp[-1], rval.address())) goto error; TypeScript::Monitor(cx, script, regs.pc, rval.reference()); @@ -2542,7 +2541,7 @@ BEGIN_CASE(JSOP_CALLNAME) { RootedValue &rval = rootValue0; - if (!NameOperation(cx, regs.pc, rval.address())) + if (!NameOperation(cx, script, regs.pc, rval.address())) goto error; PUSH_COPY(rval); diff --git a/js/src/jsinterpinlines.h b/js/src/jsinterpinlines.h index 7fb6d3273f60..bdedbefe9ee9 100644 --- a/js/src/jsinterpinlines.h +++ b/js/src/jsinterpinlines.h @@ -166,7 +166,8 @@ GetPropertyGenericMaybeCallXML(JSContext *cx, JSOp op, HandleObject obj, HandleI } inline bool -GetPropertyOperation(JSContext *cx, jsbytecode *pc, const Value &lval, Value *vp) +GetPropertyOperation(JSContext *cx, JSScript *script, jsbytecode *pc, + const Value &lval, Value *vp) { JS_ASSERT(vp != &lval); @@ -320,7 +321,7 @@ SetPropertyOperation(JSContext *cx, jsbytecode *pc, const Value &lval, const Val } inline bool -NameOperation(JSContext *cx, jsbytecode *pc, Value *vp) +NameOperation(JSContext *cx, JSScript *script, jsbytecode *pc, Value *vp) { RootedObject obj(cx, cx->stack.currentScriptedScopeChain()); diff --git a/js/src/jsopcodeinlines.h b/js/src/jsopcodeinlines.h index ac538851b659..94f8c6b69d44 100644 --- a/js/src/jsopcodeinlines.h +++ b/js/src/jsopcodeinlines.h @@ -11,7 +11,7 @@ namespace js { static inline PropertyName * -GetNameFromBytecode(JSContext *cx, jsbytecode *pc, JSOp op) +GetNameFromBytecode(JSContext *cx, JSScript *script, jsbytecode *pc, JSOp op) { if (op == JSOP_LENGTH) return cx->runtime->atomState.lengthAtom; @@ -21,7 +21,6 @@ GetNameFromBytecode(JSContext *cx, jsbytecode *pc, JSOp op) if (op == JSOP_INSTANCEOF) return cx->runtime->atomState.classPrototypeAtom; - JSScript *script = cx->stack.currentScriptWithDiagnostics(); PropertyName *name; GET_NAME_FROM_BYTECODE(script, pc, 0, name); return name; diff --git a/js/src/jspropertycache.cpp b/js/src/jspropertycache.cpp index efda52643a14..c24619b0d53b 100644 --- a/js/src/jspropertycache.cpp +++ b/js/src/jspropertycache.cpp @@ -122,9 +122,7 @@ PropertyCache::fullTest(JSContext *cx, jsbytecode *pc, JSObject **objp, JSObject PropertyCacheEntry *entry) { JSObject *obj, *pobj, *tmp; -#ifdef DEBUG JSScript *script = cx->stack.currentScript(); -#endif JS_ASSERT(this == &JS_PROPERTY_CACHE(cx)); JS_ASSERT(uint32_t(pc - script->code) < script->length); @@ -137,7 +135,7 @@ PropertyCache::fullTest(JSContext *cx, jsbytecode *pc, JSObject **objp, JSObject if (entry->kpc != pc) { PCMETER(kpcmisses++); - PropertyName *name = GetNameFromBytecode(cx, pc, op); + PropertyName *name = GetNameFromBytecode(cx, script, pc, op); #ifdef DEBUG_notme JSAutoByteString printable; fprintf(stderr, @@ -160,7 +158,7 @@ PropertyCache::fullTest(JSContext *cx, jsbytecode *pc, JSObject **objp, JSObject if (entry->kshape != obj->lastProperty()) { PCMETER(kshapemisses++); - return GetNameFromBytecode(cx, pc, op); + return GetNameFromBytecode(cx, script, pc, op); } /* @@ -193,7 +191,7 @@ PropertyCache::fullTest(JSContext *cx, jsbytecode *pc, JSObject **objp, JSObject if (pobj->lastProperty() == entry->pshape) { #ifdef DEBUG - PropertyName *name = GetNameFromBytecode(cx, pc, op); + PropertyName *name = GetNameFromBytecode(cx, script, pc, op); JS_ASSERT(pobj->nativeContains(cx, NameToId(name))); #endif *pobjp = pobj; @@ -201,7 +199,7 @@ PropertyCache::fullTest(JSContext *cx, jsbytecode *pc, JSObject **objp, JSObject } PCMETER(vcapmisses++); - return GetNameFromBytecode(cx, pc, op); + return GetNameFromBytecode(cx, script, pc, op); } #ifdef DEBUG diff --git a/js/src/methodjit/PolyIC.cpp b/js/src/methodjit/PolyIC.cpp index dee77fbf3d38..d45c39d9dd13 100644 --- a/js/src/methodjit/PolyIC.cpp +++ b/js/src/methodjit/PolyIC.cpp @@ -1924,7 +1924,7 @@ ic::GetProp(VMFrame &f, ic::PICInfo *pic) Value v; if (cached) { - if (!GetPropertyOperation(f.cx, f.pc(), f.regs.sp[-1], &v)) + if (!GetPropertyOperation(f.cx, f.script(), f.pc(), f.regs.sp[-1], &v)) THROW(); } else { if (!obj->getProperty(f.cx, name, &v)) diff --git a/js/src/methodjit/StubCalls.cpp b/js/src/methodjit/StubCalls.cpp index c5445340fa44..cedd81694df7 100644 --- a/js/src/methodjit/StubCalls.cpp +++ b/js/src/methodjit/StubCalls.cpp @@ -98,7 +98,7 @@ void JS_FASTCALL stubs::Name(VMFrame &f) { Value rval; - if (!NameOperation(f.cx, f.pc(), &rval)) + if (!NameOperation(f.cx, f.script(), f.pc(), &rval)) THROW(); f.regs.sp[0] = rval; } @@ -980,7 +980,7 @@ stubs::GetProp(VMFrame &f, PropertyName *name) FrameRegs ®s = f.regs; Value rval; - if (!GetPropertyOperation(cx, f.pc(), f.regs.sp[-1], &rval)) + if (!GetPropertyOperation(cx, f.script(), f.pc(), f.regs.sp[-1], &rval)) THROW(); regs.sp[-1] = rval; diff --git a/js/src/vm/Stack-inl.h b/js/src/vm/Stack-inl.h index 9f21a3a3c1cc..4ad61c13c289 100644 --- a/js/src/vm/Stack-inl.h +++ b/js/src/vm/Stack-inl.h @@ -557,45 +557,6 @@ ContextStack::currentScript(jsbytecode **ppc) const return script; } -inline JSScript * -ContextStack::currentScriptWithDiagnostics(jsbytecode **ppc) const -{ - if (ppc) - *ppc = NULL; - - FrameRegs *regs = maybeRegs(); - StackFrame *fp = regs ? regs->fp() : NULL; - while (fp && fp->isDummyFrame()) - fp = fp->prev(); - if (!fp) - *(int *) 0x10 = 0; - -#ifdef JS_METHODJIT - mjit::CallSite *inlined = regs->inlined(); - if (inlined) { - mjit::JITChunk *chunk = fp->jit()->chunk(regs->pc); - JS_ASSERT(inlined->inlineIndex < chunk->nInlineFrames); - mjit::InlineFrame *frame = &chunk->inlineFrames()[inlined->inlineIndex]; - JSScript *script = frame->fun->script(); - if (script->compartment() != cx_->compartment) - *(int *) 0x20 = 0; - if (ppc) - *ppc = script->code + inlined->pcOffset; - return script; - } -#endif - - JSScript *script = fp->script(); - if (script->compartment() != cx_->compartment) - *(int *) 0x30 = 0; - - if (ppc) - *ppc = fp->pcQuadratic(*this); - if (!script) - *(int *) 0x40 = 0; - return script; -} - inline HandleObject ContextStack::currentScriptedScopeChain() const { diff --git a/js/src/vm/Stack.h b/js/src/vm/Stack.h index f054ea917f28..5668aad3eeb0 100644 --- a/js/src/vm/Stack.h +++ b/js/src/vm/Stack.h @@ -1660,7 +1660,6 @@ class ContextStack /* Get the topmost script and optional pc on the stack. */ inline JSScript *currentScript(jsbytecode **pc = NULL) const; - inline JSScript *currentScriptWithDiagnostics(jsbytecode **pc = NULL) const; /* Get the scope chain for the topmost scripted call on the stack. */ inline HandleObject currentScriptedScopeChain() const; From 7f1bc8526ffcb1a6d792297fd457a8bd8625bd86 Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Thu, 28 Jun 2012 15:26:36 -0400 Subject: [PATCH 47/54] Bug 769362 - Mark the rest of the relative position change tests as random on Mac; r=bzbarsky --- .../reftests/position-dynamic-changes/relative/reftest.list | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/layout/reftests/position-dynamic-changes/relative/reftest.list b/layout/reftests/position-dynamic-changes/relative/reftest.list index 77605ee07a30..f1c5622a7d0b 100644 --- a/layout/reftests/position-dynamic-changes/relative/reftest.list +++ b/layout/reftests/position-dynamic-changes/relative/reftest.list @@ -1,4 +1,4 @@ -fuzzy-if(d2d,85,20) == move-right-bottom.html move-right-bottom-ref.html # Bug 742176 +random-if(cocoaWidget) fuzzy-if(d2d,85,20) == move-right-bottom.html move-right-bottom-ref.html # Bug 742176 random-if(cocoaWidget) fuzzy-if(d2d,85,20) == move-top-left.html move-top-left-ref.html # Bug 688545, bug 742176 -fuzzy-if(d2d,85,20) == move-right-bottom-table.html move-right-bottom-table-ref.html # Bug 742176 +random-if(cocoaWidget) fuzzy-if(d2d,85,20) == move-right-bottom-table.html move-right-bottom-table-ref.html # Bug 742176 random-if(cocoaWidget) fuzzy-if(d2d,85,20) == move-top-left-table.html move-top-left-table-ref.html # Bug 688545, bug 742176 From fca108d98c14e15fa7b35b6c201ebd163334e8d9 Mon Sep 17 00:00:00 2001 From: Jonathan Watt Date: Thu, 28 Jun 2012 20:51:09 +0100 Subject: [PATCH 48/54] Bug 769238 - Rename aEffectsFrame to just aFrame in nsSVGIntegrationUtils. r=dholbert. --- layout/svg/base/src/nsSVGIntegrationUtils.cpp | 26 +++++++++---------- layout/svg/base/src/nsSVGIntegrationUtils.h | 4 +-- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/layout/svg/base/src/nsSVGIntegrationUtils.cpp b/layout/svg/base/src/nsSVGIntegrationUtils.cpp index cc1cfb296217..8f1f785a67f0 100644 --- a/layout/svg/base/src/nsSVGIntegrationUtils.cpp +++ b/layout/svg/base/src/nsSVGIntegrationUtils.cpp @@ -363,13 +363,13 @@ private: void nsSVGIntegrationUtils::PaintFramesWithEffects(nsRenderingContext* aCtx, - nsIFrame* aEffectsFrame, + nsIFrame* aFrame, const nsRect& aDirtyRect, nsDisplayListBuilder* aBuilder, nsDisplayList* aInnerList) { #ifdef DEBUG - nsISVGChildFrame *svgChildFrame = do_QueryFrame(aEffectsFrame); + nsISVGChildFrame *svgChildFrame = do_QueryFrame(aFrame); NS_ASSERTION(!svgChildFrame, "Should never be called on an SVG frame"); #endif @@ -387,7 +387,7 @@ nsSVGIntegrationUtils::PaintFramesWithEffects(nsRenderingContext* aCtx, * + Merge opacity and masking if both used together. */ - float opacity = aEffectsFrame->GetStyleDisplay()->mOpacity; + float opacity = aFrame->GetStyleDisplay()->mOpacity; if (opacity == 0.0f) { return; } @@ -395,7 +395,7 @@ nsSVGIntegrationUtils::PaintFramesWithEffects(nsRenderingContext* aCtx, /* Properties are added lazily and may have been removed by a restyle, so make sure all applicable ones are set again. */ nsIFrame* firstFrame = - nsLayoutUtils::GetFirstContinuationOrSpecialSibling(aEffectsFrame); + nsLayoutUtils::GetFirstContinuationOrSpecialSibling(aFrame); nsSVGEffects::EffectProperties effectProperties = nsSVGEffects::GetEffectProperties(firstFrame); @@ -413,14 +413,14 @@ nsSVGIntegrationUtils::PaintFramesWithEffects(nsRenderingContext* aCtx, gfxContextMatrixAutoSaveRestore matrixAutoSaveRestore(gfx); PRInt32 appUnitsPerDevPixel = - aEffectsFrame->PresContext()->AppUnitsPerDevPixel(); + aFrame->PresContext()->AppUnitsPerDevPixel(); nsPoint firstFrameOffset = GetOffsetToUserSpace(firstFrame); nsPoint offset = (aBuilder->ToReferenceFrame(firstFrame) - firstFrameOffset). ToNearestPixels(appUnitsPerDevPixel). ToAppUnits(appUnitsPerDevPixel); aCtx->Translate(offset); - gfxMatrix cssPxToDevPxMatrix = GetCSSPxToDevPxMatrix(aEffectsFrame); + gfxMatrix cssPxToDevPxMatrix = GetCSSPxToDevPxMatrix(aFrame); bool complexEffects = false; /* Check if we need to do additional operations on this child's @@ -428,7 +428,7 @@ nsSVGIntegrationUtils::PaintFramesWithEffects(nsRenderingContext* aCtx, if (opacity != 1.0f || maskFrame || (clipPathFrame && !isTrivialClip)) { complexEffects = true; gfx->Save(); - aCtx->IntersectClip(aEffectsFrame->GetVisualOverflowRect()); + aCtx->IntersectClip(aFrame->GetVisualOverflowRect()); gfx->PushGroup(gfxASurface::CONTENT_COLOR_ALPHA); } @@ -437,18 +437,18 @@ nsSVGIntegrationUtils::PaintFramesWithEffects(nsRenderingContext* aCtx, */ if (clipPathFrame && isTrivialClip) { gfx->Save(); - clipPathFrame->ClipPaint(aCtx, aEffectsFrame, cssPxToDevPxMatrix); + clipPathFrame->ClipPaint(aCtx, aFrame, cssPxToDevPxMatrix); } /* Paint the child */ if (filterFrame) { - RegularFramePaintCallback callback(aBuilder, aInnerList, aEffectsFrame, + RegularFramePaintCallback callback(aBuilder, aInnerList, aFrame, offset); nsRect dirtyRect = aDirtyRect - offset; - filterFrame->PaintFilteredFrame(aCtx, aEffectsFrame, &callback, &dirtyRect); + filterFrame->PaintFilteredFrame(aCtx, aFrame, &callback, &dirtyRect); } else { gfx->SetMatrix(matrixAutoSaveRestore.Matrix()); - aInnerList->PaintForFrame(aBuilder, aCtx, aEffectsFrame, + aInnerList->PaintForFrame(aBuilder, aCtx, aFrame, nsDisplayList::PAINT_DEFAULT); aCtx->Translate(offset); } @@ -465,14 +465,14 @@ nsSVGIntegrationUtils::PaintFramesWithEffects(nsRenderingContext* aCtx, gfx->PopGroupToSource(); nsRefPtr maskSurface = - maskFrame ? maskFrame->ComputeMaskAlpha(aCtx, aEffectsFrame, + maskFrame ? maskFrame->ComputeMaskAlpha(aCtx, aFrame, cssPxToDevPxMatrix, opacity) : nsnull; nsRefPtr clipMaskSurface; if (clipPathFrame && !isTrivialClip) { gfx->PushGroup(gfxASurface::CONTENT_COLOR_ALPHA); - nsresult rv = clipPathFrame->ClipPaint(aCtx, aEffectsFrame, cssPxToDevPxMatrix); + nsresult rv = clipPathFrame->ClipPaint(aCtx, aFrame, cssPxToDevPxMatrix); clipMaskSurface = gfx->PopGroup(); if (NS_SUCCEEDED(rv) && clipMaskSurface) { diff --git a/layout/svg/base/src/nsSVGIntegrationUtils.h b/layout/svg/base/src/nsSVGIntegrationUtils.h index d75bc806762c..41b34254b1bf 100644 --- a/layout/svg/base/src/nsSVGIntegrationUtils.h +++ b/layout/svg/base/src/nsSVGIntegrationUtils.h @@ -129,12 +129,10 @@ public: /** * Paint non-SVG frame with SVG effects. - * @param aOffset the offset in appunits where aFrame should be positioned - * in aCtx's coordinate system */ static void PaintFramesWithEffects(nsRenderingContext* aCtx, - nsIFrame* aEffectsFrame, const nsRect& aDirtyRect, + nsIFrame* aFrame, const nsRect& aDirtyRect, nsDisplayListBuilder* aBuilder, nsDisplayList* aInnerList); From c0fda032aa77b6092f0aae8d03eded0250f6e1f2 Mon Sep 17 00:00:00 2001 From: Jonathan Watt Date: Thu, 28 Jun 2012 20:51:20 +0100 Subject: [PATCH 49/54] Bug 769242 - Rename nsDisplaySVG to nsDisplayOuterSVG in preparation for adding other SVG display list item types. r=dholbert. --- layout/base/nsDisplayItemTypes.h | 2 +- layout/svg/base/src/nsSVGOuterSVGFrame.cpp | 24 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/layout/base/nsDisplayItemTypes.h b/layout/base/nsDisplayItemTypes.h index 7747b4d0fcf9..0689604f207a 100644 --- a/layout/base/nsDisplayItemTypes.h +++ b/layout/base/nsDisplayItemTypes.h @@ -56,7 +56,7 @@ enum Type { TYPE_SELECTION_OVERLAY, TYPE_SOLID_COLOR, TYPE_SVG_EFFECTS, - TYPE_SVG_EVENT_RECEIVER, + TYPE_SVG_OUTER_SVG, TYPE_TABLE_CELL_BACKGROUND, TYPE_TABLE_CELL_SELECTION, TYPE_TABLE_ROW_BACKGROUND, diff --git a/layout/svg/base/src/nsSVGOuterSVGFrame.cpp b/layout/svg/base/src/nsSVGOuterSVGFrame.cpp index 669f6f6837b5..d85c77263f4a 100644 --- a/layout/svg/base/src/nsSVGOuterSVGFrame.cpp +++ b/layout/svg/base/src/nsSVGOuterSVGFrame.cpp @@ -440,16 +440,16 @@ nsSVGOuterSVGFrame::DidReflow(nsPresContext* aPresContext, //---------------------------------------------------------------------- // container methods -class nsDisplaySVG : public nsDisplayItem { +class nsDisplayOuterSVG : public nsDisplayItem { public: - nsDisplaySVG(nsDisplayListBuilder* aBuilder, - nsSVGOuterSVGFrame* aFrame) : + nsDisplayOuterSVG(nsDisplayListBuilder* aBuilder, + nsSVGOuterSVGFrame* aFrame) : nsDisplayItem(aBuilder, aFrame) { - MOZ_COUNT_CTOR(nsDisplaySVG); + MOZ_COUNT_CTOR(nsDisplayOuterSVG); } #ifdef NS_BUILD_REFCNT_LOGGING - virtual ~nsDisplaySVG() { - MOZ_COUNT_DTOR(nsDisplaySVG); + virtual ~nsDisplayOuterSVG() { + MOZ_COUNT_DTOR(nsDisplayOuterSVG); } #endif @@ -457,12 +457,12 @@ public: HitTestState* aState, nsTArray *aOutFrames); virtual void Paint(nsDisplayListBuilder* aBuilder, nsRenderingContext* aCtx); - NS_DISPLAY_DECL_NAME("SVGEventReceiver", TYPE_SVG_EVENT_RECEIVER) + NS_DISPLAY_DECL_NAME("SVGOuterSVG", TYPE_SVG_OUTER_SVG) }; void -nsDisplaySVG::HitTest(nsDisplayListBuilder* aBuilder, const nsRect& aRect, - HitTestState* aState, nsTArray *aOutFrames) +nsDisplayOuterSVG::HitTest(nsDisplayListBuilder* aBuilder, const nsRect& aRect, + HitTestState* aState, nsTArray *aOutFrames) { nsSVGOuterSVGFrame *outerSVGFrame = static_cast(mFrame); nsRect rectAtOrigin = aRect - ToReferenceFrame(); @@ -482,8 +482,8 @@ nsDisplaySVG::HitTest(nsDisplayListBuilder* aBuilder, const nsRect& aRect, } void -nsDisplaySVG::Paint(nsDisplayListBuilder* aBuilder, - nsRenderingContext* aContext) +nsDisplayOuterSVG::Paint(nsDisplayListBuilder* aBuilder, + nsRenderingContext* aContext) { nsSVGOuterSVGFrame *frame = static_cast(mFrame); @@ -625,7 +625,7 @@ nsSVGOuterSVGFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, nsDisplayList replacedContent; rv = replacedContent.AppendNewToTop( - new (aBuilder) nsDisplaySVG(aBuilder, this)); + new (aBuilder) nsDisplayOuterSVG(aBuilder, this)); NS_ENSURE_SUCCESS(rv, rv); WrapReplacedContentForBorderRadius(aBuilder, &replacedContent, aLists); From e93280a6990119b3a6d083b49d910f1db7dd145f Mon Sep 17 00:00:00 2001 From: Jonathan Watt Date: Thu, 28 Jun 2012 20:51:31 +0100 Subject: [PATCH 50/54] Bug 769306 - Move more code from nsSVGOuterSVGFrame::Paint to nsDisplaySVG::Paint. r=dholbert. --- layout/svg/base/src/nsSVGOuterSVGFrame.cpp | 38 +++++++++++----------- layout/svg/base/src/nsSVGOuterSVGFrame.h | 2 +- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/layout/svg/base/src/nsSVGOuterSVGFrame.cpp b/layout/svg/base/src/nsSVGOuterSVGFrame.cpp index d85c77263f4a..792b80e6f321 100644 --- a/layout/svg/base/src/nsSVGOuterSVGFrame.cpp +++ b/layout/svg/base/src/nsSVGOuterSVGFrame.cpp @@ -485,17 +485,14 @@ void nsDisplayOuterSVG::Paint(nsDisplayListBuilder* aBuilder, nsRenderingContext* aContext) { - nsSVGOuterSVGFrame *frame = static_cast(mFrame); - - if (frame->GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD) - return; - #if defined(DEBUG) && defined(SVG_DEBUG_PAINT_TIMING) PRTime start = PR_Now(); #endif aContext->PushState(); + nsSVGOuterSVGFrame *frame = static_cast(mFrame); + #ifdef XP_MACOSX if (frame->BitmapFallbackEnabled()) { // nquartz fallback paths, which svg tends to trigger, need @@ -504,7 +501,14 @@ nsDisplayOuterSVG::Paint(nsDisplayListBuilder* aBuilder, } #endif - frame->Paint(aBuilder, aContext, mVisibleRect, ToReferenceFrame()); + nsRect viewportRect = + frame->GetContentRectRelativeToSelf() + ToReferenceFrame(); + nsRect clipRect = mVisibleRect.Intersect(viewportRect); + + aContext->IntersectClip(clipRect); + aContext->Translate(viewportRect.TopLeft()); + + frame->Paint(aBuilder, aContext, clipRect - viewportRect.TopLeft()); #ifdef XP_MACOSX if (frame->BitmapFallbackEnabled()) { @@ -619,6 +623,10 @@ nsSVGOuterSVGFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, const nsRect& aDirtyRect, const nsDisplayListSet& aLists) { + if (GetStateBits() & NS_STATE_SVG_NONDISPLAY_CHILD) { + return NS_OK; + } + nsresult rv = DisplayBorderBackgroundOutline(aBuilder, aLists); NS_ENSURE_SUCCESS(rv, rv); @@ -636,20 +644,8 @@ nsSVGOuterSVGFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder, void nsSVGOuterSVGFrame::Paint(const nsDisplayListBuilder* aBuilder, nsRenderingContext* aContext, - const nsRect& aDirtyRect, nsPoint aPt) + const nsRect& aDirtyRect) { - nsRect viewportRect = GetContentRect(); - nsPoint viewportOffset = aPt + viewportRect.TopLeft() - GetPosition(); - viewportRect.MoveTo(viewportOffset); - - nsRect clipRect; - clipRect.IntersectRect(aDirtyRect, viewportRect); - aContext->IntersectClip(clipRect); - aContext->Translate(viewportRect.TopLeft()); - nsRect dirtyRect = clipRect - viewportOffset; - - nsIntRect dirtyPxRect = dirtyRect.ToOutsidePixels(PresContext()->AppUnitsPerDevPixel()); - // Create an SVGAutoRenderState so we can call SetPaintingToWindow on // it, but don't change the render mode: SVGAutoRenderState state(aContext, SVGAutoRenderState::GetRenderMode(aContext)); @@ -658,6 +654,10 @@ nsSVGOuterSVGFrame::Paint(const nsDisplayListBuilder* aBuilder, state.SetPaintingToWindow(true); } + // Convert the (content area relative) dirty rect to dev pixels: + nsIntRect dirtyPxRect = + aDirtyRect.ToOutsidePixels(PresContext()->AppUnitsPerDevPixel()); + nsSVGUtils::PaintFrameWithEffects(aContext, &dirtyPxRect, this); } diff --git a/layout/svg/base/src/nsSVGOuterSVGFrame.h b/layout/svg/base/src/nsSVGOuterSVGFrame.h index 949b0c3b7e5f..1f882844f609 100644 --- a/layout/svg/base/src/nsSVGOuterSVGFrame.h +++ b/layout/svg/base/src/nsSVGOuterSVGFrame.h @@ -67,7 +67,7 @@ public: void Paint(const nsDisplayListBuilder* aBuilder, nsRenderingContext* aContext, - const nsRect& aDirtyRect, nsPoint aPt); + const nsRect& aDirtyRect); #ifdef DEBUG NS_IMETHOD GetFrameName(nsAString& aResult) const From 4887dfc4b311d0ec7b3456ab33ca8d03a2afee52 Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Thu, 28 Jun 2012 17:37:01 -0400 Subject: [PATCH 51/54] Bug 769038 - Margin on root element causes selection handles to appear at wrong spot. r=mbrubeck --- mobile/android/chrome/content/browser.js | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index 7dfc725232c9..fd918bd0d165 100644 --- a/mobile/android/chrome/content/browser.js +++ b/mobile/android/chrome/content/browser.js @@ -1384,6 +1384,7 @@ var SelectionHandler = { // Keeps track of data about the dimensions of the selection cache: null, _active: false, + _viewOffset: null, // The window that holds the selection (can be a sub-frame) get _view() { @@ -1463,6 +1464,10 @@ var SelectionHandler = { this._view = aElement.ownerDocument.defaultView; this._isRTL = (this._view.getComputedStyle(aElement, "").direction == "rtl"); + let computedStyle = this._view.getComputedStyle(this._view.document.documentElement); + this._viewOffset = { top: parseInt(computedStyle.getPropertyValue("margin-top").replace("px", "")), + left: parseInt(computedStyle.getPropertyValue("margin-left").replace("px", "")) }; + // Remove any previous selected or created ranges. Tapping anywhere on a // page will create an empty range. let selection = this._view.getSelection(); @@ -1577,11 +1582,11 @@ var SelectionHandler = { // Update the handle position as it's dragged if (aIsStartHandle) { - this._start.style.left = aX + this._view.scrollX + "px"; - this._start.style.top = aY + this._view.scrollY + "px"; + this._start.style.left = aX + this._view.scrollX - this._viewOffset.left + "px"; + this._start.style.top = aY + this._view.scrollY - this._viewOffset.top + "px"; } else { - this._end.style.left = aX + this._view.scrollX + "px"; - this._end.style.top = aY + this._view.scrollY + "px"; + this._end.style.left = aX + this._view.scrollX - this._viewOffset.left + "px"; + this._end.style.top = aY + this._view.scrollY - this._viewOffset.top + "px"; } let cwu = this._view.QueryInterface(Ci.nsIInterfaceRequestor).getInterface(Ci.nsIDOMWindowUtils); @@ -1696,6 +1701,7 @@ var SelectionHandler = { this._isRTL = false; this._view = null; + this._viewOffset = null; this.cache = null; return selectedText; @@ -1744,11 +1750,11 @@ var SelectionHandler = { // Adjust start/end positions to account for scroll, and account for the dimensions of the // handle elements to ensure the handles point exactly at the ends of the selection. positionHandles: function sh_positionHandles() { - this._start.style.left = (this.cache.start.x + this._view.scrollX - this.HANDLE_WIDTH - this.HANDLE_PADDING) + "px"; - this._start.style.top = (this.cache.start.y + this._view.scrollY - this.HANDLE_VERTICAL_OFFSET) + "px"; + this._start.style.left = (this.cache.start.x + this._view.scrollX - this._viewOffset.left - this.HANDLE_WIDTH - this.HANDLE_PADDING) + "px"; + this._start.style.top = (this.cache.start.y + this._view.scrollY - this._viewOffset.top - this.HANDLE_VERTICAL_OFFSET) + "px"; - this._end.style.left = (this.cache.end.x + this._view.scrollX - this.HANDLE_PADDING) + "px"; - this._end.style.top = (this.cache.end.y + this._view.scrollY - this.HANDLE_VERTICAL_OFFSET) + "px"; + this._end.style.left = (this.cache.end.x + this._view.scrollX - this._viewOffset.left - this.HANDLE_PADDING) + "px"; + this._end.style.top = (this.cache.end.y + this._view.scrollY - this._viewOffset.top - this.HANDLE_VERTICAL_OFFSET) + "px"; }, showHandles: function sh_showHandles() { From 89f9ccd6994fafc5686adb27857ded1a0b36109b Mon Sep 17 00:00:00 2001 From: Margaret Leibovic Date: Thu, 28 Jun 2012 17:37:04 -0400 Subject: [PATCH 52/54] Bug 766556 - Cancel text selection on device rotation. r=mfinkle --- mobile/android/chrome/content/browser.js | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js index fd918bd0d165..1295beae22b7 100644 --- a/mobile/android/chrome/content/browser.js +++ b/mobile/android/chrome/content/browser.js @@ -1428,17 +1428,31 @@ var SelectionHandler = { init: function sh_init() { Services.obs.addObserver(this, "Gesture:SingleTap", false); + Services.obs.addObserver(this, "Window:Resize", false); }, uninit: function sh_uninit() { Services.obs.removeObserver(this, "Gesture:SingleTap", false); + Services.obs.removeObserver(this, "Window:Resize", false); }, observe: function sh_observe(aSubject, aTopic, aData) { - let data = JSON.parse(aData); + switch (aTopic) { + case "Gesture:SingleTap": { + if (!this._active) + return; - if (this._active) - this.endSelection(data.x, data.y); + let data = JSON.parse(aData); + this.endSelection(data.x, data.y); + break; + } + case "Window:Resize": { + // Knowing when the page is done drawing is hard, so let's just cancel + // the selection when the window changes. We should fix this later. + this.endSelection(); + break; + } + } }, notifySelectionChanged: function sh_notifySelectionChanged(aDoc, aSel, aReason) { From 654172f842c86a8f92b375d5c63c3849e9dae46e Mon Sep 17 00:00:00 2001 From: Jason Duell Date: Thu, 28 Jun 2012 14:42:21 -0700 Subject: [PATCH 53/54] Bug 711793 - Delay websocket reconnection after abnormal termination. r=mcmanus --- modules/libpref/src/init/all.js | 4 + .../protocol/websocket/WebSocketChannel.cpp | 683 ++++++++++++------ netwerk/protocol/websocket/WebSocketChannel.h | 19 +- 3 files changed, 486 insertions(+), 220 deletions(-) diff --git a/modules/libpref/src/init/all.js b/modules/libpref/src/init/all.js index 5aca4ca30c24..337e8f6597bb 100644 --- a/modules/libpref/src/init/all.js +++ b/modules/libpref/src/init/all.js @@ -919,6 +919,10 @@ pref("network.websocket.max-connections", 200); // (i.e. wss://) websockets. pref("network.websocket.allowInsecureFromHTTPS", false); +// by default we delay websocket reconnects to same host/port if previous +// connection failed, per RFC 6455 section 7.2.3 +pref("network.websocket.delay-failed-reconnects", true); + // // Server-Sent Events diff --git a/netwerk/protocol/websocket/WebSocketChannel.cpp b/netwerk/protocol/websocket/WebSocketChannel.cpp index ef6460e6301f..b02620ab2f54 100644 --- a/netwerk/protocol/websocket/WebSocketChannel.cpp +++ b/netwerk/protocol/websocket/WebSocketChannel.cpp @@ -34,6 +34,7 @@ #include "nsProxyRelease.h" #include "nsNetUtil.h" #include "mozilla/Attributes.h" +#include "TimeStamp.h" #include "plbase64.h" #include "prmem.h" @@ -41,8 +42,14 @@ #include "prbit.h" #include "zlib.h" +// rather than slurp up all of nsIWebSocket.idl, which lives outside necko, just +// dupe one constant we need from it +#define CLOSE_GOING_AWAY 1001 + extern PRThread *gSocketThread; +using namespace mozilla; + namespace mozilla { namespace net { @@ -77,6 +84,385 @@ NS_IMPL_THREADSAFE_ISUPPORTS11(WebSocketChannel, // some helper classes +//----------------------------------------------------------------------------- +// FailDelayManager +// +// Stores entries (searchable by {host, port}) of connections that have recently +// failed, so we can do delay of reconnects per RFC 6455 Section 7.2.3 +//----------------------------------------------------------------------------- + + +// Initial reconnect delay is randomly chosen between 200-400 ms. +// This is a gentler backoff than the 0-5 seconds the spec offhandedly suggests. +const PRUint32 kWSReconnectInitialBaseDelay = 200; +const PRUint32 kWSReconnectInitialRandomDelay = 200; + +// Base lifetime (in ms) of a FailDelay: kept longer if more failures occur +const PRUint32 kWSReconnectBaseLifeTime = 60 * 1000; +// Maximum reconnect delay (in ms) +const PRUint32 kWSReconnectMaxDelay = 60 * 1000; + +// hold record of failed connections, and calculates needed delay for reconnects +// to same host/port. +class FailDelay +{ +public: + FailDelay(nsCString address, PRInt32 port) + : mAddress(address), mPort(port) + { + mLastFailure = TimeStamp::Now(); + mNextDelay = kWSReconnectInitialBaseDelay + + (rand() % kWSReconnectInitialRandomDelay); + } + + // Called to update settings when connection fails again. + void FailedAgain() + { + mLastFailure = TimeStamp::Now(); + // We use a truncated exponential backoff as suggested by RFC 6455, + // but multiply by 1.5 instead of 2 to be more gradual. + mNextDelay = PR_MIN(kWSReconnectMaxDelay, mNextDelay * 1.5); + LOG(("WebSocket: FailedAgain: host=%s, port=%d: incremented delay to %lu", + mAddress.get(), mPort, mNextDelay)); + } + + // returns 0 if there is no need to delay (i.e. delay interval is over) + PRUint32 RemainingDelay(TimeStamp rightNow) + { + TimeDuration dur = rightNow - mLastFailure; + PRUint32 sinceFail = (PRUint32) dur.ToMilliseconds(); + if (sinceFail > mNextDelay) + return 0; + + return mNextDelay - sinceFail; + } + + bool IsExpired(TimeStamp rightNow) + { + return (mLastFailure + + TimeDuration::FromMilliseconds(kWSReconnectBaseLifeTime + mNextDelay)) + <= rightNow; + } + + nsCString mAddress; // IP address (or hostname if using proxy) + PRInt32 mPort; + +private: + TimeStamp mLastFailure; // Time of last failed attempt + // mLastFailure + mNextDelay is the soonest we'll allow a reconnect + PRUint32 mNextDelay; // milliseconds +}; + +class FailDelayManager +{ +public: + FailDelayManager() + { + mDelaysDisabled = false; + + nsCOMPtr prefService = + do_GetService(NS_PREFSERVICE_CONTRACTID); + bool boolpref = true; + nsresult rv; + rv = prefService->GetBoolPref("network.websocket.delay-failed-reconnects", + &boolpref); + if (NS_SUCCEEDED(rv) && !boolpref) { + mDelaysDisabled = true; + } + } + + void Add(nsCString &address, PRInt32 port) + { + NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); + + if (mDelaysDisabled) + return; + + FailDelay *record = new FailDelay(address, port); + mEntries.AppendElement(record); + } + + // Element returned may not be valid after next main thread event: don't keep + // pointer to it around + FailDelay* Lookup(nsCString &address, PRInt32 port, + PRUint32 *outIndex = nsnull) + { + NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); + + if (mDelaysDisabled) + return nsnull; + + for (PRUint32 i = 0; i < mEntries.Length(); i++) { + FailDelay *fail = mEntries[i]; + if (fail->mAddress.Equals(address) && fail->mPort == port) { + if (outIndex) + *outIndex = i; + return fail; + } + } + return nsnull; + } + + // returns true if channel connects immediately, or false if it's delayed + bool DelayOrBegin(WebSocketChannel *ws) + { + if (!mDelaysDisabled) { + PRUint32 failIndex = 0; + FailDelay *fail = Lookup(ws->mAddress, ws->mPort, &failIndex); + + if (fail) { + TimeStamp rightNow = TimeStamp::Now(); + + PRUint32 remainingDelay = fail->RemainingDelay(rightNow); + if (remainingDelay) { + // reconnecting within delay interval: delay by remaining time + nsresult rv; + ws->mReconnectDelayTimer = + do_CreateInstance("@mozilla.org/timer;1", &rv); + if (NS_SUCCEEDED(rv)) { + rv = ws->mReconnectDelayTimer->InitWithCallback( + ws, remainingDelay, nsITimer::TYPE_ONE_SHOT); + if (NS_SUCCEEDED(rv)) { + LOG(("WebSocket: delaying websocket [this=%p] by %lu ms", + ws, (unsigned long)remainingDelay)); + ws->mConnecting = CONNECTING_DELAYED; + return false; + } + } + // if timer fails (which is very unlikely), drop down to BeginOpen call + } else if (fail->IsExpired(rightNow)) { + mEntries.RemoveElementAt(failIndex); + delete fail; + } + } + } + + // Delays disabled, or no previous failure, or we're reconnecting after scheduled + // delay interval has passed: connect. + // + ws->mConnecting = CONNECTING_IN_PROGRESS; + // If BeginOpen fails, it calls AbortSession, which calls OnStopSession, + // which will ensure any remaining queued connection(s) are scheduled + return ws->BeginOpen(); + } + + // Remove() also deletes all expired entries as it iterates: better for + // battery life than using a periodic timer. + void Remove(nsCString &address, PRInt32 port) + { + NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); + + TimeStamp rightNow = TimeStamp::Now(); + + // iterate from end, to make deletion indexing easier + for (PRInt32 i = mEntries.Length() - 1; i >= 0; --i) { + FailDelay *entry = mEntries[i]; + if ((entry->mAddress.Equals(address) && entry->mPort == port) || + entry->IsExpired(rightNow)) { + mEntries.RemoveElementAt(i); + delete entry; + } + } + } + +private: + nsTArray mEntries; + bool mDelaysDisabled; +}; + +//----------------------------------------------------------------------------- +// nsWSAdmissionManager +// +// 1) Ensures that only one websocket at a time is CONNECTING to a given IP +// address (or hostname, if using proxy), per RFC 6455 Section 4.1. +// 2) Delays reconnects to IP/host after connection failure, per Section 7.2.3 +//----------------------------------------------------------------------------- + +class nsWSAdmissionManager +{ +public: + nsWSAdmissionManager() : mSessionCount(0) + { + MOZ_COUNT_CTOR(nsWSAdmissionManager); + } + + class nsOpenConn + { + public: + nsOpenConn(nsCString &addr, WebSocketChannel *channel) + : mAddress(addr), mChannel(channel) { MOZ_COUNT_CTOR(nsOpenConn); } + ~nsOpenConn() { MOZ_COUNT_DTOR(nsOpenConn); } + + nsCString mAddress; + WebSocketChannel *mChannel; + }; + + ~nsWSAdmissionManager() + { + MOZ_COUNT_DTOR(nsWSAdmissionManager); + for (PRUint32 i = 0; i < mQueue.Length(); i++) + delete mQueue[i]; + } + + // Determine if we will open connection immediately (returns true), or + // delay/queue the connection (returns false) + bool ConditionallyConnect(WebSocketChannel *ws) + { + NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); + NS_ABORT_IF_FALSE(ws->mConnecting == NOT_CONNECTING, "opening state"); + + // If there is already another WS channel connecting to this IP address, + // defer BeginOpen and mark as waiting in queue. + bool found = (IndexOf(ws->mAddress) >= 0); + + // Always add ourselves to queue, even if we'll connect immediately + nsOpenConn *newdata = new nsOpenConn(ws->mAddress, ws); + mQueue.AppendElement(newdata); + + if (found) { + ws->mConnecting = CONNECTING_QUEUED; + return false; + } + + return mFailures.DelayOrBegin(ws); + } + + bool OnConnected(WebSocketChannel *aChannel) + { + NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); + NS_ABORT_IF_FALSE(aChannel->mConnecting == CONNECTING_IN_PROGRESS, + "Channel completed connect, but not connecting?"); + + aChannel->mConnecting = NOT_CONNECTING; + + // Remove from queue + RemoveFromQueue(aChannel); + + // Connection succeeded, so stop keeping track of any previous failures + mFailures.Remove(aChannel->mAddress, aChannel->mPort); + + // Check for queued connections to same host. + // Note: still need to check for failures, since next websocket with same + // host may have different port + return ConnectNext(aChannel->mAddress); + } + + // Called every time a websocket channel ends its session (including going away + // w/o ever successfully creating a connection) + bool OnStopSession(WebSocketChannel *aChannel, nsresult aReason) + { + NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); + + if (NS_FAILED(aReason)) { + // Have we seen this failure before? + FailDelay *knownFailure = mFailures.Lookup(aChannel->mAddress, + aChannel->mPort); + if (knownFailure) { + // repeated failure to connect: increase delay for next connection + knownFailure->FailedAgain(); + } else { + // new connection failure: record it. + LOG(("WebSocket: connection to %s, %d failed: [this=%p]", + aChannel->mAddress.get(), (int)aChannel->mPort, aChannel)); + mFailures.Add(aChannel->mAddress, aChannel->mPort); + } + } + + if (aChannel->mConnecting) { + // Only way a connecting channel may get here w/o failing is if it was + // closed with GOING_AWAY (1001) because of navigation, tab close, etc. + NS_ABORT_IF_FALSE(NS_FAILED(aReason) || + aChannel->mScriptCloseCode == CLOSE_GOING_AWAY, + "websocket closed while connecting w/o failing?"); + + RemoveFromQueue(aChannel); + + bool wasNotQueued = (aChannel->mConnecting != CONNECTING_QUEUED); + aChannel->mConnecting = NOT_CONNECTING; + if (wasNotQueued) + return ConnectNext(aChannel->mAddress); + } + return false; + } + + bool ConnectNext(nsCString &hostName) + { + PRInt32 index = IndexOf(hostName); + if (index >= 0) { + WebSocketChannel *chan = mQueue[index]->mChannel; + + NS_ABORT_IF_FALSE(chan->mConnecting == CONNECTING_QUEUED, + "transaction not queued but in queue"); + LOG(("WebSocket: ConnectNext: found channel [this=%p] in queue", chan)); + + return mFailures.DelayOrBegin(chan); + } + + return false; + } + + void IncrementSessionCount() + { + PR_ATOMIC_INCREMENT(&mSessionCount); + } + + void DecrementSessionCount() + { + PR_ATOMIC_DECREMENT(&mSessionCount); + } + + PRInt32 SessionCount() + { + return mSessionCount; + } + +private: + + void RemoveFromQueue(WebSocketChannel *aChannel) + { + PRInt32 index = IndexOf(aChannel); + NS_ABORT_IF_FALSE(index >= 0, "connection to remove not in queue"); + if (index >= 0) { + nsOpenConn *olddata = mQueue[index]; + mQueue.RemoveElementAt(index); + delete olddata; + } + } + + PRInt32 IndexOf(nsCString &aStr) + { + for (PRUint32 i = 0; i < mQueue.Length(); i++) + if (aStr == (mQueue[i])->mAddress) + return i; + return -1; + } + + PRInt32 IndexOf(WebSocketChannel *aChannel) + { + for (PRUint32 i = 0; i < mQueue.Length(); i++) + if (aChannel == (mQueue[i])->mChannel) + return i; + return -1; + } + + // SessionCount might be decremented from the main or the socket + // thread, so manage it with atomic counters + PRInt32 mSessionCount; + + // Queue for websockets that have not completed connecting yet. + // The first nsOpenConn with a given address will be either be + // CONNECTING_IN_PROGRESS or CONNECTING_DELAYED. Later ones with the same + // hostname must be CONNECTING_QUEUED. + // + // We could hash hostnames instead of using a single big vector here, but the + // dataset is expected to be small. + nsTArray mQueue; + + FailDelayManager mFailures; +}; + +static nsWSAdmissionManager *sWebSocketAdmissions = nsnull; + //----------------------------------------------------------------------------- // CallOnMessageAvailable //----------------------------------------------------------------------------- @@ -121,14 +507,18 @@ public: NS_DECL_ISUPPORTS CallOnStop(WebSocketChannel *aChannel, - nsresult aData) + nsresult aReason) : mChannel(aChannel), - mData(aData) {} + mReason(aReason) {} NS_IMETHOD Run() { NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); - mChannel->mListener->OnStop(mChannel->mContext, mData); + + sWebSocketAdmissions->OnStopSession(mChannel, mReason); + + if (mChannel->mListener) + mChannel->mListener->OnStop(mChannel->mContext, mReason); return NS_OK; } @@ -136,7 +526,7 @@ private: ~CallOnStop() {} nsRefPtr mChannel; - nsresult mData; + nsresult mReason; }; NS_IMPL_THREADSAFE_ISUPPORTS1(CallOnStop, nsIRunnable) @@ -370,176 +760,6 @@ private: }; NS_IMPL_THREADSAFE_ISUPPORTS1(OutboundEnqueuer, nsIRunnable) -//----------------------------------------------------------------------------- -// nsWSAdmissionManager -//----------------------------------------------------------------------------- - -// Section 4.1 requires that only a single websocket at a time can be CONNECTING -// to any given IP address (or hostname, if proxy doing DNS for us). This class -// ensures that we delay connecting until any pending connection for the same -// IP/addr is complete (i.e. until before the 101 upgrade complete response -// comes back and an 'open' javascript event is created) - -class nsWSAdmissionManager -{ -public: - nsWSAdmissionManager() : mSessionCount(0) - { - MOZ_COUNT_CTOR(nsWSAdmissionManager); - } - - class nsOpenConn - { - public: - nsOpenConn(nsCString &addr, WebSocketChannel *channel) - : mAddress(addr), mChannel(channel) { MOZ_COUNT_CTOR(nsOpenConn); } - ~nsOpenConn() { MOZ_COUNT_DTOR(nsOpenConn); } - - nsCString mAddress; - WebSocketChannel *mChannel; - }; - - ~nsWSAdmissionManager() - { - MOZ_COUNT_DTOR(nsWSAdmissionManager); - for (PRUint32 i = 0; i < mData.Length(); i++) - delete mData[i]; - } - - bool ConditionallyConnect(nsCString &aStr, WebSocketChannel *ws) - { - NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); - - // if aStr is not in mData then we return true, else false. - // in either case aStr is then added to mData - meaning - // there will be duplicates when this function has been - // called with the same parameter multiple times. - - // we could hash this, but the dataset is expected to be - // small - - // There may already be another WS channel connecting to this IP address, in - // which case we'll still create a new nsOpenConn but defer BeginOpen until - // that channel completes connecting. - bool found = (IndexOf(aStr) >= 0); - nsOpenConn *newdata = new nsOpenConn(aStr, ws); - mData.AppendElement(newdata); - - NS_ABORT_IF_FALSE (!ws->mOpenRunning && !ws->mOpenBlocked, - "opening state"); - - if (!found) { - ws->mOpenRunning = 1; - ws->BeginOpen(); - } else { - ws->mOpenBlocked = 1; - } - - return !found; - } - - bool Complete(WebSocketChannel *aChannel) - { - NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); - NS_ABORT_IF_FALSE(!aChannel->mOpenBlocked, - "blocked, but complete nsOpenConn"); - - // It is possible this has already been canceled - if (!aChannel->mOpenRunning) - return false; - - PRInt32 index = IndexOf(aChannel); - NS_ABORT_IF_FALSE(index >= 0, "completed connection not in open list"); - - aChannel->mOpenRunning = 0; - nsOpenConn *olddata = mData[index]; - mData.RemoveElementAt(index); - delete olddata; - - // are there more of the same address pending dispatch? - return ConnectNext(aChannel->mAddress); - } - - bool Cancel(WebSocketChannel *aChannel) - { - NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); - PRInt32 index = IndexOf(aChannel); - NS_ABORT_IF_FALSE(index >= 0, "Cancelled connection not in open list"); - NS_ABORT_IF_FALSE(aChannel->mOpenRunning ^ aChannel->mOpenBlocked, - "cancel without running xor blocked"); - - bool wasRunning = aChannel->mOpenRunning; - aChannel->mOpenRunning = 0; - aChannel->mOpenBlocked = 0; - nsOpenConn *olddata = mData[index]; - mData.RemoveElementAt(index); - delete olddata; - - // if we are running we can run another one - if (wasRunning) - return ConnectNext(aChannel->mAddress); - - return false; - } - - bool ConnectNext(nsCString &hostName) - { - PRInt32 index = IndexOf(hostName); - if (index >= 0) { - WebSocketChannel *chan = mData[index]->mChannel; - - NS_ABORT_IF_FALSE(chan->mOpenBlocked, - "transaction not blocked but in queue"); - NS_ABORT_IF_FALSE(!chan->mOpenRunning, "transaction already running"); - - chan->mOpenBlocked = 0; - chan->mOpenRunning = 1; - chan->BeginOpen(); - return true; - } - - return false; - } - - void IncrementSessionCount() - { - PR_ATOMIC_INCREMENT(&mSessionCount); - } - - void DecrementSessionCount() - { - PR_ATOMIC_DECREMENT(&mSessionCount); - } - - PRInt32 SessionCount() - { - return mSessionCount; - } - -private: - nsTArray mData; - - PRInt32 IndexOf(nsCString &aStr) - { - for (PRUint32 i = 0; i < mData.Length(); i++) - if (aStr == (mData[i])->mAddress) - return i; - return -1; - } - - PRInt32 IndexOf(WebSocketChannel *aChannel) - { - for (PRUint32 i = 0; i < mData.Length(); i++) - if (aChannel == (mData[i])->mChannel) - return i; - return -1; - } - - // SessionCount might be decremented from the main or the socket - // thread, so manage it with atomic counters - PRInt32 mSessionCount; -}; - //----------------------------------------------------------------------------- // nsWSCompression // @@ -667,15 +887,15 @@ private: PRUint8 mBuffer[kBufferLen]; }; -static nsWSAdmissionManager *sWebSocketAdmissions = nsnull; - //----------------------------------------------------------------------------- // WebSocketChannel //----------------------------------------------------------------------------- WebSocketChannel::WebSocketChannel() : + mPort(0), mCloseTimeout(20000), mOpenTimeout(20000), + mConnecting(NOT_CONNECTING), mPingTimeout(0), mPingResponseTimeout(10000), mMaxConcurrentConnections(200), @@ -691,8 +911,6 @@ WebSocketChannel::WebSocketChannel() : mAutoFollowRedirects(0), mReleaseOnTransmit(0), mTCPClosed(0), - mOpenBlocked(0), - mOpenRunning(0), mChannelWasOpened(0), mDataStarted(0), mIncrementedSessionCount(0), @@ -727,7 +945,7 @@ WebSocketChannel::~WebSocketChannel() // this stop is a nop if the normal connect/close is followed StopSession(NS_ERROR_UNEXPECTED); - NS_ABORT_IF_FALSE(!mOpenRunning && !mOpenBlocked, "op"); + NS_ABORT_IF_FALSE(mConnecting == NOT_CONNECTING, "op"); moz_free(mBuffer); moz_free(mDynamicOutput); @@ -781,7 +999,7 @@ WebSocketChannel::Shutdown() sWebSocketAdmissions = nsnull; } -nsresult +bool WebSocketChannel::BeginOpen() { LOG(("WebSocketChannel::BeginOpen() %p\n", this)); @@ -793,29 +1011,40 @@ WebSocketChannel::BeginOpen() LOG(("WebSocketChannel::BeginOpen: Resuming Redirect\n")); rv = mRedirectCallback->OnRedirectVerifyCallback(NS_OK); mRedirectCallback = nsnull; - return rv; + return false; } nsCOMPtr localChannel = do_QueryInterface(mChannel, &rv); if (NS_FAILED(rv)) { LOG(("WebSocketChannel::BeginOpen: cannot async open\n")); AbortSession(NS_ERROR_UNEXPECTED); - return rv; + return false; } rv = localChannel->AsyncOpen(this, mHttpChannel); if (NS_FAILED(rv)) { LOG(("WebSocketChannel::BeginOpen: cannot async open\n")); AbortSession(NS_ERROR_CONNECTION_REFUSED); - return rv; + return false; } mChannelWasOpened = 1; mOpenTimer = do_CreateInstance("@mozilla.org/timer;1", &rv); - if (NS_SUCCEEDED(rv)) - mOpenTimer->InitWithCallback(this, mOpenTimeout, nsITimer::TYPE_ONE_SHOT); + if (NS_FAILED(rv)) { + LOG(("WebSocketChannel::BeginOpen: cannot create open timer\n")); + AbortSession(NS_ERROR_UNEXPECTED); + return false; + } - return rv; + rv = mOpenTimer->InitWithCallback(this, mOpenTimeout, + nsITimer::TYPE_ONE_SHOT); + if (NS_FAILED(rv)) { + LOG(("WebSocketChannel::BeginOpen: cannot initialize open timer\n")); + AbortSession(NS_ERROR_UNEXPECTED); + return false; + } + + return true; } bool @@ -1578,9 +1807,6 @@ WebSocketChannel::StopSession(nsresult reason) mCallbacks = nsnull; } - if (mOpenRunning || mOpenBlocked) - sWebSocketAdmissions->Cancel(this); - if (mCloseTimer) { mCloseTimer->Cancel(); mCloseTimer = nsnull; @@ -1591,6 +1817,11 @@ WebSocketChannel::StopSession(nsresult reason) mOpenTimer = nsnull; } + if (mReconnectDelayTimer) { + mReconnectDelayTimer->Cancel(); + mReconnectDelayTimer = nsnull; + } + if (mPingTimer) { mPingTimer->Cancel(); mPingTimer = nsnull; @@ -1657,8 +1888,7 @@ WebSocketChannel::StopSession(nsresult reason) if (!mCalledOnStop) { mCalledOnStop = 1; - if (mListener) - NS_DispatchToMainThread(new CallOnStop(this, reason)); + NS_DispatchToMainThread(new CallOnStop(this, reason)); } return; @@ -1893,6 +2123,10 @@ WebSocketChannel::ApplyForAdmission() rv = mURI->GetHost(hostName); NS_ENSURE_SUCCESS(rv, rv); mAddress = hostName; + rv = mURI->GetPort(&mPort); + NS_ENSURE_SUCCESS(rv, rv); + if (mPort == -1) + mPort = (mEncrypted ? kDefaultWSSPort : kDefaultWSPort); // expect the callback in ::OnLookupComplete LOG(("WebSocketChannel::ApplyForAdmission: checking for concurrent open\n")); @@ -1913,7 +2147,12 @@ WebSocketChannel::StartWebsocketData() LOG(("WebSocketChannel::StartWebsocketData() %p", this)); NS_ABORT_IF_FALSE(!mDataStarted, "StartWebsocketData twice"); mDataStarted = 1; - + + // We're now done CONNECTING, which means we can now open another, + // perhaps parallel, connection to the same host if one + // is pending + sWebSocketAdmissions->OnConnected(this); + LOG(("WebSocketChannel::StartWebsocketData Notifying Listener %p\n", mListener.get())); @@ -1953,7 +2192,7 @@ WebSocketChannel::OnLookupComplete(nsICancelable *aRequest, LOG(("WebSocketChannel::OnLookupComplete: Failed GetNextAddr\n")); } - if (sWebSocketAdmissions->ConditionallyConnect(mAddress, this)) { + if (sWebSocketAdmissions->ConditionallyConnect(this)) { LOG(("WebSocketChannel::OnLookupComplete: Proceeding with Open\n")); } else { LOG(("WebSocketChannel::OnLookupComplete: Deferring Open\n")); @@ -2077,17 +2316,19 @@ WebSocketChannel::AsyncOnChannelRedirect( return rv; } - // We cannot just tell the callback OK right now due to the 1 connect at a - // time policy. First we need to complete the old location and then start the - // lookup chain for the new location - once that is complete and we have been - // admitted, OnRedirectVerifyCallback(NS_OK) will be called out of BeginOpen() - - sWebSocketAdmissions->Complete(this); - mAddress.Truncate(); + // Redirected-to URI may need to be delayed by 1-connecting-per-host and + // delay-after-fail algorithms. So hold off calling OnRedirectVerifyCallback + // until BeginOpen, when we know it's OK to proceed with new channel. mRedirectCallback = callback; - mChannelWasOpened = 0; + // Mark old channel as successfully connected so we'll clear any FailDelay + // associated with the old URI. Note: no need to also call OnStopSession: + // it's a no-op for successful, already-connected channels. + sWebSocketAdmissions->OnConnected(this); + // ApplyForAdmission as if we were starting from fresh... + mAddress.Truncate(); + mChannelWasOpened = 0; rv = ApplyForAdmission(); if (NS_FAILED(rv)) { LOG(("WebSocketChannel: Redirect failed due to DNS failure\n")); @@ -2127,6 +2368,16 @@ WebSocketChannel::Notify(nsITimer *timer) return NS_OK; AbortSession(NS_ERROR_NET_TIMEOUT); + } else if (timer == mReconnectDelayTimer) { + NS_ABORT_IF_FALSE(mConnecting == CONNECTING_DELAYED, + "woke up from delay w/o being delayed?"); + + mReconnectDelayTimer = nsnull; + LOG(("WebSocketChannel: connecting [this=%p] after reconnect delay", this)); + // - if BeginOpen fails, it calls AbortSession, which calls OnStopSession, + // which will ensure any remaining queued connection(s) are scheduled + mConnecting = CONNECTING_IN_PROGRESS; + this->BeginOpen(); } else if (timer == mPingTimer) { NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "not socket thread"); @@ -2351,12 +2602,6 @@ WebSocketChannel::Close(PRUint16 code, const nsACString & reason) return NS_OK; } - if (!mTransport) { - LOG(("WebSocketChannel::Close() without transport - aborting.")); - AbortSession(NS_ERROR_NOT_CONNECTED); - return NS_ERROR_NOT_CONNECTED; - } - // The API requires the UTF-8 string to be 123 or less bytes if (reason.Length() > 123) return NS_ERROR_ILLEGAL_VALUE; @@ -2365,6 +2610,20 @@ WebSocketChannel::Close(PRUint16 code, const nsACString & reason) mScriptCloseReason = reason; mScriptCloseCode = code; + if (!mTransport) { + nsresult rv; + if (code == CLOSE_GOING_AWAY) { + // Not an error: for example, tab has closed or navigated away + LOG(("WebSocketChannel::Close() GOING_AWAY without transport.")); + rv = NS_OK; + } else { + LOG(("WebSocketChannel::Close() without transport - error.")); + rv = NS_ERROR_NOT_CONNECTED; + } + StopSession(rv); + return rv; + } + return mSocketThread->Dispatch( new OutboundEnqueuer(this, new OutboundMessage(kMsgTypeFin, nsnull)), nsIEventTarget::DISPATCH_NORMAL); @@ -2470,16 +2729,6 @@ WebSocketChannel::OnStartRequest(nsIRequest *aRequest, NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); NS_ABORT_IF_FALSE(!mRecvdHttpOnStartRequest, "OTA duplicated"); - // Generating the onStart event will take us out of the - // CONNECTING state which means we can now open another, - // perhaps parallel, connection to the same host if one - // is pending - - if (sWebSocketAdmissions->Complete(this)) - LOG(("WebSocketChannel::OnStartRequest: Starting Pending Open\n")); - else - LOG(("WebSocketChannel::OnStartRequest: No More Pending Opens\n")); - if (mOpenTimer) { mOpenTimer->Cancel(); mOpenTimer = nsnull; diff --git a/netwerk/protocol/websocket/WebSocketChannel.h b/netwerk/protocol/websocket/WebSocketChannel.h index 3fcb34267e65..f18fe0850f75 100644 --- a/netwerk/protocol/websocket/WebSocketChannel.h +++ b/netwerk/protocol/websocket/WebSocketChannel.h @@ -42,6 +42,14 @@ class CallOnStop; class CallOnServerClose; class CallAcknowledge; +// Used to enforce "1 connecting websocket per host" rule, and reconnect delays +enum wsConnectingState { + NOT_CONNECTING = 0, // Not yet (or no longer) trying to open connection + CONNECTING_QUEUED, // Waiting for other ws to same host to finish opening + CONNECTING_DELAYED, // Delayed by "reconnect after failure" algorithm + CONNECTING_IN_PROGRESS // Started connection: waiting for result +}; + class WebSocketChannel : public BaseWebSocketChannel, public nsIHttpUpgradeListener, public nsIStreamListener, @@ -101,6 +109,7 @@ protected: private: friend class OutboundEnqueuer; friend class nsWSAdmissionManager; + friend class FailDelayManager; friend class CallOnMessageAvailable; friend class CallOnStop; friend class CallOnServerClose; @@ -117,7 +126,7 @@ private: void GeneratePong(PRUint8 *payload, PRUint32 len); void GeneratePing(); - nsresult BeginOpen(); + bool BeginOpen(); nsresult HandleExtensions(); nsresult SetupRequest(); nsresult ApplyForAdmission(); @@ -149,7 +158,11 @@ private: nsCOMPtr mRandomGenerator; nsCString mHashedSecret; + + // Used as key for connection managment: Initially set to hostname from URI, + // then to IP address (unless we're leaving DNS resolution to a proxy server) nsCString mAddress; + PRInt32 mPort; // WS server port nsCOMPtr mTransport; nsCOMPtr mSocketIn; @@ -160,6 +173,8 @@ private: nsCOMPtr mOpenTimer; PRUint32 mOpenTimeout; /* milliseconds */ + wsConnectingState mConnecting; /* 0 if not connecting */ + nsCOMPtr mReconnectDelayTimer; nsCOMPtr mPingTimer; PRUint32 mPingTimeout; /* milliseconds */ @@ -183,8 +198,6 @@ private: PRUint32 mAutoFollowRedirects : 1; PRUint32 mReleaseOnTransmit : 1; PRUint32 mTCPClosed : 1; - PRUint32 mOpenBlocked : 1; - PRUint32 mOpenRunning : 1; PRUint32 mChannelWasOpened : 1; PRUint32 mDataStarted : 1; PRUint32 mIncrementedSessionCount : 1; From ebd1c1462099f18be002b717a8ca22fe30e2b2e1 Mon Sep 17 00:00:00 2001 From: Jason Duell Date: Thu, 28 Jun 2012 16:36:14 -0700 Subject: [PATCH 54/54] Backed out changeset 25663e7d7f56 a=memory leak --- modules/libpref/src/init/all.js | 4 - .../protocol/websocket/WebSocketChannel.cpp | 683 ++++++------------ netwerk/protocol/websocket/WebSocketChannel.h | 19 +- 3 files changed, 220 insertions(+), 486 deletions(-) diff --git a/modules/libpref/src/init/all.js b/modules/libpref/src/init/all.js index 337e8f6597bb..5aca4ca30c24 100644 --- a/modules/libpref/src/init/all.js +++ b/modules/libpref/src/init/all.js @@ -919,10 +919,6 @@ pref("network.websocket.max-connections", 200); // (i.e. wss://) websockets. pref("network.websocket.allowInsecureFromHTTPS", false); -// by default we delay websocket reconnects to same host/port if previous -// connection failed, per RFC 6455 section 7.2.3 -pref("network.websocket.delay-failed-reconnects", true); - // // Server-Sent Events diff --git a/netwerk/protocol/websocket/WebSocketChannel.cpp b/netwerk/protocol/websocket/WebSocketChannel.cpp index b02620ab2f54..ef6460e6301f 100644 --- a/netwerk/protocol/websocket/WebSocketChannel.cpp +++ b/netwerk/protocol/websocket/WebSocketChannel.cpp @@ -34,7 +34,6 @@ #include "nsProxyRelease.h" #include "nsNetUtil.h" #include "mozilla/Attributes.h" -#include "TimeStamp.h" #include "plbase64.h" #include "prmem.h" @@ -42,14 +41,8 @@ #include "prbit.h" #include "zlib.h" -// rather than slurp up all of nsIWebSocket.idl, which lives outside necko, just -// dupe one constant we need from it -#define CLOSE_GOING_AWAY 1001 - extern PRThread *gSocketThread; -using namespace mozilla; - namespace mozilla { namespace net { @@ -84,385 +77,6 @@ NS_IMPL_THREADSAFE_ISUPPORTS11(WebSocketChannel, // some helper classes -//----------------------------------------------------------------------------- -// FailDelayManager -// -// Stores entries (searchable by {host, port}) of connections that have recently -// failed, so we can do delay of reconnects per RFC 6455 Section 7.2.3 -//----------------------------------------------------------------------------- - - -// Initial reconnect delay is randomly chosen between 200-400 ms. -// This is a gentler backoff than the 0-5 seconds the spec offhandedly suggests. -const PRUint32 kWSReconnectInitialBaseDelay = 200; -const PRUint32 kWSReconnectInitialRandomDelay = 200; - -// Base lifetime (in ms) of a FailDelay: kept longer if more failures occur -const PRUint32 kWSReconnectBaseLifeTime = 60 * 1000; -// Maximum reconnect delay (in ms) -const PRUint32 kWSReconnectMaxDelay = 60 * 1000; - -// hold record of failed connections, and calculates needed delay for reconnects -// to same host/port. -class FailDelay -{ -public: - FailDelay(nsCString address, PRInt32 port) - : mAddress(address), mPort(port) - { - mLastFailure = TimeStamp::Now(); - mNextDelay = kWSReconnectInitialBaseDelay + - (rand() % kWSReconnectInitialRandomDelay); - } - - // Called to update settings when connection fails again. - void FailedAgain() - { - mLastFailure = TimeStamp::Now(); - // We use a truncated exponential backoff as suggested by RFC 6455, - // but multiply by 1.5 instead of 2 to be more gradual. - mNextDelay = PR_MIN(kWSReconnectMaxDelay, mNextDelay * 1.5); - LOG(("WebSocket: FailedAgain: host=%s, port=%d: incremented delay to %lu", - mAddress.get(), mPort, mNextDelay)); - } - - // returns 0 if there is no need to delay (i.e. delay interval is over) - PRUint32 RemainingDelay(TimeStamp rightNow) - { - TimeDuration dur = rightNow - mLastFailure; - PRUint32 sinceFail = (PRUint32) dur.ToMilliseconds(); - if (sinceFail > mNextDelay) - return 0; - - return mNextDelay - sinceFail; - } - - bool IsExpired(TimeStamp rightNow) - { - return (mLastFailure + - TimeDuration::FromMilliseconds(kWSReconnectBaseLifeTime + mNextDelay)) - <= rightNow; - } - - nsCString mAddress; // IP address (or hostname if using proxy) - PRInt32 mPort; - -private: - TimeStamp mLastFailure; // Time of last failed attempt - // mLastFailure + mNextDelay is the soonest we'll allow a reconnect - PRUint32 mNextDelay; // milliseconds -}; - -class FailDelayManager -{ -public: - FailDelayManager() - { - mDelaysDisabled = false; - - nsCOMPtr prefService = - do_GetService(NS_PREFSERVICE_CONTRACTID); - bool boolpref = true; - nsresult rv; - rv = prefService->GetBoolPref("network.websocket.delay-failed-reconnects", - &boolpref); - if (NS_SUCCEEDED(rv) && !boolpref) { - mDelaysDisabled = true; - } - } - - void Add(nsCString &address, PRInt32 port) - { - NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); - - if (mDelaysDisabled) - return; - - FailDelay *record = new FailDelay(address, port); - mEntries.AppendElement(record); - } - - // Element returned may not be valid after next main thread event: don't keep - // pointer to it around - FailDelay* Lookup(nsCString &address, PRInt32 port, - PRUint32 *outIndex = nsnull) - { - NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); - - if (mDelaysDisabled) - return nsnull; - - for (PRUint32 i = 0; i < mEntries.Length(); i++) { - FailDelay *fail = mEntries[i]; - if (fail->mAddress.Equals(address) && fail->mPort == port) { - if (outIndex) - *outIndex = i; - return fail; - } - } - return nsnull; - } - - // returns true if channel connects immediately, or false if it's delayed - bool DelayOrBegin(WebSocketChannel *ws) - { - if (!mDelaysDisabled) { - PRUint32 failIndex = 0; - FailDelay *fail = Lookup(ws->mAddress, ws->mPort, &failIndex); - - if (fail) { - TimeStamp rightNow = TimeStamp::Now(); - - PRUint32 remainingDelay = fail->RemainingDelay(rightNow); - if (remainingDelay) { - // reconnecting within delay interval: delay by remaining time - nsresult rv; - ws->mReconnectDelayTimer = - do_CreateInstance("@mozilla.org/timer;1", &rv); - if (NS_SUCCEEDED(rv)) { - rv = ws->mReconnectDelayTimer->InitWithCallback( - ws, remainingDelay, nsITimer::TYPE_ONE_SHOT); - if (NS_SUCCEEDED(rv)) { - LOG(("WebSocket: delaying websocket [this=%p] by %lu ms", - ws, (unsigned long)remainingDelay)); - ws->mConnecting = CONNECTING_DELAYED; - return false; - } - } - // if timer fails (which is very unlikely), drop down to BeginOpen call - } else if (fail->IsExpired(rightNow)) { - mEntries.RemoveElementAt(failIndex); - delete fail; - } - } - } - - // Delays disabled, or no previous failure, or we're reconnecting after scheduled - // delay interval has passed: connect. - // - ws->mConnecting = CONNECTING_IN_PROGRESS; - // If BeginOpen fails, it calls AbortSession, which calls OnStopSession, - // which will ensure any remaining queued connection(s) are scheduled - return ws->BeginOpen(); - } - - // Remove() also deletes all expired entries as it iterates: better for - // battery life than using a periodic timer. - void Remove(nsCString &address, PRInt32 port) - { - NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); - - TimeStamp rightNow = TimeStamp::Now(); - - // iterate from end, to make deletion indexing easier - for (PRInt32 i = mEntries.Length() - 1; i >= 0; --i) { - FailDelay *entry = mEntries[i]; - if ((entry->mAddress.Equals(address) && entry->mPort == port) || - entry->IsExpired(rightNow)) { - mEntries.RemoveElementAt(i); - delete entry; - } - } - } - -private: - nsTArray mEntries; - bool mDelaysDisabled; -}; - -//----------------------------------------------------------------------------- -// nsWSAdmissionManager -// -// 1) Ensures that only one websocket at a time is CONNECTING to a given IP -// address (or hostname, if using proxy), per RFC 6455 Section 4.1. -// 2) Delays reconnects to IP/host after connection failure, per Section 7.2.3 -//----------------------------------------------------------------------------- - -class nsWSAdmissionManager -{ -public: - nsWSAdmissionManager() : mSessionCount(0) - { - MOZ_COUNT_CTOR(nsWSAdmissionManager); - } - - class nsOpenConn - { - public: - nsOpenConn(nsCString &addr, WebSocketChannel *channel) - : mAddress(addr), mChannel(channel) { MOZ_COUNT_CTOR(nsOpenConn); } - ~nsOpenConn() { MOZ_COUNT_DTOR(nsOpenConn); } - - nsCString mAddress; - WebSocketChannel *mChannel; - }; - - ~nsWSAdmissionManager() - { - MOZ_COUNT_DTOR(nsWSAdmissionManager); - for (PRUint32 i = 0; i < mQueue.Length(); i++) - delete mQueue[i]; - } - - // Determine if we will open connection immediately (returns true), or - // delay/queue the connection (returns false) - bool ConditionallyConnect(WebSocketChannel *ws) - { - NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); - NS_ABORT_IF_FALSE(ws->mConnecting == NOT_CONNECTING, "opening state"); - - // If there is already another WS channel connecting to this IP address, - // defer BeginOpen and mark as waiting in queue. - bool found = (IndexOf(ws->mAddress) >= 0); - - // Always add ourselves to queue, even if we'll connect immediately - nsOpenConn *newdata = new nsOpenConn(ws->mAddress, ws); - mQueue.AppendElement(newdata); - - if (found) { - ws->mConnecting = CONNECTING_QUEUED; - return false; - } - - return mFailures.DelayOrBegin(ws); - } - - bool OnConnected(WebSocketChannel *aChannel) - { - NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); - NS_ABORT_IF_FALSE(aChannel->mConnecting == CONNECTING_IN_PROGRESS, - "Channel completed connect, but not connecting?"); - - aChannel->mConnecting = NOT_CONNECTING; - - // Remove from queue - RemoveFromQueue(aChannel); - - // Connection succeeded, so stop keeping track of any previous failures - mFailures.Remove(aChannel->mAddress, aChannel->mPort); - - // Check for queued connections to same host. - // Note: still need to check for failures, since next websocket with same - // host may have different port - return ConnectNext(aChannel->mAddress); - } - - // Called every time a websocket channel ends its session (including going away - // w/o ever successfully creating a connection) - bool OnStopSession(WebSocketChannel *aChannel, nsresult aReason) - { - NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); - - if (NS_FAILED(aReason)) { - // Have we seen this failure before? - FailDelay *knownFailure = mFailures.Lookup(aChannel->mAddress, - aChannel->mPort); - if (knownFailure) { - // repeated failure to connect: increase delay for next connection - knownFailure->FailedAgain(); - } else { - // new connection failure: record it. - LOG(("WebSocket: connection to %s, %d failed: [this=%p]", - aChannel->mAddress.get(), (int)aChannel->mPort, aChannel)); - mFailures.Add(aChannel->mAddress, aChannel->mPort); - } - } - - if (aChannel->mConnecting) { - // Only way a connecting channel may get here w/o failing is if it was - // closed with GOING_AWAY (1001) because of navigation, tab close, etc. - NS_ABORT_IF_FALSE(NS_FAILED(aReason) || - aChannel->mScriptCloseCode == CLOSE_GOING_AWAY, - "websocket closed while connecting w/o failing?"); - - RemoveFromQueue(aChannel); - - bool wasNotQueued = (aChannel->mConnecting != CONNECTING_QUEUED); - aChannel->mConnecting = NOT_CONNECTING; - if (wasNotQueued) - return ConnectNext(aChannel->mAddress); - } - return false; - } - - bool ConnectNext(nsCString &hostName) - { - PRInt32 index = IndexOf(hostName); - if (index >= 0) { - WebSocketChannel *chan = mQueue[index]->mChannel; - - NS_ABORT_IF_FALSE(chan->mConnecting == CONNECTING_QUEUED, - "transaction not queued but in queue"); - LOG(("WebSocket: ConnectNext: found channel [this=%p] in queue", chan)); - - return mFailures.DelayOrBegin(chan); - } - - return false; - } - - void IncrementSessionCount() - { - PR_ATOMIC_INCREMENT(&mSessionCount); - } - - void DecrementSessionCount() - { - PR_ATOMIC_DECREMENT(&mSessionCount); - } - - PRInt32 SessionCount() - { - return mSessionCount; - } - -private: - - void RemoveFromQueue(WebSocketChannel *aChannel) - { - PRInt32 index = IndexOf(aChannel); - NS_ABORT_IF_FALSE(index >= 0, "connection to remove not in queue"); - if (index >= 0) { - nsOpenConn *olddata = mQueue[index]; - mQueue.RemoveElementAt(index); - delete olddata; - } - } - - PRInt32 IndexOf(nsCString &aStr) - { - for (PRUint32 i = 0; i < mQueue.Length(); i++) - if (aStr == (mQueue[i])->mAddress) - return i; - return -1; - } - - PRInt32 IndexOf(WebSocketChannel *aChannel) - { - for (PRUint32 i = 0; i < mQueue.Length(); i++) - if (aChannel == (mQueue[i])->mChannel) - return i; - return -1; - } - - // SessionCount might be decremented from the main or the socket - // thread, so manage it with atomic counters - PRInt32 mSessionCount; - - // Queue for websockets that have not completed connecting yet. - // The first nsOpenConn with a given address will be either be - // CONNECTING_IN_PROGRESS or CONNECTING_DELAYED. Later ones with the same - // hostname must be CONNECTING_QUEUED. - // - // We could hash hostnames instead of using a single big vector here, but the - // dataset is expected to be small. - nsTArray mQueue; - - FailDelayManager mFailures; -}; - -static nsWSAdmissionManager *sWebSocketAdmissions = nsnull; - //----------------------------------------------------------------------------- // CallOnMessageAvailable //----------------------------------------------------------------------------- @@ -507,18 +121,14 @@ public: NS_DECL_ISUPPORTS CallOnStop(WebSocketChannel *aChannel, - nsresult aReason) + nsresult aData) : mChannel(aChannel), - mReason(aReason) {} + mData(aData) {} NS_IMETHOD Run() { NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); - - sWebSocketAdmissions->OnStopSession(mChannel, mReason); - - if (mChannel->mListener) - mChannel->mListener->OnStop(mChannel->mContext, mReason); + mChannel->mListener->OnStop(mChannel->mContext, mData); return NS_OK; } @@ -526,7 +136,7 @@ private: ~CallOnStop() {} nsRefPtr mChannel; - nsresult mReason; + nsresult mData; }; NS_IMPL_THREADSAFE_ISUPPORTS1(CallOnStop, nsIRunnable) @@ -760,6 +370,176 @@ private: }; NS_IMPL_THREADSAFE_ISUPPORTS1(OutboundEnqueuer, nsIRunnable) +//----------------------------------------------------------------------------- +// nsWSAdmissionManager +//----------------------------------------------------------------------------- + +// Section 4.1 requires that only a single websocket at a time can be CONNECTING +// to any given IP address (or hostname, if proxy doing DNS for us). This class +// ensures that we delay connecting until any pending connection for the same +// IP/addr is complete (i.e. until before the 101 upgrade complete response +// comes back and an 'open' javascript event is created) + +class nsWSAdmissionManager +{ +public: + nsWSAdmissionManager() : mSessionCount(0) + { + MOZ_COUNT_CTOR(nsWSAdmissionManager); + } + + class nsOpenConn + { + public: + nsOpenConn(nsCString &addr, WebSocketChannel *channel) + : mAddress(addr), mChannel(channel) { MOZ_COUNT_CTOR(nsOpenConn); } + ~nsOpenConn() { MOZ_COUNT_DTOR(nsOpenConn); } + + nsCString mAddress; + WebSocketChannel *mChannel; + }; + + ~nsWSAdmissionManager() + { + MOZ_COUNT_DTOR(nsWSAdmissionManager); + for (PRUint32 i = 0; i < mData.Length(); i++) + delete mData[i]; + } + + bool ConditionallyConnect(nsCString &aStr, WebSocketChannel *ws) + { + NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); + + // if aStr is not in mData then we return true, else false. + // in either case aStr is then added to mData - meaning + // there will be duplicates when this function has been + // called with the same parameter multiple times. + + // we could hash this, but the dataset is expected to be + // small + + // There may already be another WS channel connecting to this IP address, in + // which case we'll still create a new nsOpenConn but defer BeginOpen until + // that channel completes connecting. + bool found = (IndexOf(aStr) >= 0); + nsOpenConn *newdata = new nsOpenConn(aStr, ws); + mData.AppendElement(newdata); + + NS_ABORT_IF_FALSE (!ws->mOpenRunning && !ws->mOpenBlocked, + "opening state"); + + if (!found) { + ws->mOpenRunning = 1; + ws->BeginOpen(); + } else { + ws->mOpenBlocked = 1; + } + + return !found; + } + + bool Complete(WebSocketChannel *aChannel) + { + NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); + NS_ABORT_IF_FALSE(!aChannel->mOpenBlocked, + "blocked, but complete nsOpenConn"); + + // It is possible this has already been canceled + if (!aChannel->mOpenRunning) + return false; + + PRInt32 index = IndexOf(aChannel); + NS_ABORT_IF_FALSE(index >= 0, "completed connection not in open list"); + + aChannel->mOpenRunning = 0; + nsOpenConn *olddata = mData[index]; + mData.RemoveElementAt(index); + delete olddata; + + // are there more of the same address pending dispatch? + return ConnectNext(aChannel->mAddress); + } + + bool Cancel(WebSocketChannel *aChannel) + { + NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); + PRInt32 index = IndexOf(aChannel); + NS_ABORT_IF_FALSE(index >= 0, "Cancelled connection not in open list"); + NS_ABORT_IF_FALSE(aChannel->mOpenRunning ^ aChannel->mOpenBlocked, + "cancel without running xor blocked"); + + bool wasRunning = aChannel->mOpenRunning; + aChannel->mOpenRunning = 0; + aChannel->mOpenBlocked = 0; + nsOpenConn *olddata = mData[index]; + mData.RemoveElementAt(index); + delete olddata; + + // if we are running we can run another one + if (wasRunning) + return ConnectNext(aChannel->mAddress); + + return false; + } + + bool ConnectNext(nsCString &hostName) + { + PRInt32 index = IndexOf(hostName); + if (index >= 0) { + WebSocketChannel *chan = mData[index]->mChannel; + + NS_ABORT_IF_FALSE(chan->mOpenBlocked, + "transaction not blocked but in queue"); + NS_ABORT_IF_FALSE(!chan->mOpenRunning, "transaction already running"); + + chan->mOpenBlocked = 0; + chan->mOpenRunning = 1; + chan->BeginOpen(); + return true; + } + + return false; + } + + void IncrementSessionCount() + { + PR_ATOMIC_INCREMENT(&mSessionCount); + } + + void DecrementSessionCount() + { + PR_ATOMIC_DECREMENT(&mSessionCount); + } + + PRInt32 SessionCount() + { + return mSessionCount; + } + +private: + nsTArray mData; + + PRInt32 IndexOf(nsCString &aStr) + { + for (PRUint32 i = 0; i < mData.Length(); i++) + if (aStr == (mData[i])->mAddress) + return i; + return -1; + } + + PRInt32 IndexOf(WebSocketChannel *aChannel) + { + for (PRUint32 i = 0; i < mData.Length(); i++) + if (aChannel == (mData[i])->mChannel) + return i; + return -1; + } + + // SessionCount might be decremented from the main or the socket + // thread, so manage it with atomic counters + PRInt32 mSessionCount; +}; + //----------------------------------------------------------------------------- // nsWSCompression // @@ -887,15 +667,15 @@ private: PRUint8 mBuffer[kBufferLen]; }; +static nsWSAdmissionManager *sWebSocketAdmissions = nsnull; + //----------------------------------------------------------------------------- // WebSocketChannel //----------------------------------------------------------------------------- WebSocketChannel::WebSocketChannel() : - mPort(0), mCloseTimeout(20000), mOpenTimeout(20000), - mConnecting(NOT_CONNECTING), mPingTimeout(0), mPingResponseTimeout(10000), mMaxConcurrentConnections(200), @@ -911,6 +691,8 @@ WebSocketChannel::WebSocketChannel() : mAutoFollowRedirects(0), mReleaseOnTransmit(0), mTCPClosed(0), + mOpenBlocked(0), + mOpenRunning(0), mChannelWasOpened(0), mDataStarted(0), mIncrementedSessionCount(0), @@ -945,7 +727,7 @@ WebSocketChannel::~WebSocketChannel() // this stop is a nop if the normal connect/close is followed StopSession(NS_ERROR_UNEXPECTED); - NS_ABORT_IF_FALSE(mConnecting == NOT_CONNECTING, "op"); + NS_ABORT_IF_FALSE(!mOpenRunning && !mOpenBlocked, "op"); moz_free(mBuffer); moz_free(mDynamicOutput); @@ -999,7 +781,7 @@ WebSocketChannel::Shutdown() sWebSocketAdmissions = nsnull; } -bool +nsresult WebSocketChannel::BeginOpen() { LOG(("WebSocketChannel::BeginOpen() %p\n", this)); @@ -1011,40 +793,29 @@ WebSocketChannel::BeginOpen() LOG(("WebSocketChannel::BeginOpen: Resuming Redirect\n")); rv = mRedirectCallback->OnRedirectVerifyCallback(NS_OK); mRedirectCallback = nsnull; - return false; + return rv; } nsCOMPtr localChannel = do_QueryInterface(mChannel, &rv); if (NS_FAILED(rv)) { LOG(("WebSocketChannel::BeginOpen: cannot async open\n")); AbortSession(NS_ERROR_UNEXPECTED); - return false; + return rv; } rv = localChannel->AsyncOpen(this, mHttpChannel); if (NS_FAILED(rv)) { LOG(("WebSocketChannel::BeginOpen: cannot async open\n")); AbortSession(NS_ERROR_CONNECTION_REFUSED); - return false; + return rv; } mChannelWasOpened = 1; mOpenTimer = do_CreateInstance("@mozilla.org/timer;1", &rv); - if (NS_FAILED(rv)) { - LOG(("WebSocketChannel::BeginOpen: cannot create open timer\n")); - AbortSession(NS_ERROR_UNEXPECTED); - return false; - } + if (NS_SUCCEEDED(rv)) + mOpenTimer->InitWithCallback(this, mOpenTimeout, nsITimer::TYPE_ONE_SHOT); - rv = mOpenTimer->InitWithCallback(this, mOpenTimeout, - nsITimer::TYPE_ONE_SHOT); - if (NS_FAILED(rv)) { - LOG(("WebSocketChannel::BeginOpen: cannot initialize open timer\n")); - AbortSession(NS_ERROR_UNEXPECTED); - return false; - } - - return true; + return rv; } bool @@ -1807,6 +1578,9 @@ WebSocketChannel::StopSession(nsresult reason) mCallbacks = nsnull; } + if (mOpenRunning || mOpenBlocked) + sWebSocketAdmissions->Cancel(this); + if (mCloseTimer) { mCloseTimer->Cancel(); mCloseTimer = nsnull; @@ -1817,11 +1591,6 @@ WebSocketChannel::StopSession(nsresult reason) mOpenTimer = nsnull; } - if (mReconnectDelayTimer) { - mReconnectDelayTimer->Cancel(); - mReconnectDelayTimer = nsnull; - } - if (mPingTimer) { mPingTimer->Cancel(); mPingTimer = nsnull; @@ -1888,7 +1657,8 @@ WebSocketChannel::StopSession(nsresult reason) if (!mCalledOnStop) { mCalledOnStop = 1; - NS_DispatchToMainThread(new CallOnStop(this, reason)); + if (mListener) + NS_DispatchToMainThread(new CallOnStop(this, reason)); } return; @@ -2123,10 +1893,6 @@ WebSocketChannel::ApplyForAdmission() rv = mURI->GetHost(hostName); NS_ENSURE_SUCCESS(rv, rv); mAddress = hostName; - rv = mURI->GetPort(&mPort); - NS_ENSURE_SUCCESS(rv, rv); - if (mPort == -1) - mPort = (mEncrypted ? kDefaultWSSPort : kDefaultWSPort); // expect the callback in ::OnLookupComplete LOG(("WebSocketChannel::ApplyForAdmission: checking for concurrent open\n")); @@ -2147,12 +1913,7 @@ WebSocketChannel::StartWebsocketData() LOG(("WebSocketChannel::StartWebsocketData() %p", this)); NS_ABORT_IF_FALSE(!mDataStarted, "StartWebsocketData twice"); mDataStarted = 1; - - // We're now done CONNECTING, which means we can now open another, - // perhaps parallel, connection to the same host if one - // is pending - sWebSocketAdmissions->OnConnected(this); - + LOG(("WebSocketChannel::StartWebsocketData Notifying Listener %p\n", mListener.get())); @@ -2192,7 +1953,7 @@ WebSocketChannel::OnLookupComplete(nsICancelable *aRequest, LOG(("WebSocketChannel::OnLookupComplete: Failed GetNextAddr\n")); } - if (sWebSocketAdmissions->ConditionallyConnect(this)) { + if (sWebSocketAdmissions->ConditionallyConnect(mAddress, this)) { LOG(("WebSocketChannel::OnLookupComplete: Proceeding with Open\n")); } else { LOG(("WebSocketChannel::OnLookupComplete: Deferring Open\n")); @@ -2316,19 +2077,17 @@ WebSocketChannel::AsyncOnChannelRedirect( return rv; } - // Redirected-to URI may need to be delayed by 1-connecting-per-host and - // delay-after-fail algorithms. So hold off calling OnRedirectVerifyCallback - // until BeginOpen, when we know it's OK to proceed with new channel. + // We cannot just tell the callback OK right now due to the 1 connect at a + // time policy. First we need to complete the old location and then start the + // lookup chain for the new location - once that is complete and we have been + // admitted, OnRedirectVerifyCallback(NS_OK) will be called out of BeginOpen() + + sWebSocketAdmissions->Complete(this); + mAddress.Truncate(); mRedirectCallback = callback; - // Mark old channel as successfully connected so we'll clear any FailDelay - // associated with the old URI. Note: no need to also call OnStopSession: - // it's a no-op for successful, already-connected channels. - sWebSocketAdmissions->OnConnected(this); - - // ApplyForAdmission as if we were starting from fresh... - mAddress.Truncate(); mChannelWasOpened = 0; + rv = ApplyForAdmission(); if (NS_FAILED(rv)) { LOG(("WebSocketChannel: Redirect failed due to DNS failure\n")); @@ -2368,16 +2127,6 @@ WebSocketChannel::Notify(nsITimer *timer) return NS_OK; AbortSession(NS_ERROR_NET_TIMEOUT); - } else if (timer == mReconnectDelayTimer) { - NS_ABORT_IF_FALSE(mConnecting == CONNECTING_DELAYED, - "woke up from delay w/o being delayed?"); - - mReconnectDelayTimer = nsnull; - LOG(("WebSocketChannel: connecting [this=%p] after reconnect delay", this)); - // - if BeginOpen fails, it calls AbortSession, which calls OnStopSession, - // which will ensure any remaining queued connection(s) are scheduled - mConnecting = CONNECTING_IN_PROGRESS; - this->BeginOpen(); } else if (timer == mPingTimer) { NS_ABORT_IF_FALSE(PR_GetCurrentThread() == gSocketThread, "not socket thread"); @@ -2602,6 +2351,12 @@ WebSocketChannel::Close(PRUint16 code, const nsACString & reason) return NS_OK; } + if (!mTransport) { + LOG(("WebSocketChannel::Close() without transport - aborting.")); + AbortSession(NS_ERROR_NOT_CONNECTED); + return NS_ERROR_NOT_CONNECTED; + } + // The API requires the UTF-8 string to be 123 or less bytes if (reason.Length() > 123) return NS_ERROR_ILLEGAL_VALUE; @@ -2610,20 +2365,6 @@ WebSocketChannel::Close(PRUint16 code, const nsACString & reason) mScriptCloseReason = reason; mScriptCloseCode = code; - if (!mTransport) { - nsresult rv; - if (code == CLOSE_GOING_AWAY) { - // Not an error: for example, tab has closed or navigated away - LOG(("WebSocketChannel::Close() GOING_AWAY without transport.")); - rv = NS_OK; - } else { - LOG(("WebSocketChannel::Close() without transport - error.")); - rv = NS_ERROR_NOT_CONNECTED; - } - StopSession(rv); - return rv; - } - return mSocketThread->Dispatch( new OutboundEnqueuer(this, new OutboundMessage(kMsgTypeFin, nsnull)), nsIEventTarget::DISPATCH_NORMAL); @@ -2729,6 +2470,16 @@ WebSocketChannel::OnStartRequest(nsIRequest *aRequest, NS_ABORT_IF_FALSE(NS_IsMainThread(), "not main thread"); NS_ABORT_IF_FALSE(!mRecvdHttpOnStartRequest, "OTA duplicated"); + // Generating the onStart event will take us out of the + // CONNECTING state which means we can now open another, + // perhaps parallel, connection to the same host if one + // is pending + + if (sWebSocketAdmissions->Complete(this)) + LOG(("WebSocketChannel::OnStartRequest: Starting Pending Open\n")); + else + LOG(("WebSocketChannel::OnStartRequest: No More Pending Opens\n")); + if (mOpenTimer) { mOpenTimer->Cancel(); mOpenTimer = nsnull; diff --git a/netwerk/protocol/websocket/WebSocketChannel.h b/netwerk/protocol/websocket/WebSocketChannel.h index f18fe0850f75..3fcb34267e65 100644 --- a/netwerk/protocol/websocket/WebSocketChannel.h +++ b/netwerk/protocol/websocket/WebSocketChannel.h @@ -42,14 +42,6 @@ class CallOnStop; class CallOnServerClose; class CallAcknowledge; -// Used to enforce "1 connecting websocket per host" rule, and reconnect delays -enum wsConnectingState { - NOT_CONNECTING = 0, // Not yet (or no longer) trying to open connection - CONNECTING_QUEUED, // Waiting for other ws to same host to finish opening - CONNECTING_DELAYED, // Delayed by "reconnect after failure" algorithm - CONNECTING_IN_PROGRESS // Started connection: waiting for result -}; - class WebSocketChannel : public BaseWebSocketChannel, public nsIHttpUpgradeListener, public nsIStreamListener, @@ -109,7 +101,6 @@ protected: private: friend class OutboundEnqueuer; friend class nsWSAdmissionManager; - friend class FailDelayManager; friend class CallOnMessageAvailable; friend class CallOnStop; friend class CallOnServerClose; @@ -126,7 +117,7 @@ private: void GeneratePong(PRUint8 *payload, PRUint32 len); void GeneratePing(); - bool BeginOpen(); + nsresult BeginOpen(); nsresult HandleExtensions(); nsresult SetupRequest(); nsresult ApplyForAdmission(); @@ -158,11 +149,7 @@ private: nsCOMPtr mRandomGenerator; nsCString mHashedSecret; - - // Used as key for connection managment: Initially set to hostname from URI, - // then to IP address (unless we're leaving DNS resolution to a proxy server) nsCString mAddress; - PRInt32 mPort; // WS server port nsCOMPtr mTransport; nsCOMPtr mSocketIn; @@ -173,8 +160,6 @@ private: nsCOMPtr mOpenTimer; PRUint32 mOpenTimeout; /* milliseconds */ - wsConnectingState mConnecting; /* 0 if not connecting */ - nsCOMPtr mReconnectDelayTimer; nsCOMPtr mPingTimer; PRUint32 mPingTimeout; /* milliseconds */ @@ -198,6 +183,8 @@ private: PRUint32 mAutoFollowRedirects : 1; PRUint32 mReleaseOnTransmit : 1; PRUint32 mTCPClosed : 1; + PRUint32 mOpenBlocked : 1; + PRUint32 mOpenRunning : 1; PRUint32 mChannelWasOpened : 1; PRUint32 mDataStarted : 1; PRUint32 mIncrementedSessionCount : 1;