From f253d2ed23e67c005ee2c5345e955c5b670def48 Mon Sep 17 00:00:00 2001 From: Gijs Kruitbosch Date: Fri, 15 May 2015 14:25:57 +0100 Subject: [PATCH 01/33] Bug 1164942 - do not load pocket's jsm or other scripts until used, r=jaws --- browser/base/content/browser.js | 24 ++++++++++++++++++- browser/base/content/browser.xul | 3 --- .../customizableui/CustomizableWidgets.jsm | 9 +++++-- .../customizableui/content/panelUI.js | 2 -- browser/components/pocket/Pocket.jsm | 24 ------------------- 5 files changed, 30 insertions(+), 32 deletions(-) diff --git a/browser/base/content/browser.js b/browser/base/content/browser.js index b63f3e3b0033..18f7b92463ee 100644 --- a/browser/base/content/browser.js +++ b/browser/base/content/browser.js @@ -55,6 +55,29 @@ XPCOMUtils.defineLazyModuleGetter(this, "LightweightThemeManager", XPCOMUtils.defineLazyModuleGetter(this, "Pocket", "resource:///modules/Pocket.jsm"); +// Can't use XPCOMUtils for these because the scripts try to define the variables +// on window, and so the defineProperty inside defineLazyGetter fails. +Object.defineProperty(window, "pktApi", { + get: function() { + // Avoid this getter running again: + delete window.pktApi; + Services.scriptloader.loadSubScript("chrome://browser/content/pocket/pktApi.js", window); + return window.pktApi; + }, + configurable: true, + enumerable: true +}); +Object.defineProperty(window, "pktUI", { + get: function() { + // Avoid this getter running again: + delete window.pktUI; + Services.scriptloader.loadSubScript("chrome://browser/content/pocket/main.js", window); + return window.pktUI; + }, + configurable: true, + enumerable: true +}); + const nsIWebNavigation = Ci.nsIWebNavigation; var gLastBrowserCharset = null; @@ -4171,7 +4194,6 @@ var XULBrowserWindow = { BookmarkingUI.onLocationChange(); SocialUI.updateState(location); UITour.onLocationChange(location); - Pocket.onLocationChange(browser, aLocationURI); } // Utility functions for disabling find diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul index e62e99ca8c5e..981c896d5b82 100644 --- a/browser/base/content/browser.xul +++ b/browser/base/content/browser.xul @@ -1307,7 +1307,4 @@ # starting with an empty iframe here in browser.xul from a Ts standpoint. - - -browser_compartments.html + +Main frame. + + + + diff --git a/toolkit/components/aboutperformance/tests/browser/browser_compartments_frame.html b/toolkit/components/aboutperformance/tests/browser/browser_compartments_frame.html new file mode 100644 index 000000000000..69edfe871bab --- /dev/null +++ b/toolkit/components/aboutperformance/tests/browser/browser_compartments_frame.html @@ -0,0 +1,12 @@ + + + + + Subframe for test browser_compartments.html (do not change this title) + + + + +Subframe loaded. + + diff --git a/toolkit/components/aboutperformance/tests/browser/browser_compartments_script.js b/toolkit/components/aboutperformance/tests/browser/browser_compartments_script.js new file mode 100644 index 000000000000..d588874b3044 --- /dev/null +++ b/toolkit/components/aboutperformance/tests/browser/browser_compartments_script.js @@ -0,0 +1,20 @@ + +window.addEventListener("message", e => { + console.log("frame content", "message", e); + if ("title" in e.data) { + document.title = e.data.title; + } +}); + +// Use some CPU. +window.setInterval(() => { + // Compute an arbitrary value, print it out to make sure that the JS + // engine doesn't discard all our computation. + var date = Date.now(); + var array = []; + var i = 0; + while (Date.now() - date <= 100) { + array[i%2] = i++; + } + console.log("Arbitrary value", window.location, array); +}, 300); diff --git a/toolkit/components/perfmonitoring/PerformanceStats.jsm b/toolkit/components/perfmonitoring/PerformanceStats.jsm index 47bbb4c0617f..bf3e30229d31 100644 --- a/toolkit/components/perfmonitoring/PerformanceStats.jsm +++ b/toolkit/components/perfmonitoring/PerformanceStats.jsm @@ -25,7 +25,8 @@ let performanceStatsService = const PROPERTIES_NUMBERED = ["totalUserTime", "totalSystemTime", "totalCPOWTime", "ticks"]; -const PROPERTIES_META = ["name", "addonId", "isSystem"]; +const PROPERTIES_META_IMMUTABLE = ["name", "addonId", "isSystem"]; +const PROPERTIES_META = [...PROPERTIES_META_IMMUTABLE, "windowId", "title"]; const PROPERTIES_FLAT = [...PROPERTIES_NUMBERED, ...PROPERTIES_META]; /** @@ -41,6 +42,15 @@ const PROPERTIES_FLAT = [...PROPERTIES_NUMBERED, ...PROPERTIES_META]; * * @field {string} addonId The identifier of the addon (e.g. "myaddon@foo.bar"). * + * @field {string|null} title The title of the webpage to which this code + * belongs. Note that this is the title of the entire webpage (i.e. the tab), + * even if the code is executed in an iframe. Also note that this title may + * change over time. + * + * @field {number} windowId The outer window ID of the top-level nsIDOMWindow + * to which this code belongs. May be 0 if the code doesn't belong to any + * nsIDOMWindow. + * * @field {boolean} isSystem `true` if the component is a system component (i.e. * an add-on or platform-code), `false` otherwise (i.e. a webpage). * diff --git a/toolkit/components/perfmonitoring/moz.build b/toolkit/components/perfmonitoring/moz.build index 6d53445e3945..b1ba6e8094e9 100644 --- a/toolkit/components/perfmonitoring/moz.build +++ b/toolkit/components/perfmonitoring/moz.build @@ -28,4 +28,9 @@ EXPORTS += [ 'nsPerformanceStats.h' ] +# We need nsGlobalWindow +LOCAL_INCLUDES += [ + '/dom/base', +] + FINAL_LIBRARY = 'xul' diff --git a/toolkit/components/perfmonitoring/nsIPerformanceStats.idl b/toolkit/components/perfmonitoring/nsIPerformanceStats.idl index 72f105ea63f3..37cc5e58b37d 100644 --- a/toolkit/components/perfmonitoring/nsIPerformanceStats.idl +++ b/toolkit/components/perfmonitoring/nsIPerformanceStats.idl @@ -6,6 +6,7 @@ #include "nsISupports.idl" #include "nsIArray.idl" +#include "nsIDOMWindow.idl" /** * Mechanisms for querying the current process about performance @@ -21,7 +22,7 @@ * All values are monotonic and are updated only when * `nsIPerformanceStatsService.isStopwatchActive` is `true`. */ -[scriptable, uuid(f015cbad-e16f-4982-a080-67e4e69a5b2e)] +[scriptable, uuid(c2e2d494-06b9-40c5-91f7-505086dc4ecd)] interface nsIPerformanceStats: nsISupports { /** * The name of the component: @@ -38,6 +39,18 @@ interface nsIPerformanceStats: nsISupports { */ readonly attribute AString addonId; + /** + * If the component is code executed in a window, the ID of the topmost + * outer window (i.e. the tab), otherwise 0. + */ + readonly attribute uint64_t windowId; + + /** + * If the component is code executed in a window, the title of the topmost + * window (i.e. the tab), otherwise an empty string. + */ + readonly attribute AString title; + /** * Total amount of time spent executing code in this group, in * microseconds. @@ -91,7 +104,7 @@ interface nsIPerformanceSnapshot: nsISupports { nsIPerformanceStats getProcessData(); }; -[scriptable, builtinclass, uuid(5795113a-39a1-4087-ba09-98b7d07d025a)] +[scriptable, builtinclass, uuid(3e4bff64-555f-41ec-9e7f-9338c77fd696)] interface nsIPerformanceStatsService : nsISupports { /** * `true` if we should monitor performance, `false` otherwise. @@ -101,7 +114,7 @@ interface nsIPerformanceStatsService : nsISupports { /** * Capture a snapshot of the performance data. */ - nsIPerformanceSnapshot getSnapshot(); + [implicit_jscontext] nsIPerformanceSnapshot getSnapshot(); }; %{C++ diff --git a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp index 544fe01210fe..bfdfa22edfbf 100644 --- a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp +++ b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp @@ -3,24 +3,37 @@ * 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/. */ -#include "jsapi.h" #include "nsPerformanceStats.h" + #include "nsMemory.h" #include "nsLiteralString.h" #include "nsCRTGlue.h" -#include "nsIJSRuntimeService.h" #include "nsServiceManagerUtils.h" + #include "nsCOMArray.h" #include "nsIMutableArray.h" + +#include "jsapi.h" #include "nsJSUtils.h" #include "xpcpublic.h" #include "jspubtd.h" +#include "nsIJSRuntimeService.h" + +#include "nsIDOMWindow.h" +#include "nsGlobalWindow.h" class nsPerformanceStats: public nsIPerformanceStats { public: - nsPerformanceStats(nsAString& aName, nsAString& aAddonId, bool aIsSystem, js::PerformanceData& aPerformanceData) + nsPerformanceStats(const nsAString& aName, + const nsAString& aAddonId, + const nsAString& aTitle, + const uint64_t aWindowId, + const bool aIsSystem, + const js::PerformanceData& aPerformanceData) : mName(aName) , mAddonId(aAddonId) + , mTitle(aTitle) + , mWindowId(aWindowId) , mIsSystem(aIsSystem) , mPerformanceData(aPerformanceData) { @@ -41,6 +54,24 @@ public: return NS_OK; }; + /* readonly attribute uint64_t windowId; */ + NS_IMETHOD GetWindowId(uint64_t *aWindowId) override { + *aWindowId = mWindowId; + return NS_OK; + } + + /* readonly attribute AString title; */ + NS_IMETHOD GetTitle(nsAString & aTitle) override { + aTitle.Assign(mTitle); + return NS_OK; + } + + /* readonly attribute bool isSystem; */ + NS_IMETHOD GetIsSystem(bool *_retval) override { + *_retval = mIsSystem; + return NS_OK; + } + /* readonly attribute unsigned long long totalUserTime; */ NS_IMETHOD GetTotalUserTime(uint64_t *aTotalUserTime) override { *aTotalUserTime = mPerformanceData.totalUserTime; @@ -78,16 +109,13 @@ public: return NS_OK; }; - /* readonly attribute bool isSystem; */ - NS_IMETHOD GetIsSystem(bool *_retval) override { - *_retval = mIsSystem; - return NS_OK; - } - private: nsString mName; nsString mAddonId; + nsString mTitle; + uint64_t mWindowId; bool mIsSystem; + js::PerformanceData mPerformanceData; virtual ~nsPerformanceStats() {} @@ -103,15 +131,45 @@ public: NS_DECL_NSIPERFORMANCESNAPSHOT nsPerformanceSnapshot(); - nsresult Init(); + nsresult Init(JSContext*); private: virtual ~nsPerformanceSnapshot(); /** - * Import a `PerformanceStats` as a `nsIPerformanceStats`. + * Import a `js::PerformanceStats` as a `nsIPerformanceStats`. + * + * Precondition: this method assumes that we have entered the JSCompartment for which data `c` + * has been collected. + * + * `cx` may be `nullptr` if we are importing the statistics for the + * entire process, rather than the statistics for a specific set of + * compartments. */ - already_AddRefed ImportStats(js::PerformanceStats* c); + already_AddRefed ImportStats(JSContext* cx, const js::PerformanceData& data); + /** + * Callbacks for iterating through the `PerformanceStats` of a runtime. + */ + bool IterPerformanceStatsCallbackInternal(JSContext* cx, const js::PerformanceData& stats); + static bool IterPerformanceStatsCallback(JSContext* cx, const js::PerformanceData& stats, void* self); + + // If the context represents a window, extract the title and window ID. + // Otherwise, extract "" and 0. + static void GetWindowData(JSContext*, + nsString& title, + uint64_t* windowId); + + // If the context presents an add-on, extract the addon ID. + // Otherwise, extract "". + static void GetAddonId(JSContext*, + JS::Handle global, + nsAString& addonId); + + // Determine whether a context is part of the system principals. + static bool GetIsSystem(JSContext*, + JS::Handle global); + +private: nsCOMArray mComponentsData; nsCOMPtr mProcessData; }; @@ -126,36 +184,118 @@ nsPerformanceSnapshot::~nsPerformanceSnapshot() { } -already_AddRefed -nsPerformanceSnapshot::ImportStats(js::PerformanceStats* c) { - nsString addonId; - if (c->addonId) { - AssignJSFlatString(addonId, (JSFlatString*)c->addonId); +/* static */ void +nsPerformanceSnapshot::GetWindowData(JSContext* cx, + nsString& title, + uint64_t* windowId) +{ + MOZ_ASSERT(windowId); + + title.SetIsVoid(true); + *windowId = 0; + + nsCOMPtr win = xpc::CurrentWindowOrNull(cx); + if (!win) { + return; } - nsCString cname(c->name); - NS_ConvertUTF8toUTF16 name(cname); - nsCOMPtr result = new nsPerformanceStats(name, addonId, c->isSystem, c->performance); + + nsCOMPtr top; + nsresult rv = win->GetTop(getter_AddRefs(top)); + if (!top || NS_FAILED(rv)) { + return; + } + + nsCOMPtr privateTop = do_QueryInterface(top); + if (!privateTop) { + return; + } + + nsCOMPtr doc = privateTop->GetExtantDoc(); + if (!doc) { + return; + } + doc->GetTitle(title); + *windowId = privateTop->WindowID(); +} + +/* static */ void +nsPerformanceSnapshot::GetAddonId(JSContext*, + JS::Handle global, + nsAString& addonId) +{ + addonId.AssignLiteral(""); + + JSAddonId* jsid = AddonIdOfObject(global); + if (!jsid) { + return; + } + AssignJSFlatString(addonId, (JSFlatString*)jsid); +} + +/* static */ bool +nsPerformanceSnapshot::GetIsSystem(JSContext*, + JS::Handle global) +{ + return nsContentUtils::IsSystemPrincipal(nsContentUtils::ObjectPrincipal(global)); +} + +already_AddRefed +nsPerformanceSnapshot::ImportStats(JSContext* cx, const js::PerformanceData& performance) { + JS::RootedObject global(cx, JS::CurrentGlobalOrNull(cx)); + + if (!global) { + // While it is possible for a compartment to have no global + // (e.g. atoms), this compartment is not very interesting for us. + return nullptr; + } + + nsString addonId; + GetAddonId(cx, global, addonId); + + nsString title; + uint64_t windowId; + GetWindowData(cx, title, &windowId); + + nsAutoString name; + nsAutoCString cname; + xpc::GetCurrentCompartmentName(cx, cname); + name.Assign(NS_ConvertUTF8toUTF16(cname)); + + bool isSystem = GetIsSystem(cx, global); + + nsCOMPtr result = + new nsPerformanceStats(name, addonId, title, windowId, isSystem, performance); return result.forget(); } -nsresult -nsPerformanceSnapshot::Init() { - JSRuntime* rt; - nsCOMPtr svc(do_GetService("@mozilla.org/js/xpc/RuntimeService;1")); - NS_ENSURE_TRUE(svc, NS_ERROR_FAILURE); - svc->GetRuntime(&rt); - js::PerformanceStats processStats; - js::PerformanceStatsVector componentsStats; - if (!js::GetPerformanceStats(rt, componentsStats, processStats)) { - return NS_ERROR_OUT_OF_MEMORY; +/*static*/ bool +nsPerformanceSnapshot::IterPerformanceStatsCallback(JSContext* cx, const js::PerformanceData& stats, void* self) { + return reinterpret_cast(self)->IterPerformanceStatsCallbackInternal(cx, stats); +} + +bool +nsPerformanceSnapshot::IterPerformanceStatsCallbackInternal(JSContext* cx, const js::PerformanceData& stats) { + nsCOMPtr result = ImportStats(cx, stats); + if (result) { + mComponentsData.AppendElement(result); } - size_t num = componentsStats.length(); - for (size_t pos = 0; pos < num; pos++) { - nsCOMPtr stats = ImportStats(&componentsStats[pos]); - mComponentsData.AppendObject(stats); + return true; +} + +nsresult +nsPerformanceSnapshot::Init(JSContext* cx) { + js::PerformanceData processStats; + if (!js::IterPerformanceStats(cx, nsPerformanceSnapshot::IterPerformanceStatsCallback, &processStats, this)) { + return NS_ERROR_UNEXPECTED; } - mProcessData = ImportStats(&processStats); + + mProcessData = new nsPerformanceStats(NS_LITERAL_STRING(""), // name + NS_LITERAL_STRING(""), // add-on id + NS_LITERAL_STRING(""), // title + 0, // window id + true, // isSystem + processStats); return NS_OK; } @@ -209,10 +349,10 @@ NS_IMETHODIMP nsPerformanceStatsService::SetIsStopwatchActive(JSContext* cx, boo } /* readonly attribute nsIPerformanceSnapshot snapshot; */ -NS_IMETHODIMP nsPerformanceStatsService::GetSnapshot(nsIPerformanceSnapshot * *aSnapshot) +NS_IMETHODIMP nsPerformanceStatsService::GetSnapshot(JSContext* cx, nsIPerformanceSnapshot * *aSnapshot) { nsRefPtr snapshot = new nsPerformanceSnapshot(); - nsresult rv = snapshot->Init(); + nsresult rv = snapshot->Init(cx); if (NS_FAILED(rv)) { return rv; } diff --git a/toolkit/components/perfmonitoring/tests/browser/browser.ini b/toolkit/components/perfmonitoring/tests/browser/browser.ini index fcf0f90c0a4d..4975f2c88d53 100644 --- a/toolkit/components/perfmonitoring/tests/browser/browser.ini +++ b/toolkit/components/perfmonitoring/tests/browser/browser.ini @@ -3,6 +3,8 @@ head = head.js support-files = browser_Addons_sample.xpi browser_compartments.html + browser_compartments_frame.html + browser_compartments_script.js [browser_AddonWatcher.js] [browser_compartments.js] diff --git a/toolkit/components/perfmonitoring/tests/browser/browser_compartments.html b/toolkit/components/perfmonitoring/tests/browser/browser_compartments.html index 120aeff789ca..d7ee6c418910 100644 --- a/toolkit/components/perfmonitoring/tests/browser/browser_compartments.html +++ b/toolkit/components/perfmonitoring/tests/browser/browser_compartments.html @@ -2,24 +2,19 @@ - browser_compartments.html + Main frame for test browser_compartments.js - -browser_compartments.html +Main frame. + + + + + diff --git a/toolkit/components/perfmonitoring/tests/browser/browser_compartments.js b/toolkit/components/perfmonitoring/tests/browser/browser_compartments.js index dfb02d7e4de1..08ce72f58ab4 100644 --- a/toolkit/components/perfmonitoring/tests/browser/browser_compartments.js +++ b/toolkit/components/perfmonitoring/tests/browser/browser_compartments.js @@ -3,10 +3,17 @@ "use strict"; +/** + * Test that we see jank that takes place in a webpage, + * and that jank from several iframes are actually charged + * to the top window. + */ Cu.import("resource://gre/modules/PerformanceStats.jsm", this); Cu.import("resource://testing-common/ContentTask.jsm", this); const URL = "http://example.com/browser/toolkit/components/perfmonitoring/tests/browser/browser_compartments.html?test=" + Math.random(); +const PARENT_TITLE = `Main frame for test browser_compartments.js ${Math.random()}`; +const FRAME_TITLE = `Subframe for test browser_compartments.js ${Math.random()}`; // This function is injected as source as a frameScript function frameScript() { @@ -30,6 +37,21 @@ function frameScript() { Cu.reportError(ex.stack); } }); + + addMessageListener("compartments-test:setTitles", titles => { + try { + console.log("content", "Setting titles", Object.keys(titles)); + content.document.title = titles.data.parent; + for (let i = 0; i < content.frames.length; ++i) { + content.frames[i].postMessage({title: titles.data.frames}, "*"); + } + console.log("content", "Done setting titles", content.document.title); + sendAsyncMessage("compartments-test:setTitles"); + } catch (ex) { + Cu.reportError("Error in content: " + ex); + Cu.reportError(ex.stack); + } + }); } // A variant of `Assert` that doesn't spam the logs @@ -72,28 +94,29 @@ function monotinicity_tester(source, testName) { }; let sanityCheck = function(prev, next) { + let name = `${testName}: ${next.name}`; info(`Sanity check: ${JSON.stringify(next, null, "\t")}`); if (prev == null) { return; } for (let k of ["name", "addonId", "isSystem"]) { - SilentAssert.equal(prev[k], next[k], `Sanity check (${testName}): ${k} hasn't changed.`); + SilentAssert.equal(prev[k], next[k], `Sanity check (${name}): ${k} hasn't changed.`); } for (let k of ["totalUserTime", "totalSystemTime", "totalCPOWTime", "ticks"]) { - SilentAssert.equal(typeof next[k], "number", `Sanity check (${testName}): ${k} is a number.`); - SilentAssert.leq(prev[k], next[k], `Sanity check (${testName}): ${k} is monotonic.`); - SilentAssert.leq(0, next[k], `Sanity check (${testName}): ${k} is >= 0.`) + SilentAssert.equal(typeof next[k], "number", `Sanity check (${name}): ${k} is a number.`); + SilentAssert.leq(prev[k], next[k], `Sanity check (${name}): ${k} is monotonic.`); + SilentAssert.leq(0, next[k], `Sanity check (${name}): ${k} is >= 0.`) } SilentAssert.equal(prev.durations.length, next.durations.length); for (let i = 0; i < next.durations.length; ++i) { SilentAssert.ok(typeof next.durations[i] == "number" && next.durations[i] >= 0, - `Sanity check (${testName}): durations[${i}] is a non-negative number.`); + `Sanity check (${name}): durations[${i}] is a non-negative number.`); SilentAssert.leq(prev.durations[i], next.durations[i], - `Sanity check (${testName}): durations[${i}] is monotonic.`) + `Sanity check (${name}): durations[${i}] is monotonic.`) } for (let i = 0; i < next.durations.length - 1; ++i) { SilentAssert.leq(next.durations[i + 1], next.durations[i], - `Sanity check (${testName}): durations[${i}] >= durations[${i + 1}].`) + `Sanity check (${name}): durations[${i}] >= durations[${i + 1}].`) } }; let iteration = 0; @@ -118,7 +141,7 @@ function monotinicity_tester(source, testName) { let set = new Set(); let keys = []; for (let item of snapshot.componentsData) { - let key = `{name: ${item.name}, addonId: ${item.addonId}, isSystem: ${item.isSystem}}`; + let key = `{name: ${item.name}, addonId: ${item.addonId}, isSystem: ${item.isSystem}, windowId: ${item.windowId}}`; keys.push(key); set.add(key); sanityCheck(previous.componentsMap.get(key), item); @@ -166,17 +189,40 @@ add_task(function* test() { let skipTotalUserTime = hasLowPrecision(); + while (true) { - let stats = (yield promiseContentResponse(browser, "compartments-test:getStatistics", null)); - let found = stats.componentsData.find(stat => { - return (stat.name.indexOf(URL) != -1) - && (skipTotalUserTime || stat.totalUserTime > 1000) + yield new Promise(resolve => setTimeout(resolve, 100)); + + // We may have race conditions with DOM loading. + // Don't waste too much brainpower here, let's just ask + // repeatedly for the title to be changed, until this works. + info("Setting titles"); + yield promiseContentResponse(browser, "compartments-test:setTitles", { + parent: PARENT_TITLE, + frames: FRAME_TITLE }); - if (found) { - info(`Expected totalUserTime > 1000, got ${found.totalUserTime}`); + info("Titles set"); + + let stats = (yield promiseContentResponse(browser, "compartments-test:getStatistics", null)); + + // While the webpage consists in three compartments, we should see only + // one `PerformanceData` in `componentsData`. Its `name` is undefined + // (could be either the main frame or one of its subframes), but its + // `title` should be the title of the main frame. + let frame = stats.componentsData.find(stat => stat.title == FRAME_TITLE); + Assert.equal(frame, null, "Searching by title, the frames don't show up in the list of components"); + + let parent = stats.componentsData.find(stat => stat.title == PARENT_TITLE); + if (!parent) { + info("Searching by title, we didn't find the main frame"); + continue; + } + info("Searching by title, we found the main frame"); + + info(`Total user time: ${parent.totalUserTime}`); + if (skipTotalUserTime || parent.totalUserTime > 1000) { break; } - yield new Promise(resolve => setTimeout(resolve, 100)); } // Cleanup diff --git a/toolkit/components/perfmonitoring/tests/browser/browser_compartments_frame.html b/toolkit/components/perfmonitoring/tests/browser/browser_compartments_frame.html new file mode 100644 index 000000000000..69edfe871bab --- /dev/null +++ b/toolkit/components/perfmonitoring/tests/browser/browser_compartments_frame.html @@ -0,0 +1,12 @@ + + + + + Subframe for test browser_compartments.html (do not change this title) + + + + +Subframe loaded. + + diff --git a/toolkit/components/perfmonitoring/tests/browser/browser_compartments_script.js b/toolkit/components/perfmonitoring/tests/browser/browser_compartments_script.js new file mode 100644 index 000000000000..d588874b3044 --- /dev/null +++ b/toolkit/components/perfmonitoring/tests/browser/browser_compartments_script.js @@ -0,0 +1,20 @@ + +window.addEventListener("message", e => { + console.log("frame content", "message", e); + if ("title" in e.data) { + document.title = e.data.title; + } +}); + +// Use some CPU. +window.setInterval(() => { + // Compute an arbitrary value, print it out to make sure that the JS + // engine doesn't discard all our computation. + var date = Date.now(); + var array = []; + var i = 0; + while (Date.now() - date <= 100) { + array[i%2] = i++; + } + console.log("Arbitrary value", window.location, array); +}, 300); From 5b077b70ed06b4596907b33bcd6683a620118977 Mon Sep 17 00:00:00 2001 From: Aaron Raimist Date: Tue, 12 May 2015 20:23:00 -0400 Subject: [PATCH 05/33] Bug 1154059 - Fix color swatches in ruleview.css and computedview.css. r=miker --HG-- extra : rebase_source : 7d4e688e2a60e48ae2dd278e5048ff7d22737f86 extra : histedit_source : 1fd30706c6fee53abd9edc8aca0df198e2e7c114 --- browser/themes/shared/devtools/computedview.css | 6 +++--- browser/themes/shared/devtools/ruleview.css | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/browser/themes/shared/devtools/computedview.css b/browser/themes/shared/devtools/computedview.css index b920d7b887bf..3f7e3dee3a4f 100644 --- a/browser/themes/shared/devtools/computedview.css +++ b/browser/themes/shared/devtools/computedview.css @@ -183,9 +183,9 @@ body { .computedview-colorswatch { border-radius: 50%; - width: 1em; - height: 1em; - vertical-align: text-top; + width: 0.9em; + height: 0.9em; + vertical-align: middle; -moz-margin-end: 5px; display: inline-block; position: relative; diff --git a/browser/themes/shared/devtools/ruleview.css b/browser/themes/shared/devtools/ruleview.css index b6647751282d..c247d6191f1c 100644 --- a/browser/themes/shared/devtools/ruleview.css +++ b/browser/themes/shared/devtools/ruleview.css @@ -160,9 +160,9 @@ .ruleview-swatch { cursor: pointer; border-radius: 50%; - width: 1em; - height: 1em; - vertical-align: text-top; + width: 0.9em; + height: 0.9em; + vertical-align: middle; -moz-margin-end: 5px; display: inline-block; position: relative; From 7fec4dca210ddeeaaa7442cf5a2d98b68605f57d Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 14 May 2015 05:35:00 -0400 Subject: [PATCH 06/33] Bug 1154809 - Remove dead css-parsing code. r=pbrosset --HG-- extra : rebase_source : 5de56b0d53f5e76d4bba51369b0f5d35d280d89e extra : histedit_source : 447dd118274dd40388369c762f820bfbd5222484 --- .../devtools/shared/test/browser_css_color.js | 15 ------ browser/devtools/shared/widgets/Tooltip.js | 3 -- browser/devtools/styleinspector/rule-view.js | 22 --------- toolkit/devtools/css-color.js | 46 ------------------- toolkit/devtools/styleinspector/css-logic.js | 8 ---- 5 files changed, 94 deletions(-) diff --git a/browser/devtools/shared/test/browser_css_color.js b/browser/devtools/shared/test/browser_css_color.js index f1dd1d537448..9280f796d54d 100644 --- a/browser/devtools/shared/test/browser_css_color.js +++ b/browser/devtools/shared/test/browser_css_color.js @@ -42,7 +42,6 @@ function testColorUtils(canvas) { testColorMatch(name, hex, hsl, rgb, color.rgba, canvas); } - testProcessCSSString(); testSetAlpha(); } @@ -106,20 +105,6 @@ function testColorMatch(name, hex, hsl, rgb, rgba, canvas) { test(rgb, "rgb"); } -function testProcessCSSString() { - let before = "border: 1px solid red; border-radius: 5px; " + - "color rgb(0, 255, 0); font-weight: bold; " + - "background-color: transparent; " + - "border-top-color: rgba(0, 0, 255, 0.5);"; - let expected = "border: 1px solid #F00; border-radius: 5px; " + - "color #0F0; font-weight: bold; " + - "background-color: transparent; " + - "border-top-color: rgba(0, 0, 255, 0.5);"; - let after = colorUtils.processCSSString(before); - - is(after, expected, "CSS string processed correctly"); -} - function testSetAlpha() { let values = [ ["longhex", "#ff0000", 0.5, "rgba(255, 0, 0, 0.5)"], diff --git a/browser/devtools/shared/widgets/Tooltip.js b/browser/devtools/shared/widgets/Tooltip.js index 34859cd3ae8a..98e8eefc9b4d 100644 --- a/browser/devtools/shared/widgets/Tooltip.js +++ b/browser/devtools/shared/widgets/Tooltip.js @@ -35,9 +35,6 @@ XPCOMUtils.defineLazyModuleGetter(this, "VariablesViewController", XPCOMUtils.defineLazyModuleGetter(this, "Task", "resource://gre/modules/Task.jsm"); -const GRADIENT_RE = /\b(repeating-)?(linear|radial)-gradient\(((rgb|hsl)a?\(.+?\)|[^\)])+\)/gi; -const BORDERCOLOR_RE = /^border-[-a-z]*color$/ig; -const BORDER_RE = /^border(-(top|bottom|left|right))?$/ig; const XHTML_NS = "http://www.w3.org/1999/xhtml"; const SPECTRUM_FRAME = "chrome://browser/content/devtools/spectrum-frame.xhtml"; const CUBIC_BEZIER_FRAME = "chrome://browser/content/devtools/cubic-bezier-frame.xhtml"; diff --git a/browser/devtools/styleinspector/rule-view.js b/browser/devtools/styleinspector/rule-view.js index 1e26110f121b..724105f0abbf 100644 --- a/browser/devtools/styleinspector/rule-view.js +++ b/browser/devtools/styleinspector/rule-view.js @@ -34,15 +34,9 @@ const FILTER_CHANGED_TIMEOUT = 150; * used to parse CSSStyleDeclaration's cssText attribute. */ -// Used to split on css line separators -const CSS_LINE_RE = /(?:[^;\(]*(?:\([^\)]*?\))?[^;\(]*)*;?/g; - // Used to parse a single property line. const CSS_PROP_RE = /\s*([^:\s]*)\s*:\s*(.*?)\s*(?:! (important))?;?$/; -// Used to parse an external resource from a property value -const CSS_RESOURCE_RE = /url\([\'\"]?(.*?)[\'\"]?\)/; - const IOService = Cc["@mozilla.org/network/io-service;1"] .getService(Ci.nsIIOService); @@ -2845,22 +2839,6 @@ TextPropertyEditor.prototype = { return relativePath; }, - /** - * Check the property value to find an external resource (if any). - * @return {string} the URI in the property value, or null if there is no match. - */ - getResourceURI: function() { - let val = this.prop.value; - let uriMatch = CSS_RESOURCE_RE.exec(val); - let uri = null; - - if (uriMatch && uriMatch[1]) { - uri = uriMatch[1]; - } - - return uri; - }, - /** * Populate the span based on changes to the TextProperty. */ diff --git a/toolkit/devtools/css-color.js b/toolkit/devtools/css-color.js index 4451ecb4f513..80de9cace9ae 100644 --- a/toolkit/devtools/css-color.js +++ b/toolkit/devtools/css-color.js @@ -9,25 +9,8 @@ const {Services} = Cu.import("resource://gre/modules/Services.jsm", {}); const COLOR_UNIT_PREF = "devtools.defaultColorUnit"; -const REGEX_JUST_QUOTES = /^""$/; const REGEX_HSL_3_TUPLE = /^\bhsl\(([\d.]+),\s*([\d.]+%),\s*([\d.]+%)\)$/i; -/** - * This regex matches: - * - #F00 - * - #FF0000 - * - hsl() - * - hsla() - * - rgb() - * - rgba() - * - red - * - * It also matches css keywords e.g. "background-color" otherwise - * "background" would be replaced with #6363CE ("background" is a platform - * color). - */ -const REGEX_ALL_COLORS = /#[0-9a-fA-F]{3}\b|#[0-9a-fA-F]{6}\b|hsl\(.*?\)|hsla\(.*?\)|rgba?\(.*?\)|\b[a-zA-Z-]+\b/g; - const SPECIALVALUES = new Set([ "currentcolor", "initial", @@ -66,9 +49,6 @@ const SPECIALVALUES = new Set([ * // Color objects can be reused * color.newColor("green") === "#0F0"; // true * - * let processed = colorUtils.processCSSString("color:red; background-color:green;"); - * // Returns "color:#F00; background-color:#0F0;" - * * Valid values for COLOR_UNIT_PREF are contained in CssColor.COLORUNIT. */ @@ -78,7 +58,6 @@ function CssColor(colorValue) { module.exports.colorUtils = { CssColor: CssColor, - processCSSString: processCSSString, rgbToHsl: rgbToHsl, setAlpha: setAlpha }; @@ -364,31 +343,6 @@ CssColor.prototype = { }, }; -/** - * Process a CSS string - * - * @param {String} value - * CSS string e.g. "color:red; background-color:green;" - * @return {String} - * Converted CSS String e.g. "color:#F00; background-color:#0F0;" - */ -function processCSSString(value) { - if (value && REGEX_JUST_QUOTES.test(value)) { - return value; - } - - let colorPattern = REGEX_ALL_COLORS; - - value = value.replace(colorPattern, function(match) { - let color = new CssColor(match); - if (color.valid) { - return color; - } - return match; - }); - return value; -} - /** * Convert rgb value to hsl * diff --git a/toolkit/devtools/styleinspector/css-logic.js b/toolkit/devtools/styleinspector/css-logic.js index 9ba9c9cbc023..e56b74efbc22 100644 --- a/toolkit/devtools/styleinspector/css-logic.js +++ b/toolkit/devtools/styleinspector/css-logic.js @@ -42,14 +42,6 @@ const { Cc, Ci, Cu } = require("chrome"); const Services = require("Services"); const DevToolsUtils = require("devtools/toolkit/DevToolsUtils"); -const RX_UNIVERSAL_SELECTOR = /\s*\*\s*/g; -const RX_NOT = /:not\((.*?)\)/g; -const RX_PSEUDO_CLASS_OR_ELT = /(:[\w-]+\().*?\)/g; -const RX_CONNECTORS = /\s*[\s>+~]\s*/g; -const RX_ID = /\s*#\w+\s*/g; -const RX_CLASS_OR_ATTRIBUTE = /\s*(?:\.\w+|\[.+?\])\s*/g; -const RX_PSEUDO = /\s*:?:([\w-]+)(\(?\)?)\s*/g; - // This should be ok because none of the functions that use this should be used // on the worker thread, where Cu is not available. if (Cu) { From 412abec581a708e13b82e06071dc7098e5df47d5 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 14 May 2015 05:35:00 -0400 Subject: [PATCH 07/33] Bug 1154809 - Rename and clarify last CSS regexp in rule-view.js. r=pbrosset --HG-- extra : rebase_source : fc11cc70ad7efd590732302791d380ea8748c51c extra : histedit_source : 9892fd09dd39fcc5a82d76f286e3fb8c808a3aef --- browser/devtools/styleinspector/rule-view.js | 11 ++-- .../devtools/styleinspector/test/browser.ini | 1 + .../test/browser_ruleview_search-filter_11.js | 51 +++++++++++++++++++ 3 files changed, 55 insertions(+), 8 deletions(-) create mode 100644 browser/devtools/styleinspector/test/browser_ruleview_search-filter_11.js diff --git a/browser/devtools/styleinspector/rule-view.js b/browser/devtools/styleinspector/rule-view.js index 724105f0abbf..975b361987a0 100644 --- a/browser/devtools/styleinspector/rule-view.js +++ b/browser/devtools/styleinspector/rule-view.js @@ -29,13 +29,8 @@ const PREF_ENABLE_MDN_DOCS_TOOLTIP = "devtools.inspector.mdnDocsTooltip.enabled" const PROPERTY_NAME_CLASS = "ruleview-propertyname"; const FILTER_CHANGED_TIMEOUT = 150; -/** - * These regular expressions are adapted from firebug's css.js, and are - * used to parse CSSStyleDeclaration's cssText attribute. - */ - -// Used to parse a single property line. -const CSS_PROP_RE = /\s*([^:\s]*)\s*:\s*(.*?)\s*(?:! (important))?;?$/; +// This is used to parse user input when filtering. +const FILTER_PROP_RE = /\s*([^:\s]*)\s*:\s*(.*?)\s*;?$/; const IOService = Cc["@mozilla.org/network/io-service;1"] .getService(Ci.nsIIOService); @@ -2094,7 +2089,7 @@ CssRuleView.prototype = { // Parse search value as a single property line and extract the property // name and value. Otherwise, use the search value as both the name and // value. - let propertyMatch = CSS_PROP_RE.exec(aValue); + let propertyMatch = FILTER_PROP_RE.exec(aValue); let name = propertyMatch ? propertyMatch[1] : aValue; let value = propertyMatch ? propertyMatch[2] : aValue; diff --git a/browser/devtools/styleinspector/test/browser.ini b/browser/devtools/styleinspector/test/browser.ini index abacd8c15d62..1ce19c233271 100644 --- a/browser/devtools/styleinspector/test/browser.ini +++ b/browser/devtools/styleinspector/test/browser.ini @@ -132,6 +132,7 @@ skip-if = e10s # Bug 1090340 [browser_ruleview_search-filter_08.js] [browser_ruleview_search-filter_09.js] [browser_ruleview_search-filter_10.js] +[browser_ruleview_search-filter_11.js] [browser_ruleview_search-filter_clear.js] [browser_ruleview_search-filter_context-menu.js] [browser_ruleview_search-filter_escape-keypress.js] diff --git a/browser/devtools/styleinspector/test/browser_ruleview_search-filter_11.js b/browser/devtools/styleinspector/test/browser_ruleview_search-filter_11.js new file mode 100644 index 000000000000..dcbade932692 --- /dev/null +++ b/browser/devtools/styleinspector/test/browser_ruleview_search-filter_11.js @@ -0,0 +1,51 @@ +/* vim: set ft=javascript ts=2 et sw=2 tw=80: */ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +// Tests that the rule view search filter can find "!important". + +const SEARCH = "!important"; + +let TEST_URI = [ + '', + '

Styled Node

' +].join("\n"); + +add_task(function*() { + yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(TEST_URI)); + let {toolbox, inspector, view} = yield openRuleView(); + yield selectNode("#testid", inspector); + yield testAddTextInFilter(inspector, view); +}); + +function* testAddTextInFilter(inspector, ruleView) { + info("Setting filter text to \"" + SEARCH + "\""); + + let win = ruleView.doc.defaultView; + let searchField = ruleView.searchField; + let onRuleViewFiltered = inspector.once("ruleview-filtered"); + + searchField.focus(); + synthesizeKeys(SEARCH, win); + yield onRuleViewFiltered; + + info("Check that the correct rules are visible"); + is(ruleView.element.children.length, 2, "Should have 2 rules."); + is(getRuleViewRuleEditor(ruleView, 0).rule.selectorText, "element", + "First rule is inline element."); + + let rule = getRuleViewRuleEditor(ruleView, 1).rule; + + is(rule.selectorText, "#testid", "Second rule is #testid."); + ok(rule.textProps[0].editor.container.classList.contains("ruleview-highlight"), + "background-color text property is correctly highlighted."); +} From 4a110448f013cb85e89ff71449354ee98b30e84b Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 14 May 2015 05:35:00 -0400 Subject: [PATCH 08/33] Bug 1154809 - Rewrite prettifyCSS to use CSSLexer. r=pbrosset --HG-- extra : rebase_source : afa69805be07b9f3918333ea8edcb8a7b2115c4a extra : histedit_source : cdb808df2d2d01a2817c3675ace9a3a4f85a0c1f --- .../test/browser_styleeditor_pretty.js | 8 +- toolkit/devtools/styleinspector/css-logic.js | 186 +++++++++++++++--- .../devtools/tests/unit/test_prettifyCSS.js | 61 ++++++ toolkit/devtools/tests/unit/xpcshell.ini | 1 + 4 files changed, 219 insertions(+), 37 deletions(-) create mode 100644 toolkit/devtools/tests/unit/test_prettifyCSS.js diff --git a/browser/devtools/styleeditor/test/browser_styleeditor_pretty.js b/browser/devtools/styleeditor/test/browser_styleeditor_pretty.js index 8bc7c2c406c0..513ea56e2900 100644 --- a/browser/devtools/styleeditor/test/browser_styleeditor_pretty.js +++ b/browser/devtools/styleeditor/test/browser_styleeditor_pretty.js @@ -9,16 +9,14 @@ const TESTCASE_URI = TEST_BASE_HTTP + "minified.html"; const PRETTIFIED_SOURCE = "" + -"body\{\r?\n" + // body{ +"body \{\r?\n" + // body{ "\tbackground\:white;\r?\n" + // background:white; "\}\r?\n" + // } -"\r?\n" + // -"div\{\r?\n" + // div{ +"div \{\r?\n" + // div{ "\tfont\-size\:4em;\r?\n" + // font-size:4em; "\tcolor\:red\r?\n" + // color:red "\}\r?\n" + // } -"\r?\n" + // -"span\{\r?\n" + // span{ +"span \{\r?\n" + // span{ "\tcolor\:green;\r?\n" // color:green; "\}\r?\n"; // } diff --git a/toolkit/devtools/styleinspector/css-logic.js b/toolkit/devtools/styleinspector/css-logic.js index e56b74efbc22..ee65eb56dfbb 100644 --- a/toolkit/devtools/styleinspector/css-logic.js +++ b/toolkit/devtools/styleinspector/css-logic.js @@ -995,50 +995,172 @@ CssLogic.prettifyCSS = function(text, ruleCount) { return text; } - let parts = []; // indented parts - let partStart = 0; // start offset of currently parsed part + // We reformat the text using a simple state machine. The + // reformatting preserves most of the input text, changing only + // whitespace. The rules are: + // + // * After a "{" or ";" symbol, ensure there is a newline and + // indentation before the next non-comment, non-whitespace token. + // * Additionally after a "{" symbol, increase the indentation. + // * A "}" symbol ensures there is a preceding newline, and + // decreases the indentation level. + // * Ensure there is whitespace before a "{". + // + // This approach can be confused sometimes, but should do ok on a + // minified file. let indent = ""; let indentLevel = 0; + let tokens = domUtils.getCSSLexer(text); + let result = ""; + let pushbackToken = undefined; - for (let i = 0; i < text.length; i++) { - let c = text[i]; - let shouldIndent = false; - - switch (c) { - case "}": - if (i - partStart > 1) { - // there's more than just } on the line, add line - parts.push(indent + text.substring(partStart, i)); - partStart = i; - } - indent = TAB_CHARS.repeat(--indentLevel); - /* fallthrough */ - case ";": - case "{": - shouldIndent = true; - break; + // A helper function that reads tokens, looking for the next + // non-comment, non-whitespace token. Comment and whitespace tokens + // are appended to |result|. If this encounters EOF, it returns + // null. Otherwise it returns the last whitespace token that was + // seen. This function also updates |pushbackToken|. + let readUntilSignificantToken = () => { + while (true) { + let token = tokens.nextToken(); + if (!token || token.tokenType !== "whitespace") { + pushbackToken = token; + return token; + } + // Saw whitespace. Before committing to it, check the next + // token. + let nextToken = tokens.nextToken(); + if (!nextToken || nextToken.tokenType !== "comment") { + pushbackToken = nextToken; + return token; + } + // Saw whitespace + comment. Update the result and continue. + result = result + text.substring(token.startOffset, nextToken.endOffset); } + }; - if (shouldIndent) { - let la = text[i+1]; // one-character lookahead - if (!/\n/.test(la) || /^\s+$/.test(text.substring(i+1, text.length))) { - // following character should be a new line, but isn't, - // or it's whitespace at the end of the file - parts.push(indent + text.substring(partStart, i + 1)); - if (c == "}") { - parts.push(""); // for extra line separator - } - partStart = i + 1; + // State variables for readUntilNewlineNeeded. + // + // Starting index of the accumulated tokens. + let startIndex; + // Ending index of the accumulated tokens. + let endIndex; + // True if any non-whitespace token was seen. + let anyNonWS; + // True if the terminating token is "}". + let isCloseBrace; + // True if the token just before the terminating token was + // whitespace. + let lastWasWS; + + // A helper function that reads tokens until there is a reason to + // insert a newline. This updates the state variables as needed. + // If this encounters EOF, it returns null. Otherwise it returns + // the final token read. Note that if the returned token is "{", + // then it will not be included in the computed start/end token + // range. This is used to handle whitespace insertion before a "{". + let readUntilNewlineNeeded = () => { + let token; + while (true) { + if (pushbackToken) { + token = pushbackToken; + pushbackToken = undefined; } else { - return text; // assume it is not minified, early exit + token = tokens.nextToken(); + } + if (!token) { + endIndex = text.length; + break; + } + + // A "}" symbol must be inserted later, to deal with indentation + // and newline. + if (token.tokenType === "symbol" && token.text === "}") { + isCloseBrace = true; + break; + } else if (token.tokenType === "symbol" && token.text === "{") { + break; + } + + if (token.tokenType !== "whitespace") { + anyNonWS = true; + } + + if (startIndex === undefined) { + startIndex = token.startOffset; + } + endIndex = token.endOffset; + + if (token.tokenType === "symbol" && token.text === ';') { + break; + } + + lastWasWS = token.tokenType === "whitespace"; + } + return token; + }; + + while (true) { + // Set the initial state. + startIndex = undefined; + endIndex = undefined; + anyNonWS = false; + isCloseBrace = false; + lastWasWS = false; + + // Read tokens until we see a reason to insert a newline. + let token = readUntilNewlineNeeded(); + + // Append any saved up text to the result, applying indentation. + if (startIndex !== undefined) { + if (isCloseBrace && !anyNonWS) { + // If we saw only whitespace followed by a "}", then we don't + // need anything here. + } else { + result = result + indent + text.substring(startIndex, endIndex); + if (isCloseBrace) + result += CssLogic.LINE_SEPARATOR; } } - if (c == "{") { + if (isCloseBrace) { + indent = TAB_CHARS.repeat(--indentLevel); + result = result + indent + '}'; + } + + if (!token) { + break; + } + + if (token.tokenType === "symbol" && token.text === '{') { + if (!lastWasWS) { + result += ' '; + } + result += '{'; indent = TAB_CHARS.repeat(++indentLevel); } + + // Now it is time to insert a newline. However first we want to + // deal with any trailing comments. + token = readUntilSignificantToken(); + + // "Early" bail-out if the text does not appear to be minified. + // Here we ignore the case where whitespace appears at the end of + // the text. + if (pushbackToken && token && token.tokenType === "whitespace" && + /\n/g.test(text.substring(token.startOffset, token.endOffset))) { + return text; + } + + // Finally time for that newline. + result = result + CssLogic.LINE_SEPARATOR; + + // Maybe we hit EOF. + if (!pushbackToken) { + break; + } } - return parts.join(CssLogic.LINE_SEPARATOR); + + return result; }; /** diff --git a/toolkit/devtools/tests/unit/test_prettifyCSS.js b/toolkit/devtools/tests/unit/test_prettifyCSS.js new file mode 100644 index 000000000000..d3d680354814 --- /dev/null +++ b/toolkit/devtools/tests/unit/test_prettifyCSS.js @@ -0,0 +1,61 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +// Test prettifyCSS. + +"use strict"; + +const loader = new DevToolsLoader(); +const require = loader.require; +const {CssLogic} = require("devtools/styleinspector/css-logic"); + + +const TESTS = [ + { name: "simple test", + input: "div { font-family:'Arial Black', Arial, sans-serif; }", + expected: [ + "div {", + "\tfont-family:'Arial Black', Arial, sans-serif;", + "}" + ] + }, + + { name: "whitespace before open brace", + input: "div{}", + expected: [ + "div {", + "}" + ] + }, + + { name: "minified with trailing newline", + input: "\nbody{background:white;}div{font-size:4em;color:red}span{color:green;}\n", + expected: [ + "", + "body {", + "\tbackground:white;", + "}", + "div {", + "\tfont-size:4em;", + "\tcolor:red", + "}", + "span {", + "\tcolor:green;", + "}" + ] + }, + +]; + +function run_test() { + for (let test of TESTS) { + do_print(test.name); + + // Note that CssLogic.LINE_SEPARATOR is computed lazily, so we + // have to do things in the right order here. + let output = CssLogic.prettifyCSS(test.input); + let expected = test.expected.join(CssLogic.LINE_SEPARATOR) + + CssLogic.LINE_SEPARATOR; + equal(output, expected, test.name); + } +} diff --git a/toolkit/devtools/tests/unit/xpcshell.ini b/toolkit/devtools/tests/unit/xpcshell.ini index 7ba8de72e345..81684e11e806 100644 --- a/toolkit/devtools/tests/unit/xpcshell.ini +++ b/toolkit/devtools/tests/unit/xpcshell.ini @@ -12,6 +12,7 @@ support-files = [test_defineLazyPrototypeGetter.js] [test_async-utils.js] [test_consoleID.js] +[test_prettifyCSS.js] [test_require_lazy.js] [test_require.js] [test_stack.js] From a04ecc80d27e7a6036635ae45453c1fc30ee582e Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 14 May 2015 05:36:00 -0400 Subject: [PATCH 09/33] Bug 1154809 - Rewrite tokenizeComputedFilter to use cssTokenizer. r=pbrosset --HG-- extra : rebase_source : b6671492aaccc26110eeef31db4cecb4e8210cd1 extra : histedit_source : 44b256236eeef8221147574606d9a53e40732be9 --- .../shared/test/browser_filter-editor-01.js | 2 +- .../shared/test/browser_filter-editor-02.js | 2 +- .../shared/test/browser_filter-editor-05.js | 2 +- .../devtools/shared/widgets/FilterWidget.js | 56 ++++++++++--------- 4 files changed, 34 insertions(+), 28 deletions(-) diff --git a/browser/devtools/shared/test/browser_filter-editor-01.js b/browser/devtools/shared/test/browser_filter-editor-01.js index c63a61e50b64..bfcf4c55c048 100644 --- a/browser/devtools/shared/test/browser_filter-editor-01.js +++ b/browser/devtools/shared/test/browser_filter-editor-01.js @@ -29,7 +29,7 @@ add_task(function *() { widget.setCssValue("drop-shadow( 2px 1px 5px black) url( example.svg#filter )"); const computedURI = "chrome://browser/content/devtools/example.svg#filter"; - const expected = `drop-shadow(rgb(0, 0, 0) 2px 1px 5px) url("${computedURI}")`; + const expected = `drop-shadow(rgb(0, 0, 0) 2px 1px 5px) url(${computedURI})`; is(widget.getCssValue(), expected, "setCssValue should work for string-typed values"); }); diff --git a/browser/devtools/shared/test/browser_filter-editor-02.js b/browser/devtools/shared/test/browser_filter-editor-02.js index 9ff2c5d1adfb..a673d78e0519 100644 --- a/browser/devtools/shared/test/browser_filter-editor-02.js +++ b/browser/devtools/shared/test/browser_filter-editor-02.js @@ -47,7 +47,7 @@ add_task(function*() { expected: [ { label: "url", - value: "\"chrome://browser/content/devtools/example.svg\"", + value: "chrome://browser/content/devtools/example.svg", unit: null } ] diff --git a/browser/devtools/shared/test/browser_filter-editor-05.js b/browser/devtools/shared/test/browser_filter-editor-05.js index 3227a5e3b92f..b6423f871914 100644 --- a/browser/devtools/shared/test/browser_filter-editor-05.js +++ b/browser/devtools/shared/test/browser_filter-editor-05.js @@ -127,6 +127,6 @@ add_task(function*() { shiftKey: true }); - is(widget.getValueAt(1), "\"chrome://browser/content/devtools/test.svg\"", + is(widget.getValueAt(1), "chrome://browser/content/devtools/test.svg", "Label-dragging on string-type filters shouldn't affect their value"); }); diff --git a/browser/devtools/shared/widgets/FilterWidget.js b/browser/devtools/shared/widgets/FilterWidget.js index 8d35d254be28..2174004f77af 100644 --- a/browser/devtools/shared/widgets/FilterWidget.js +++ b/browser/devtools/shared/widgets/FilterWidget.js @@ -14,6 +14,7 @@ const { Cu } = require("chrome"); const { ViewHelpers } = Cu.import("resource:///modules/devtools/ViewHelpers.jsm", {}); const STRINGS_URI = "chrome://browser/locale/devtools/filterwidget.properties"; const L10N = new ViewHelpers.L10N(STRINGS_URI); +const {cssTokenizer} = require("devtools/sourceeditor/css-tokenizer"); const DEFAULT_FILTER_TYPE = "length"; const UNIT_MAPPING = { @@ -750,41 +751,46 @@ function swapArrayIndices(array, a, b) { */ function tokenizeComputedFilter(css) { let filters = []; - let current = ""; let depth = 0; if (css === "none") { return filters; } - while (css.length) { - const char = css[0]; + let state = "initial"; + let name; + let contents; + for (let token of cssTokenizer(css)) { + switch (state) { + case "initial": + if (token.tokenType === "function") { + name = token.text; + contents = ""; + state = "function"; + depth = 1; + } else if (token.tokenType === "url" || token.tokenType === "bad_url") { + filters.push({name: "url", value: token.text}); + // Leave state as "initial" because the URL token includes + // the trailing close paren. + } + break; - switch (char) { - case "(": - depth++; - if (depth === 1) { - filters.push({name: current.trim()}); - current = ""; - } else { - current += char; + case "function": + if (token.tokenType === "symbol" && token.text === ")") { + --depth; + if (depth === 0) { + filters.push({name: name, value: contents}); + state = "initial"; + break; + } } - break; - case ")": - depth--; - if (depth === 0) { - filters[filters.length - 1].value = current.trim(); - current = ""; - } else { - current += char; + contents += css.substring(token.startOffset, token.endOffset); + if (token.tokenType === "function" || + (token.tokenType === "symbol" && token.text === "(")) { + ++depth; } - break; - default: - current += char; - break; + break; } - - css = css.slice(1); } return filters; From 4ba6a0cf35fb9517a1f0488b5ed34a1ee8d766ca Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 14 May 2015 05:36:00 -0400 Subject: [PATCH 10/33] Bug 1154809 - Rewrite CubicBezierWidget to use CSSLexer. r=pbrosset --HG-- extra : rebase_source : 750e574f9f9d0336690b6f7e3ec4d95dc2f02ebb extra : histedit_source : b65e1789225b3b4dd84ccb69451cdb8d48eff1c8 --- .../shared/test/browser_cubic-bezier-03.js | 4 +- .../shared/test/unit/test_cubicBezier.js | 27 +++++++- .../shared/widgets/CubicBezierWidget.js | 61 ++++++++++++++----- 3 files changed, 73 insertions(+), 19 deletions(-) diff --git a/browser/devtools/shared/test/browser_cubic-bezier-03.js b/browser/devtools/shared/test/browser_cubic-bezier-03.js index 2c231d5d9216..12d22efa536c 100644 --- a/browser/devtools/shared/test/browser_cubic-bezier-03.js +++ b/browser/devtools/shared/test/browser_cubic-bezier-03.js @@ -58,12 +58,12 @@ function* coordinatesCanBeChangedByProvidingAValue(widget) { info("Setting a custom cubic-bezier css value"); onUpdated = widget.once("updated"); - widget.cssCubicBezierValue = "cubic-bezier(.25,-0.5, 1, 1.45)"; + widget.cssCubicBezierValue = "cubic-bezier(.25,-0.5, 1, 1.25)"; bezier = yield onUpdated; ok(true, "The updated event was fired as a result of setting cssValue"); is(bezier.P1[0], .25, "The new P1 time coordinate is correct"); is(bezier.P1[1], -.5, "The new P1 progress coordinate is correct"); is(bezier.P2[0], 1, "The new P2 time coordinate is correct"); - is(bezier.P2[1], 1.45, "The new P2 progress coordinate is correct"); + is(bezier.P2[1], 1.25, "The new P2 progress coordinate is correct"); } diff --git a/browser/devtools/shared/test/unit/test_cubicBezier.js b/browser/devtools/shared/test/unit/test_cubicBezier.js index ee6c0e07bfde..2230049c8c77 100644 --- a/browser/devtools/shared/test/unit/test_cubicBezier.js +++ b/browser/devtools/shared/test/unit/test_cubicBezier.js @@ -10,7 +10,7 @@ const Cu = Components.utils; let {devtools} = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}); let require = devtools.require; -let {CubicBezier} = require("devtools/shared/widgets/CubicBezierWidget"); +let {CubicBezier, _parseTimingFunction} = require("devtools/shared/widgets/CubicBezierWidget"); function run_test() { throwsWhenMissingCoordinates(); @@ -20,6 +20,7 @@ function run_test() { pointGettersReturnPointCoordinatesArrays(); toStringOutputsCubicBezierValue(); toStringOutputsCssPresetValues(); + testParseTimingFunction(); } function throwsWhenMissingCoordinates() { @@ -108,6 +109,30 @@ function toStringOutputsCssPresetValues() { do_check_eq(c.toString(), "ease-in-out"); } +function testParseTimingFunction() { + do_print("test parseTimingFunction"); + + for (let test of ["ease", "linear", "ease-in", "ease-out", "ease-in-out"]) { + ok(_parseTimingFunction(test), test); + } + + ok(!_parseTimingFunction("something"), "non-function token"); + ok(!_parseTimingFunction("something()"), "non-cubic-bezier function"); + ok(!_parseTimingFunction("cubic-bezier(something)", + "cubic-bezier with non-numeric argument")); + ok(!_parseTimingFunction("cubic-bezier(1,2,3:7)", + "did not see comma")); + ok(!_parseTimingFunction("cubic-bezier(1,2,3,7:", + "did not see close paren")); + ok(!_parseTimingFunction("cubic-bezier(1,2", "early EOF after number")); + ok(!_parseTimingFunction("cubic-bezier(1,2,", "early EOF after comma")); + deepEqual(_parseTimingFunction("cubic-bezier(1,2,3,7)"), [1,2,3,7], + "correct invocation"); + deepEqual(_parseTimingFunction("cubic-bezier(1, /* */ 2,3, 7 )"), + [1,2,3,7], + "correct with comments and whitespace"); +} + function do_check_throws(cb, info) { do_print(info); diff --git a/browser/devtools/shared/widgets/CubicBezierWidget.js b/browser/devtools/shared/widgets/CubicBezierWidget.js index 8b7dcb08c808..7c1e1c4545e1 100644 --- a/browser/devtools/shared/widgets/CubicBezierWidget.js +++ b/browser/devtools/shared/widgets/CubicBezierWidget.js @@ -28,6 +28,10 @@ const EventEmitter = require("devtools/toolkit/event-emitter"); const {setTimeout, clearTimeout} = require("sdk/timers"); const {PREDEFINED, PRESETS, DEFAULT_PRESET_CATEGORY} = require("devtools/shared/widgets/CubicBezierPresets"); +const {Cc, Ci} = require('chrome'); +loader.lazyGetter(this, "DOMUtils", () => { + return Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils); +}); /** * CubicBezier data structure helper @@ -439,12 +443,7 @@ CubicBezierWidget.prototype = { value = value.trim(); // Try with one of the predefined values - let coordinates = PREDEFINED[value]; - - // Otherwise parse the coordinates from the cubic-bezier function - if (!coordinates && value.startsWith("cubic-bezier")) { - coordinates = value.replace(/cubic-bezier|\(|\)/g, "").split(",").map(parseFloat); - } + let coordinates = parseTimingFunction(value); this.presets.refreshMenu(coordinates); this.coordinates = coordinates; @@ -760,7 +759,7 @@ TimingFunctionPreviewWidget.prototype = { clearTimeout(this.autoRestartAnimation); - if (isValidTimingFunction(value)) { + if (parseTimingFunction(value)) { this.dot.style.animationTimingFunction = value; this.restartAnimation(); } @@ -811,24 +810,54 @@ function distance(x1, y1, x2, y2) { } /** - * Checks whether a string is a valid timing-function value + * Parse a string to see whether it is a valid timing function. + * If it is, return the coordinates as an array. + * Otherwise, return undefined. * @param {String} value - * @return {Boolean} + * @return {Array} of coordinates, or undefined */ -function isValidTimingFunction(value) { - // Either it's a predefined value +function parseTimingFunction(value) { if (value in PREDEFINED) { - return true; + return PREDEFINED[value]; } - // Or it has to match a cubic-bezier expression - if (value.match(/^cubic-bezier\(([0-9.\- ]+,){3}[0-9.\- ]+\)/)) { - return true; + let tokenStream = DOMUtils.getCSSLexer(value); + let getNextToken = () => { + while (true) { + let token = tokenStream.nextToken(); + if (!token || (token.tokenType !== "whitespace" && + token.tokenType !== "comment")) { + return token; + } + } + }; + + let token = getNextToken(); + if (token.tokenType !== "function" || token.text !== "cubic-bezier") { + return undefined; } - return false; + let result = []; + for (let i = 0; i < 4; ++i) { + token = getNextToken(); + if (!token || token.tokenType !== "number") { + return undefined; + } + result.push(token.number); + + token = getNextToken(); + if (!token || token.tokenType !== "symbol" || + token.text !== (i == 3 ? ")" : ",")) { + return undefined; + } + } + + return result; } +// This is exported for testing. +exports._parseTimingFunction = parseTimingFunction; + /** * Removes a class from a node and adds it to another. * @param {String} className the class to swap From 34f58de2b4874bf892f5e0aaad7a44b0ebb1b2fe Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 14 May 2015 05:36:00 -0400 Subject: [PATCH 11/33] Bug 1154809 - Avoid regexps in css-color. r=pbrosset --HG-- extra : rebase_source : 280b2956d5a58f9e4ebf9891c58b0a5b9dc4ed4d extra : histedit_source : 7f13bbfb92f06be5238d2742b7e40b9105db4845 --- toolkit/devtools/css-color.js | 34 ++++++++++++++++------------------ 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/toolkit/devtools/css-color.js b/toolkit/devtools/css-color.js index 80de9cace9ae..c7612c6564e1 100644 --- a/toolkit/devtools/css-color.js +++ b/toolkit/devtools/css-color.js @@ -9,8 +9,6 @@ const {Services} = Cu.import("resource://gre/modules/Services.jsm", {}); const COLOR_UNIT_PREF = "devtools.defaultColorUnit"; -const REGEX_HSL_3_TUPLE = /^\bhsl\(([\d.]+),\s*([\d.]+%),\s*([\d.]+%)\)$/i; - const SPECIALVALUES = new Set([ "currentcolor", "initial", @@ -162,9 +160,9 @@ CssColor.prototype = { if (this.hasAlpha) { return this.rgba; } - return this.rgb.replace(/\brgb\((\d{1,3}),\s*(\d{1,3}),\s*(\d{1,3})\)/gi, function(_, r, g, b) { - return "#" + ((1 << 24) + (r << 16) + (g << 8) + (b << 0)).toString(16).substr(-6).toUpperCase(); - }); + + let tuple = this._getRGBATuple(); + return "#" + ((1 << 24) + (tuple.r << 16) + (tuple.g << 8) + (tuple.b << 0)).toString(16).substr(-6).toUpperCase(); }, get rgb() { @@ -211,7 +209,7 @@ CssColor.prototype = { if (this.hasAlpha) { return this.hsla; } - return this._hslNoAlpha(); + return this._hsl(); }, get hsla() { @@ -225,9 +223,9 @@ CssColor.prototype = { } if (this.hasAlpha) { let a = this._getRGBATuple().a; - return this._hslNoAlpha().replace("hsl", "hsla").replace(")", ", " + a + ")"); + return this._hsl(a); } - return this._hslNoAlpha().replace("hsl", "hsla").replace(")", ", 1)"); + return this._hsl(1); }, /** @@ -320,19 +318,19 @@ CssColor.prototype = { return tuple; }, - _hslNoAlpha: function() { - let {r, g, b} = this._getRGBATuple(); - - if (this.authored.startsWith("hsl(")) { - // We perform string manipulations on our output so let's ensure that it - // is formatted as we expect. - let [, h, s, l] = this.authored.match(REGEX_HSL_3_TUPLE); - return "hsl(" + h + ", " + s + ", " + l + ")"; + _hsl: function(maybeAlpha) { + if (this.authored.startsWith("hsl(") && maybeAlpha === undefined) { + // We can use it as-is. + return this.authored; } + let {r, g, b} = this._getRGBATuple(); let [h,s,l] = rgbToHsl([r,g,b]); - - return "hsl(" + h + ", " + s + "%, " + l + "%)"; + if (maybeAlpha !== undefined) { + return "hsla(" + h + ", " + s + "%, " + l + "%, " + maybeAlpha + ")"; + } else { + return "hsl(" + h + ", " + s + "%, " + l + "%)"; + } }, /** From dcbfc55e0d13f83050fed59195bbd28cd2ae3f56 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Thu, 14 May 2015 05:36:00 -0400 Subject: [PATCH 12/33] Bug 1154809 - Rewrite output-parser.js to use CSSLexer. r=pbrosset --HG-- extra : rebase_source : d34aedb698014930b3c079b2aef4f32414b285c6 extra : histedit_source : bf2d4ec50f27663826a5607653895ab9cd264a78 --- toolkit/devtools/output-parser.js | 285 +++++++++++++----------------- 1 file changed, 122 insertions(+), 163 deletions(-) diff --git a/toolkit/devtools/output-parser.js b/toolkit/devtools/output-parser.js index cf421305c03f..efb7d2b58acd 100644 --- a/toolkit/devtools/output-parser.js +++ b/toolkit/devtools/output-parser.js @@ -11,28 +11,18 @@ const {Services} = Cu.import("resource://gre/modules/Services.jsm", {}); const HTML_NS = "http://www.w3.org/1999/xhtml"; const MAX_ITERATIONS = 100; -const REGEX_QUOTES = /^".*?"|^".*|^'.*?'|^'.*/; -const REGEX_WHITESPACE = /^\s+/; -const REGEX_FIRST_WORD_OR_CHAR = /^\w+|^./; -const REGEX_CUBIC_BEZIER = /^linear|^ease-in-out|^ease-in|^ease-out|^ease|^cubic-bezier\(([0-9.\- ]+,){3}[0-9.\- ]+\)/; -// CSS variable names are identifiers which the spec defines as follows: -// In CSS, identifiers (including element names, classes, and IDs in -// selectors) can contain only the characters [a-zA-Z0-9] and ISO 10646 -// characters U+00A0 and higher, plus the hyphen (-) and the underscore (_). -const REGEX_CSS_VAR = /^\bvar\(\s*--[-_a-zA-Z0-9\u00A0-\u10FFFF]+\s*\)/; +const BEZIER_KEYWORDS = ["linear", "ease-in-out", "ease-in", "ease-out", "ease"]; -/** - * This regex matches: - * - #F00 - * - #FF0000 - * - hsl() - * - hsla() - * - rgb() - * - rgba() - * - color names - */ -const REGEX_ALL_COLORS = /^#[0-9a-fA-F]{3}\b|^#[0-9a-fA-F]{6}\b|^hsl\(.*?\)|^hsla\(.*?\)|^rgba?\(.*?\)|^[a-zA-Z-]+/; +// Functions that accept a color argument. +const COLOR_TAKING_FUNCTIONS = ["linear-gradient", + "-moz-linear-gradient", + "repeating-linear-gradient", + "-moz-repeating-linear-gradient", + "radial-gradient", + "-moz-radial-gradient", + "repeating-radial-gradient", + "-moz-repeating-radial-gradient"]; loader.lazyGetter(this, "DOMUtils", function () { return Cc["@mozilla.org/inspector/dom-utils;1"].getService(Ci.inIDOMUtils); @@ -94,29 +84,44 @@ OutputParser.prototype = { }, /** - * Matches the beginning of the provided string to a css background-image url - * and return both the whole url(...) match and the url itself. - * This isn't handled via a regular expression to make sure we can match urls - * that contain parenthesis easily + * Given an initial FUNCTION token, read tokens from |tokenStream| + * and collect all the (non-comment) text. Return the collected + * text. The function token and the close paren are included in the + * result. + * + * @param {CSSToken} initialToken + * The FUNCTION token. + * @param {String} text + * The original CSS text. + * @param {CSSLexer} tokenStream + * The token stream from which to read. + * @return {String} + * The text of body of the function call. */ - _matchBackgroundUrl: function(text) { - let startToken = "url("; - if (text.indexOf(startToken) !== 0) { - return null; + _collectFunctionText: function(initialToken, text, tokenStream) { + let result = text.substring(initialToken.startOffset, + initialToken.endOffset); + let depth = 1; + while (depth > 0) { + let token = tokenStream.nextToken(); + if (!token) { + break; + } + if (token.tokenType === "comment") { + continue; + } + result += text.substring(token.startOffset, token.endOffset); + if (token.tokenType === "symbol") { + if (token.text === "(") { + ++depth; + } else if (token.text === ")") { + --depth; + } + } else if (token.tokenType === "function") { + ++depth; + } } - - let uri = text.substring(startToken.length).trim(); - let quote = uri.substring(0, 1); - if (quote === "'" || quote === '"') { - uri = uri.substring(1, uri.search(new RegExp(quote + "\\s*\\)"))); - } else { - uri = uri.substring(0, uri.indexOf(")")); - quote = ""; - } - let end = startToken + quote + uri; - text = text.substring(0, text.indexOf(")", end.length) + 1); - - return [text, uri.trim()]; + return result; }, /** @@ -133,142 +138,96 @@ OutputParser.prototype = { _parse: function(text, options={}) { text = text.trim(); this.parsed.length = 0; - let i = 0; - while (text.length > 0) { - let matched = null; - - // Prevent this loop from slowing down the browser with too - // many nodes being appended into output. In practice it is very unlikely - // that this will ever happen. - i++; - if (i > MAX_ITERATIONS) { - this._appendTextNode(text); - text = ""; - break; - } - - if (options.expectFilter) { - this._appendFilter(text, options); - break; - } - - matched = text.match(REGEX_QUOTES); - if (matched) { - let match = matched[0]; - - text = this._trimMatchFromStart(text, match); - this._appendTextNode(match); - continue; - } - - matched = text.match(REGEX_CSS_VAR); - if (matched) { - let match = matched[0]; - - text = this._trimMatchFromStart(text, match); - this._appendTextNode(match); - continue; - } - - matched = text.match(REGEX_WHITESPACE); - if (matched) { - let match = matched[0]; - - text = this._trimMatchFromStart(text, match); - this._appendTextNode(match); - continue; - } - - matched = this._matchBackgroundUrl(text); - if (matched) { - let [match, url] = matched; - text = this._trimMatchFromStart(text, match); - - this._appendURL(match, url, options); - continue; - } - - if (options.expectCubicBezier) { - matched = text.match(REGEX_CUBIC_BEZIER); - if (matched) { - let match = matched[0]; - text = this._trimMatchFromStart(text, match); - - this._appendCubicBezier(match, options); + if (options.expectFilter) { + this._appendFilter(text, options); + } else { + let tokenStream = DOMUtils.getCSSLexer(text); + let i = 0; + while (true) { + let token = tokenStream.nextToken(); + if (!token) + break; + if (token.tokenType === "comment") { continue; } - } - if (options.supportsColor) { - let dirty; - - [text, dirty] = this._appendColorOnMatch(text, options); - - if (dirty) { + // Prevent this loop from slowing down the browser with too + // many nodes being appended into output. In practice it is very unlikely + // that this will ever happen. + i++; + if (i > MAX_ITERATIONS) { + this._appendTextNode(text.substring(token.startOffset, + token.endOffset)); continue; } - } - // This test must always be last as it indicates use of an unknown - // character that needs to be removed to prevent infinite loops. - matched = text.match(REGEX_FIRST_WORD_OR_CHAR); - if (matched) { - let match = matched[0]; + switch (token.tokenType) { + case "function": { + if (COLOR_TAKING_FUNCTIONS.indexOf(token.text) >= 0) { + // The function can accept a color argument, and we know + // it isn't special in some other way. So, we let it + // through to the ordinary parsing loop so that colors + // can be handled in a single place. + this._appendTextNode(text.substring(token.startOffset, + token.endOffset)); + } else { + let functionText = this._collectFunctionText(token, text, + tokenStream); - text = this._trimMatchFromStart(text, match); - this._appendTextNode(match); + if (options.expectCubicBezier && token.text === "cubic-bezier") { + this._appendCubicBezier(functionText, options); + } else if (options.supportsColor && + DOMUtils.isValidCSSColor(functionText)) { + this._appendColor(functionText, options); + } else { + this._appendTextNode(functionText); + } + } + break; + } + + case "ident": + if (options.expectCubicBezier && + BEZIER_KEYWORDS.indexOf(token.text) >= 0) { + this._appendCubicBezier(token.text, options); + } else if (options.supportsColor && + DOMUtils.isValidCSSColor(token.text)) { + this._appendColor(token.text, options); + } else { + this._appendTextNode(text.substring(token.startOffset, + token.endOffset)); + } + break; + + case "id": + case "hash": { + let original = text.substring(token.startOffset, token.endOffset); + if (options.supportsColor && DOMUtils.isValidCSSColor(original)) { + this._appendColor(original, options); + } else { + this._appendTextNode(original); + } + break; + } + + case "url": + case "bad_url": + this._appendURL(text.substring(token.startOffset, token.endOffset), + token.text, options); + break; + + default: + this._appendTextNode(text.substring(token.startOffset, + token.endOffset)); + break; + } } } return this._toDOM(); }, - /** - * Convenience function to make the parser a little more readable. - * - * @param {String} text - * Main text - * @param {String} match - * Text to remove from the beginning - * - * @return {String} - * The string passed as 'text' with 'match' stripped from the start. - */ - _trimMatchFromStart: function(text, match) { - return text.substr(match.length); - }, - - /** - * Check if there is a color match and append it if it is valid. - * - * @param {String} text - * Main text - * @param {Object} options - * Options object. For valid options and default values see - * _mergeOptions(). - * - * @return {Array} - * An array containing the remaining text and a dirty flag. This array - * is designed for deconstruction using [text, dirty]. - */ - _appendColorOnMatch: function(text, options) { - let dirty; - let matched = text.match(REGEX_ALL_COLORS); - - if (matched) { - let match = matched[0]; - if (this._appendColor(match, options)) { - text = this._trimMatchFromStart(text, match); - dirty = true; - } - } else { - dirty = false; - } - - return [text, dirty]; - }, - /** * Append a cubic-bezier timing function value to the output * From dfc7a247b346291885aba3ce3068b988ec461a8b Mon Sep 17 00:00:00 2001 From: Sami Jaktholm Date: Thu, 14 May 2015 14:21:53 +0300 Subject: [PATCH 13/33] Bug 1158634 - Modify font-inspector tests for better code sharing. r=pbrosset This patch adds a new method openFontInspectorForURL to the head.js file which a) opens a new tab with the given URL b) opens inspector for the tab c) selects font-inspector and waits for it to initialize Previously the only font-inspector test was doing all that inside the test but as new tests require the similar functionality it's better to reside in head.js. --HG-- extra : rebase_source : 96d31183bdbd0131beadc9540ad2b7e2e4f9a67b extra : histedit_source : 3a9a9cb62e9b480e6f6e349570f3cbf23543487f --- .../test/browser_fontinspector.js | 48 +++++-------------- browser/devtools/fontinspector/test/head.js | 48 +++++++++++++++++++ 2 files changed, 60 insertions(+), 36 deletions(-) diff --git a/browser/devtools/fontinspector/test/browser_fontinspector.js b/browser/devtools/fontinspector/test/browser_fontinspector.js index 978ba3f3dc02..3f90fad59ebb 100644 --- a/browser/devtools/fontinspector/test/browser_fontinspector.js +++ b/browser/devtools/fontinspector/test/browser_fontinspector.js @@ -1,17 +1,9 @@ +/* vim: set ts=2 et sw=2 tw=80: */ /* Any copyright is dedicated to the Public Domain. http://creativecommons.org/publicdomain/zero/1.0/ */ +"use strict"; -let tempScope = {}; -let {gDevTools} = Cu.import("resource:///modules/devtools/gDevTools.jsm", {}); -let {devtools} = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}); -let TargetFactory = devtools.TargetFactory; - -let TEST_URI = "http://mochi.test:8888/browser/browser/devtools/fontinspector/test/browser_fontinspector.html"; - -let view, viewDoc; - -const BASE_URI = "http://mochi.test:8888/browser/browser/devtools/fontinspector/test/" - +const TEST_URI = BASE_URI + "browser_fontinspector.html"; const FONTS = [ {name: "Ostrich Sans Medium", remote: true, url: BASE_URI + "ostrich-regular.ttf", format: "truetype", cssName: "bar"}, @@ -24,33 +16,17 @@ const FONTS = [ ]; add_task(function*() { - yield loadTab(TEST_URI); - let {toolbox, inspector} = yield openInspector(); + let { inspector, fontInspector } = yield openFontInspectorForURL(TEST_URI); + ok(!!fontInspector, "Font inspector document is alive."); - info("Selecting the test node"); - yield selectNode("body", inspector); + let viewDoc = fontInspector.chromeDoc; - let updated = inspector.once("fontinspector-updated"); - inspector.sidebar.select("fontinspector"); - yield updated; - - info("Font Inspector ready"); - - view = inspector.sidebar.getWindowForTab("fontinspector"); - viewDoc = view.document; - - ok(!!view.fontInspector, "Font inspector document is alive."); - - yield testBodyFonts(inspector); - - yield testDivFonts(inspector); - - yield testShowAllFonts(inspector); - - view = viewDoc = null; + yield testBodyFonts(inspector, viewDoc); + yield testDivFonts(inspector, viewDoc); + yield testShowAllFonts(inspector, viewDoc); }); -function* testBodyFonts(inspector) { +function* testBodyFonts(inspector, viewDoc) { let s = viewDoc.querySelectorAll("#all-fonts > section"); is(s.length, 5, "Found 5 fonts"); @@ -89,7 +65,7 @@ function* testBodyFonts(inspector) { "Arial", "local font has right css name"); } -function* testDivFonts(inspector) { +function* testDivFonts(inspector, viewDoc) { let updated = inspector.once("fontinspector-updated"); yield selectNode("div", inspector); yield updated; @@ -100,7 +76,7 @@ function* testDivFonts(inspector) { "The DIV font has the right name"); } -function* testShowAllFonts(inspector) { +function* testShowAllFonts(inspector, viewDoc) { info("testing showing all fonts"); let updated = inspector.once("fontinspector-updated"); diff --git a/browser/devtools/fontinspector/test/head.js b/browser/devtools/fontinspector/test/head.js index b89dfbd9bf74..1106adab7be9 100644 --- a/browser/devtools/fontinspector/test/head.js +++ b/browser/devtools/fontinspector/test/head.js @@ -13,6 +13,8 @@ const { Promise: promise } = Cu.import("resource://gre/modules/Promise.jsm", {}) let {devtools} = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}); let TargetFactory = devtools.TargetFactory; +const BASE_URI = "http://mochi.test:8888/browser/browser/devtools/fontinspector/test/" + // All test are asynchronous waitForExplicitFinish(); @@ -98,6 +100,52 @@ let openInspector = Task.async(function*(cb) { } }); +/** + * Adds a new tab with the given URL, opens the inspector and selects the + * font-inspector tab. + * + * @return Object + * { + * toolbox, + * inspector, + * fontInspector + * } + */ +let openFontInspectorForURL = Task.async(function* (url) { + info("Opening tab " + url); + yield loadTab(url); + + let { toolbox, inspector } = yield openInspector(); + + /** + * Call selectNode to trigger font-inspector update so that we don't timeout + * if following conditions hold + * a) the initial 'fontinspector-updated' was emitted while we were waiting + * for openInspector to resolve + * b) the font-inspector tab was selected by default which means the call to + * select will not trigger another update. + * + * selectNode calls setNodeFront which always emits 'new-node' which calls + * FontInspector.update that emits the 'fontinspector-updated' event. + */ + let updated = inspector.once("fontinspector-updated"); + + yield selectNode("body", inspector); + inspector.sidebar.select("fontinspector"); + + info("Waiting for font-inspector to update."); + yield updated; + + info("Font Inspector ready."); + + let { fontInspector } = inspector.sidebar.getWindowForTab("fontinspector"); + return { + fontInspector, + inspector, + toolbox + }; +}); + /** * Select a node in the inspector given its selector. */ From 7fda4770c9002db4bf74cb6fabf8bd30b69988c5 Mon Sep 17 00:00:00 2001 From: Patrick Brosset Date: Thu, 23 Apr 2015 14:15:52 +0200 Subject: [PATCH 14/33] Bug 1156754 - Introduce a pref for the new animation inspector UI. r=jwalker --HG-- extra : rebase_source : 8f12113e8ffd63419c4d4be8ac07a14a2ed83490 extra : histedit_source : 9685d304e0cc8a42a20a20951e07acf7ae6cd70f --- browser/app/profile/firefox.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/browser/app/profile/firefox.js b/browser/app/profile/firefox.js index 4ef0771abdb8..55194c01e82d 100644 --- a/browser/app/profile/firefox.js +++ b/browser/app/profile/firefox.js @@ -1403,6 +1403,8 @@ pref("devtools.inspector.showUserAgentStyles", false); pref("devtools.inspector.showAllAnonymousContent", false); // Enable the MDN docs tooltip pref("devtools.inspector.mdnDocsTooltip.enabled", true); +// Show the new animation inspector UI +pref("devtools.inspector.animationInspectorV3", false); // DevTools default color unit pref("devtools.defaultColorUnit", "hex"); From c647ff437faee78b9676dfbedc7cb81389f97a03 Mon Sep 17 00:00:00 2001 From: Bevis Tseng Date: Fri, 8 May 2015 10:57:16 +0800 Subject: [PATCH 15/33] Bug 1162464 - Part 1: IDL Changes. r=echen --- dom/icc/interfaces/nsIIccProvider.idl | 69 +------------------ dom/icc/interfaces/nsIIccService.idl | 97 ++++++++++++++++++++++++++- 2 files changed, 97 insertions(+), 69 deletions(-) diff --git a/dom/icc/interfaces/nsIIccProvider.idl b/dom/icc/interfaces/nsIIccProvider.idl index ec3d01ef0933..d3c68b582668 100644 --- a/dom/icc/interfaces/nsIIccProvider.idl +++ b/dom/icc/interfaces/nsIIccProvider.idl @@ -6,51 +6,12 @@ interface nsIDOMDOMRequest; interface nsIDOMWindow; -interface nsIIccInfo; interface nsIIccListener; -[scriptable, uuid(6136acab-b50e-494a-a86d-df392a032897)] -interface nsIIccChannelCallback : nsISupports -{ - /** - * Callback function to notify on successfully opening a logical channel. - * - * @param channel - * The Channel Number/Handle that is successfully opened. - */ - void notifyOpenChannelSuccess(in long channel); - - /** - * Callback function to notify on successfully closing the logical channel. - * - */ - void notifyCloseChannelSuccess(); - - /** - * Callback function to notify the status of 'iccExchangeAPDU' command. - * - * @param sw1 - * Response's First Status Byte - * @param sw2 - * Response's Second Status Byte - * @param data - * Response's data - */ - void notifyExchangeAPDUResponse(in octet sw1, - in octet sw2, - in DOMString data); - - /** - * Callback function to notify error - * - */ - void notifyError(in DOMString error); -}; - /** * XPCOM component (in the content process) that provides the ICC information. */ -[scriptable, uuid(7dd6e186-b007-11e4-9b7e-7717d7863cb8)] +[scriptable, uuid(2fbacfc4-f52d-11e4-9667-33b72f279d14)] interface nsIIccProvider : nsISupports { /** @@ -91,32 +52,4 @@ interface nsIIccProvider : nsISupports in unsigned long contactType, in jsval contact, in DOMString pin2); - - /** - * Secure Card Icc communication channel - */ - void iccOpenChannel(in unsigned long clientId, - in DOMString aid, - in nsIIccChannelCallback callback); - - /** - * Exchange Command APDU (C-APDU) with SIM on the given logical channel. - * Note that 'P3' parameter could be Le/Lc depending on command APDU case. - * For Case 1 scenario (when only command header is present), the value - * of 'P3' should be set to '-1' explicitly. - * Refer to 3G TS 31.101 , 10.2 'Command APDU Structure' for all the cases. - */ - void iccExchangeAPDU(in unsigned long clientId, - in long channel, - in octet cla, - in octet ins, - in octet p1, - in octet p2, - in short p3, - in DOMString data, - in nsIIccChannelCallback callback); - - void iccCloseChannel(in unsigned long clientId, - in long channel, - in nsIIccChannelCallback callback); }; diff --git a/dom/icc/interfaces/nsIIccService.idl b/dom/icc/interfaces/nsIIccService.idl index 15ae8a987f3c..57b5cb1e0a93 100644 --- a/dom/icc/interfaces/nsIIccService.idl +++ b/dom/icc/interfaces/nsIIccService.idl @@ -63,6 +63,44 @@ interface nsIIccCallback : nsISupports void notifyCardLockError(in DOMString aErrorMsg, in long aRetryCount); }; +[scriptable, uuid(6136acab-b50e-494a-a86d-df392a032897)] +interface nsIIccChannelCallback : nsISupports +{ + /** + * Callback function to notify on successfully opening a logical channel. + * + * @param channel + * The Channel Number/Handle that is successfully opened. + */ + void notifyOpenChannelSuccess(in long channel); + + /** + * Callback function to notify on successfully closing the logical channel. + * + */ + void notifyCloseChannelSuccess(); + + /** + * Callback function to notify the status of 'iccExchangeAPDU' command. + * + * @param sw1 + * Response's First Status Byte + * @param sw2 + * Response's Second Status Byte + * @param data + * Response's data + */ + void notifyExchangeAPDUResponse(in octet sw1, + in octet sw2, + in DOMString data); + + /** + * Callback function to notify error + * + */ + void notifyError(in DOMString error); +}; + %{C++ #define ICC_SERVICE_CID \ { 0xbab0277a, 0x900e, 0x11e4, { 0x80, 0xc7, 0xdb, 0xd7, 0xad, 0x05, 0x24, 0x01 } } @@ -97,7 +135,7 @@ NS_CreateIccService(); /** * XPCOM component that provides the access to the selected ICC. */ -[scriptable, uuid(20a99186-e4cb-11e4-a5f9-938abcf7c826)] +[scriptable, uuid(6ad6b686-f52d-11e4-942d-db2884bd9242)] interface nsIIcc : nsISupports { /** @@ -331,4 +369,61 @@ interface nsIIcc : nsISupports */ void getServiceStateEnabled(in unsigned long aService, in nsIIccCallback aCallback); + + /** + * Open Secure Card Icc communication channel + * + * @param aAid + * Card Application Id in this UICC. + * @param aCallback + * An instance of nsIIccChannelCallback. + * nsIIccChannelCallback::notifyOpenChannelSuccess() if success. + * nsIIccChannelCallback::notifyError(), otherwise. + */ + void iccOpenChannel(in DOMString aAid, + in nsIIccChannelCallback aCallback); + + /** + * Exchange Command APDU (C-APDU) with UICC on the given logical channel. + * Note that 'P3' parameter could be Le/Lc depending on command APDU case. + * For Case 1 scenario (when only command header is present), the value + * of 'P3' should be set to '-1' explicitly. + * Refer to 3G TS 31.101 , 10.2 'Command APDU Structure' for all the cases. + * + * @param aChannel + * given logical channel. + * @param aCla + * APDU class. + * @param aIns + * Instruction code. + * @param aP1, aP2, aP3 + * P1, P2, P3 parameters in APDU. + * @param aData + * The hex data to be sent by this PDU. + * @param aCallback + * An instance of nsIIccChannelCallback. + * nsIIccChannelCallback::notifyExchangeAPDUResponse() if success. + * nsIIccChannelCallback::notifyError(), otherwise. + */ + void iccExchangeAPDU(in long aChannel, + in octet aCla, + in octet aIns, + in octet aP1, + in octet aP2, + in short aP3, + in DOMString aData, + in nsIIccChannelCallback aCallback); + + /** + * Close Secure Card Icc communication channel + * + * @param aChannel + * Channel to be closed. + * @param aCallback + * An instance of nsIIccChannelCallback. + * nsIIccChannelCallback::notifyCloseChannelSuccess() if success. + * nsIIccChannelCallback::notifyError(), otherwise. + */ + void iccCloseChannel(in long aChannel, + in nsIIccChannelCallback aCallback); }; From 364c86e99a98706c88c6ae69a5bbe03e897e2260 Mon Sep 17 00:00:00 2001 From: Bevis Tseng Date: Fri, 8 May 2015 11:23:09 +0800 Subject: [PATCH 16/33] Bug 1162464 - Part 2: Implementation Changes. r=echen --- dom/icc/gonk/IccService.js | 54 +++++++++++ dom/icc/ipc/IccChild.cpp | 20 ++++ dom/system/gonk/RILContentHelper.js | 127 ------------------------- dom/system/gonk/RadioInterfaceLayer.js | 12 --- 4 files changed, 74 insertions(+), 139 deletions(-) diff --git a/dom/icc/gonk/IccService.js b/dom/icc/gonk/IccService.js index dac766dba142..ffd581bba174 100644 --- a/dom/icc/gonk/IccService.js +++ b/dom/icc/gonk/IccService.js @@ -482,6 +482,60 @@ Icc.prototype = { aCallback.notifySuccessWithBoolean(aResponse.result); }); + }, + + iccOpenChannel: function(aAid, aCallback) { + this._radioInterface.sendWorkerMessage("iccOpenChannel", + { aid: aAid }, + (aResponse) => { + if (aResponse.errorMsg) { + aCallback.notifyError(aResponse.errorMsg); + return; + } + + aCallback.notifyOpenChannelSuccess(aResponse.channel); + }); + }, + + iccExchangeAPDU: function(aChannel, aCla, aIns, aP1, aP2, aP3, aData, aCallback) { + if (!aData) { + if (DEBUG) debug('data is not set , aP3 : ' + aP3); + } + + let apdu = { + cla: aCla, + command: aIns, + p1: aP1, + p2: aP2, + p3: aP3, + data: aData + }; + + this._radioInterface.sendWorkerMessage("iccExchangeAPDU", + { channel: aChannel, apdu: apdu }, + (aResponse) => { + if (aResponse.errorMsg) { + aCallback.notifyError(aResponse.errorMsg); + return; + } + + aCallback.notifyExchangeAPDUResponse(aResponse.sw1, + aResponse.sw2, + aResponse.simResponse); + }); + }, + + iccCloseChannel: function(aChannel, aCallback) { + this._radioInterface.sendWorkerMessage("iccCloseChannel", + { channel: aChannel }, + (aResponse) => { + if (aResponse.errorMsg) { + aCallback.notifyError(aResponse.errorMsg); + return; + } + + aCallback.notifyCloseChannelSuccess(); + }); } }; diff --git a/dom/icc/ipc/IccChild.cpp b/dom/icc/ipc/IccChild.cpp index 7fbfb61af104..72f784ff4eb7 100644 --- a/dom/icc/ipc/IccChild.cpp +++ b/dom/icc/ipc/IccChild.cpp @@ -265,6 +265,26 @@ IccChild::GetServiceStateEnabled(uint32_t aService, ? NS_OK : NS_ERROR_FAILURE; } +NS_IMETHODIMP +IccChild::IccOpenChannel(const nsAString& aAid, nsIIccChannelCallback* aCallback) +{ + return NS_ERROR_NOT_IMPLEMENTED; +} + +NS_IMETHODIMP +IccChild::IccExchangeAPDU(int32_t aChannel, uint8_t aCla, uint8_t aIns, uint8_t aP1, + uint8_t aP2, int16_t aP3, const nsAString & aData, + nsIIccChannelCallback* aCallback) +{ + return NS_ERROR_NOT_IMPLEMENTED; +} + +NS_IMETHODIMP +IccChild::IccCloseChannel(int32_t aChannel, nsIIccChannelCallback* aCallback) +{ + return NS_ERROR_NOT_IMPLEMENTED; +} + /** * PIccRequestChild Implementation. */ diff --git a/dom/system/gonk/RILContentHelper.js b/dom/system/gonk/RILContentHelper.js index e93f8ee3995f..b27f9f438e4e 100644 --- a/dom/system/gonk/RILContentHelper.js +++ b/dom/system/gonk/RILContentHelper.js @@ -46,9 +46,6 @@ const RILCONTENTHELPER_CID = const RIL_IPC_MSG_NAMES = [ "RIL:StkCommand", "RIL:StkSessionEnd", - "RIL:IccOpenChannel", - "RIL:IccCloseChannel", - "RIL:IccExchangeAPDU", "RIL:ReadIccContacts", "RIL:UpdateIccContact", ]; @@ -89,7 +86,6 @@ function RILContentHelper() { this.initDOMRequestHelper(/* aWindow */ null, RIL_IPC_MSG_NAMES); this._windowsMap = []; this._iccListeners = []; - this._iccChannelCallback = []; Services.obs.addObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false); @@ -173,60 +169,6 @@ RILContentHelper.prototype = { }); }, - iccOpenChannel: function(clientId, aid, callback) { - let requestId = UUIDGenerator.generateUUID().toString(); - this._addIccChannelCallback(requestId, callback); - - cpmm.sendAsyncMessage("RIL:IccOpenChannel", { - clientId: clientId, - data: { - requestId: requestId, - aid: aid - } - }); - }, - - iccExchangeAPDU: function(clientId, channel, cla, ins, p1, p2, p3, data, callback) { - let requestId = UUIDGenerator.generateUUID().toString(); - this._addIccChannelCallback(requestId, callback); - - if (!data) { - if (DEBUG) debug('data is not set , p3 : ' + p3); - } - - let apdu = { - cla: cla, - command: ins, - p1: p1, - p2: p2, - p3: p3, - data: data - }; - - //Potentially you need serialization here and can't pass the jsval through - cpmm.sendAsyncMessage("RIL:IccExchangeAPDU", { - clientId: clientId, - data: { - requestId: requestId, - channel: channel, - apdu: apdu - } - }); - }, - - iccCloseChannel: function(clientId, channel, callback) { - let requestId = UUIDGenerator.generateUUID().toString(); - this._addIccChannelCallback(requestId, callback); - - cpmm.sendAsyncMessage("RIL:IccCloseChannel", { - clientId: clientId, - data: { - requestId: requestId, - channel: channel - } - }); - }, - readContacts: function(clientId, window, contactType) { if (window == null) { throw Components.Exception("Can't get window object", @@ -330,24 +272,6 @@ RILContentHelper.prototype = { } }, - _iccChannelCallback: null, - - _addIccChannelCallback: function(requestId, channelCb) { - let cbInterfaces = this._iccChannelCallback; - if (!cbInterfaces[requestId] && channelCb) { - cbInterfaces[requestId] = channelCb; - return; - } - - if (DEBUG) debug("Unable to add channelCbInterface for requestId : " + requestId); - }, - - _getIccChannelCallback: function(requestId) { - let cb = this._iccChannelCallback[requestId]; - delete this._iccChannelCallback[requestId]; - return cb; - }, - registerIccMsg: function(clientId, listener) { if (DEBUG) debug("Registering for ICC related messages"); this.registerListener("_iccListeners", clientId, listener); @@ -454,15 +378,6 @@ RILContentHelper.prototype = { case "RIL:StkSessionEnd": this._deliverEvent(clientId, "_iccListeners", "notifyStkSessionEnd", null); break; - case "RIL:IccOpenChannel": - this.handleIccOpenChannel(data); - break; - case "RIL:IccCloseChannel": - this.handleIccCloseChannel(data); - break; - case "RIL:IccExchangeAPDU": - this.handleIccExchangeAPDU(data); - break; case "RIL:ReadIccContacts": this.handleReadIccContacts(data); break; @@ -472,48 +387,6 @@ RILContentHelper.prototype = { } }, - handleSimpleRequest: function(requestId, errorMsg, result) { - if (errorMsg) { - this.fireRequestError(requestId, errorMsg); - } else { - this.fireRequestSuccess(requestId, result); - } - }, - - handleIccOpenChannel: function(message) { - let requestId = message.requestId; - let callback = this._getIccChannelCallback(requestId); - if (!callback) { - return; - } - - return !message.errorMsg ? callback.notifyOpenChannelSuccess(message.channel) : - callback.notifyError(message.errorMsg); - }, - - handleIccCloseChannel: function(message) { - let requestId = message.requestId; - let callback = this._getIccChannelCallback(requestId); - if (!callback) { - return; - } - - return !message.errorMsg ? callback.notifyCloseChannelSuccess() : - callback.notifyError(message.errorMsg); - }, - - handleIccExchangeAPDU: function(message) { - let requestId = message.requestId; - let callback = this._getIccChannelCallback(requestId); - if (!callback) { - return; - } - - return !message.errorMsg ? - callback.notifyExchangeAPDUResponse(message.sw1, message.sw2, message.simResponse) : - callback.notifyError(message.errorMsg); - }, - handleReadIccContacts: function(message) { if (message.errorMsg) { this.fireRequestError(message.requestId, message.errorMsg); diff --git a/dom/system/gonk/RadioInterfaceLayer.js b/dom/system/gonk/RadioInterfaceLayer.js index fe75de5b3ce3..360e257d2e22 100644 --- a/dom/system/gonk/RadioInterfaceLayer.js +++ b/dom/system/gonk/RadioInterfaceLayer.js @@ -93,9 +93,6 @@ const RIL_IPC_ICCMANAGER_MSG_NAMES = [ "RIL:SendStkMenuSelection", "RIL:SendStkTimerExpiration", "RIL:SendStkEventDownload", - "RIL:IccOpenChannel", - "RIL:IccExchangeAPDU", - "RIL:IccCloseChannel", "RIL:ReadIccContacts", "RIL:UpdateIccContact", "RIL:RegisterIccMsg", @@ -1732,15 +1729,6 @@ RadioInterface.prototype = { case "RIL:SendStkEventDownload": this.workerMessenger.send("sendStkEventDownload", msg.json.data); break; - case "RIL:IccOpenChannel": - this.workerMessenger.sendWithIPCMessage(msg, "iccOpenChannel"); - break; - case "RIL:IccCloseChannel": - this.workerMessenger.sendWithIPCMessage(msg, "iccCloseChannel"); - break; - case "RIL:IccExchangeAPDU": - this.workerMessenger.sendWithIPCMessage(msg, "iccExchangeAPDU"); - break; case "RIL:ReadIccContacts": this.workerMessenger.sendWithIPCMessage(msg, "readICCContacts"); break; From 6b0a6cd597550320156cf9be91ac95efc0714a6b Mon Sep 17 00:00:00 2001 From: Bevis Tseng Date: Fri, 8 May 2015 11:31:11 +0800 Subject: [PATCH 17/33] Bug 1162464 - Part 3: Refactor UiccConnector due to interface change. r=allstars.chh --- dom/secureelement/gonk/UiccConnector.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/dom/secureelement/gonk/UiccConnector.js b/dom/secureelement/gonk/UiccConnector.js index c966900e95ba..26a1958d7ec1 100644 --- a/dom/secureelement/gonk/UiccConnector.js +++ b/dom/secureelement/gonk/UiccConnector.js @@ -18,7 +18,7 @@ "use strict"; /* globals Components, XPCOMUtils, SE, dump, libcutils, Services, - iccProvider, iccService, SEUtils */ + iccService, SEUtils */ const { interfaces: Ci, utils: Cu, results: Cr } = Components; @@ -43,10 +43,6 @@ function debug(s) { XPCOMUtils.defineLazyModuleGetter(this, "SEUtils", "resource://gre/modules/SEUtils.jsm"); -XPCOMUtils.defineLazyServiceGetter(this, "iccProvider", - "@mozilla.org/ril/content-helper;1", - "nsIIccProvider"); - XPCOMUtils.defineLazyServiceGetter(this, "iccService", "@mozilla.org/icc/iccservice;1", "nsIIccService"); @@ -65,7 +61,7 @@ const PREFERRED_UICC_CLIENTID = libcutils.property_get("ro.moz.se.def_client_id", "0"); /** - * 'UiccConnector' object is a wrapper over iccProvider's channel management + * 'UiccConnector' object is a wrapper over iccService's channel management * related interfaces that implements nsISecureElementConnector interface. */ function UiccConnector() { @@ -174,8 +170,8 @@ UiccConnector.prototype = { _doIccExchangeAPDU: function(channel, cla, ins, p1, p2, p3, data, appendResp, callback) { - iccProvider.iccExchangeAPDU(PREFERRED_UICC_CLIENTID, channel, cla & 0xFC, - ins, p1, p2, p3, data, { + let icc = iccService.getIccByServiceId(PREFERRED_UICC_CLIENTID); + icc.iccExchangeAPDU(channel, cla & 0xFC, ins, p1, p2, p3, data, { notifyExchangeAPDUResponse: (sw1, sw2, response) => { debug("sw1 : " + sw1 + ", sw2 : " + sw2 + ", response : " + response); @@ -242,7 +238,8 @@ UiccConnector.prototype = { // some erroneous conditions such as gecko restart /, crash it can read // the persistent storage to check if there are any held resources // (opened channels) and close them. - iccProvider.iccOpenChannel(PREFERRED_UICC_CLIENTID, aid, { + let icc = iccService.getIccByServiceId(PREFERRED_UICC_CLIENTID); + icc.iccOpenChannel(aid, { notifyOpenChannelSuccess: (channel) => { this._doGetOpenResponse(channel, 0x00, function(result) { if (callback) { @@ -299,7 +296,8 @@ UiccConnector.prototype = { return; } - iccProvider.iccCloseChannel(PREFERRED_UICC_CLIENTID, channel, { + let icc = iccService.getIccByServiceId(PREFERRED_UICC_CLIENTID); + icc.iccCloseChannel(channel, { notifyCloseChannelSuccess: function() { debug("closeChannel successfully closed the channel # : " + channel); if (callback) { From 2b2d37c5e8a6412523157afaadeb9b989713ed8c Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Sun, 17 May 2015 18:31:03 -0400 Subject: [PATCH 18/33] Backed out 2 changesets (bug 1149486) for static analysis bustage. Backed out changeset 99b209f7d085 (bug 1149486) Backed out changeset 9b60d38c552e (bug 1149486) --- js/src/jsapi.cpp | 41 +++- js/src/jsapi.h | 90 ++++---- js/src/jscompartment.cpp | 2 +- js/src/vm/Interpreter.cpp | 65 +++--- js/src/vm/Runtime.cpp | 29 +-- js/src/vm/Runtime.h | 13 -- js/xpconnect/src/XPCJSRuntime.cpp | 57 ----- js/xpconnect/src/xpcpublic.h | 9 +- .../content/aboutPerformance.js | 2 +- .../tests/browser/browser.ini | 8 +- .../tests/browser/browser_aboutperformance.js | 24 +- .../tests/browser/browser_compartments.html | 32 +-- .../browser/browser_compartments_frame.html | 12 - .../browser/browser_compartments_script.js | 20 -- .../perfmonitoring/PerformanceStats.jsm | 12 +- toolkit/components/perfmonitoring/moz.build | 5 - .../perfmonitoring/nsIPerformanceStats.idl | 19 +- .../perfmonitoring/nsPerformanceStats.cpp | 212 +++--------------- .../perfmonitoring/tests/browser/browser.ini | 2 - .../tests/browser/browser_compartments.html | 27 ++- .../tests/browser/browser_compartments.js | 76 ++----- .../browser/browser_compartments_frame.html | 12 - .../browser/browser_compartments_script.js | 20 -- 23 files changed, 233 insertions(+), 556 deletions(-) delete mode 100644 toolkit/components/aboutperformance/tests/browser/browser_compartments_frame.html delete mode 100644 toolkit/components/aboutperformance/tests/browser/browser_compartments_script.js delete mode 100644 toolkit/components/perfmonitoring/tests/browser/browser_compartments_frame.html delete mode 100644 toolkit/components/perfmonitoring/tests/browser/browser_compartments_script.js diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 8c0ac29ab4ed..343eec9ae640 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -274,10 +274,9 @@ JS_GetEmptyString(JSRuntime* rt) namespace js { JS_PUBLIC_API(bool) -IterPerformanceStats(JSContext* cx, - PerformanceStatsWalker walker, - PerformanceData* processStats, - void* closure) +GetPerformanceStats(JSRuntime* rt, + PerformanceStatsVector& stats, + PerformanceStats& processStats) { // As a PerformanceGroup is typically associated to several // compartments, use a HashSet to make sure that we only report @@ -290,16 +289,13 @@ IterPerformanceStats(JSContext* cx, return false; } - JSRuntime* rt = JS_GetRuntime(cx); for (CompartmentsIter c(rt, WithAtoms); !c.done(); c.next()) { JSCompartment* compartment = c.get(); if (!compartment->performanceMonitoring.isLinked()) { // Don't report compartments that do not even have a PerformanceGroup. continue; } - - js::AutoCompartment autoCompartment(cx, compartment); - PerformanceGroup* group = compartment->performanceMonitoring.getGroup(cx); + PerformanceGroup* group = compartment->performanceMonitoring.getGroup(); if (group->data.ticks == 0) { // Don't report compartments that have never been used. @@ -312,16 +308,38 @@ IterPerformanceStats(JSContext* cx, continue; } - if (!(*walker)(cx, group->data, closure)) { - // Issue in callback + if (!stats.growBy(1)) { + // Memory issue return false; } + PerformanceStats* stat = &stats.back(); + stat->isSystem = compartment->isSystem(); + if (compartment->addonId) + stat->addonId = compartment->addonId; + + if (compartment->addonId || !compartment->isSystem()) { + if (rt->compartmentNameCallback) { + (*rt->compartmentNameCallback)(rt, compartment, + stat->name, + mozilla::ArrayLength(stat->name)); + } else { + strcpy(stat->name, ""); + } + } else { + strcpy(stat->name, ""); + } + stat->performance = group->data; if (!set.add(ptr, group)) { // Memory issue return false; } } - *processStats = rt->stopwatch.performance; + + strcpy(processStats.name, ""); + processStats.addonId = nullptr; + processStats.isSystem = true; + processStats.performance = rt->stopwatch.performance; + return true; } @@ -911,7 +929,6 @@ JSAutoCompartment::JSAutoCompartment(JSContext* cx, JSScript* target cx_->enterCompartment(target->compartment()); } - JSAutoCompartment::~JSAutoCompartment() { cx_->leaveCompartment(oldCompartment_); diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 6bf2bbe74d05..9a3bc4793747 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -720,18 +720,6 @@ typedef void (* JSCompartmentNameCallback)(JSRuntime* rt, JSCompartment* compartment, char* buf, size_t bufsize); -/** - * Callback used to ask the embedding to determine in which - * Performance Group the current execution belongs. Typically, this is - * used to regroup JSCompartments from several iframes from the same - * page or from several compartments of the same addon into a single - * Performance Group. - * - * Returns an opaque key. - */ -typedef void* -(* JSCurrentPerfGroupCallback)(JSContext*); - /************************************************************************/ static MOZ_ALWAYS_INLINE jsval @@ -5444,10 +5432,9 @@ struct PerformanceGroup { stopwatch_ = nullptr; } - explicit PerformanceGroup(void* key) + PerformanceGroup() : stopwatch_(nullptr) , iteration_(0) - , key_(key) , refCount_(0) { } ~PerformanceGroup() @@ -5466,9 +5453,6 @@ struct PerformanceGroup { // may safely overflow. uint64_t iteration_; - // The hash key for this PerformanceGroup. - void* const key_; - // Increment/decrement the refcounter, return the updated value. uint64_t incRefCount() { MOZ_ASSERT(refCount_ + 1 > 0); @@ -5480,7 +5464,7 @@ struct PerformanceGroup { } friend struct PerformanceGroupHolder; -private: + private: // A reference counter. Maintained by PerformanceGroupHolder. uint64_t refCount_; }; @@ -5493,7 +5477,7 @@ struct PerformanceGroupHolder { // Get the group. // On first call, this causes a single Hashtable lookup. // Successive calls do not require further lookups. - js::PerformanceGroup* getGroup(JSContext*); + js::PerformanceGroup* getGroup(); // `true` if the this holder is currently associated to a // PerformanceGroup, `false` otherwise. Use this method to avoid @@ -5508,8 +5492,9 @@ struct PerformanceGroupHolder { // (new values of `isSystem()`, `principals()` or `addonId`). void unlink(); - PerformanceGroupHolder(JSRuntime* runtime) + PerformanceGroupHolder(JSRuntime* runtime, JSCompartment* compartment) : runtime_(runtime) + , compartment_(compartment) , group_(nullptr) { } ~PerformanceGroupHolder(); @@ -5517,9 +5502,10 @@ private: // Return the key representing this PerformanceGroup in // Runtime::Stopwatch. // Do not deallocate the key. - void* getHashKey(JSContext* cx); + void* getHashKey(); - JSRuntime *runtime_; + JSRuntime* runtime_; + JSCompartment* compartment_; // The PerformanceGroup held by this object. // Initially set to `nullptr` until the first cal to `getGroup`. @@ -5552,30 +5538,56 @@ IsStopwatchActive(JSRuntime*); extern JS_PUBLIC_API(PerformanceData*) GetPerformanceData(JSRuntime*); -typedef bool -(PerformanceStatsWalker)(JSContext* cx, const PerformanceData& stats, void* closure); - /** + * Performance statistics for a performance group (a process, an + * add-on, a webpage, the built-ins or a special compartment). + */ +struct PerformanceStats { + /** + * If this group represents an add-on, the ID of the addon, + * otherwise `nullptr`. + */ + JSAddonId* addonId; + + /** + * If this group represents a webpage, the process itself or a special + * compartment, a human-readable name. Unspecified for add-ons. + */ + char name[1024]; + + /** + * `true` if the group represents in system compartments, `false` + * otherwise. A group may never contain both system and non-system + * compartments. + */ + bool isSystem; + + /** + * Performance information. + */ + js::PerformanceData performance; + + PerformanceStats() + : addonId(nullptr) + , isSystem(false) + { + name[0] = '\0'; + } +}; + +typedef js::Vector PerformanceStatsVector; + + /** * Extract the performance statistics. * - * Note that before calling `walker`, we enter the corresponding context. + * After a successful call, `stats` holds the `PerformanceStats` for + * all performance groups, and `global` holds a `PerformanceStats` + * representing the entire process. */ extern JS_PUBLIC_API(bool) -IterPerformanceStats(JSContext* cx, PerformanceStatsWalker* walker, js::PerformanceData* process, void* closure); +GetPerformanceStats(JSRuntime* rt, js::PerformanceStatsVector& stats, js::PerformanceStats& global); } /* namespace js */ -/** - * Callback used to ask the embedding to determine in which - * Performance Group a compartment belongs. Typically, this is used to - * regroup JSCompartments from several iframes from the same page or - * from several compartments of the same addon into a single - * Performance Group. - * - * Returns an opaque key. - */ -extern JS_PUBLIC_API(void) -JS_SetCurrentPerfGroupCallback(JSRuntime *rt, JSCurrentPerfGroupCallback cb); - #endif /* jsapi_h */ diff --git a/js/src/jscompartment.cpp b/js/src/jscompartment.cpp index bd28e1f587ef..d8a44951d394 100644 --- a/js/src/jscompartment.cpp +++ b/js/src/jscompartment.cpp @@ -53,7 +53,7 @@ JSCompartment::JSCompartment(Zone* zone, const JS::CompartmentOptions& options = #endif global_(nullptr), enterCompartmentDepth(0), - performanceMonitoring(runtime_), + performanceMonitoring(runtime_, this), data(nullptr), objectMetadataCallback(nullptr), lastAnimationTime(0), diff --git a/js/src/vm/Interpreter.cpp b/js/src/vm/Interpreter.cpp index 3b4ba3247de6..fa59f3bd47a2 100644 --- a/js/src/vm/Interpreter.cpp +++ b/js/src/vm/Interpreter.cpp @@ -409,7 +409,8 @@ struct AutoStopwatch final // // Previous owner is restored upon destruction. explicit inline AutoStopwatch(JSContext* cx MOZ_GUARD_OBJECT_NOTIFIER_PARAM) - : cx_(cx) + : compartment_(nullptr) + , runtime_(nullptr) , iteration_(0) , isActive_(false) , isTop_(false) @@ -418,17 +419,16 @@ struct AutoStopwatch final , CPOWTimeStart_(0) { MOZ_GUARD_OBJECT_NOTIFIER_INIT; - - JSRuntime* runtime = JS_GetRuntime(cx_); - if (!runtime->stopwatch.isActive()) + runtime_ = cx->runtime(); + if (!runtime_->stopwatch.isActive()) return; - - JSCompartment* compartment = cx_->compartment(); - if (compartment->scheduledForDestruction) + compartment_ = cx->compartment(); + MOZ_ASSERT(compartment_); + if (compartment_->scheduledForDestruction) return; - iteration_ = runtime->stopwatch.iteration; + iteration_ = runtime_->stopwatch.iteration; - PerformanceGroup *group = compartment->performanceMonitoring.getGroup(cx); + PerformanceGroup* group = compartment_->performanceMonitoring.getGroup(); MOZ_ASSERT(group); if (group->hasStopwatch(iteration_)) { @@ -438,22 +438,22 @@ struct AutoStopwatch final } // Start the stopwatch. - if (!this->getTimes(runtime, &userTimeStart_, &systemTimeStart_)) + if (!this->getTimes(&userTimeStart_, &systemTimeStart_)) return; isActive_ = true; - CPOWTimeStart_ = runtime->stopwatch.performance.totalCPOWTime; + CPOWTimeStart_ = runtime_->stopwatch.performance.totalCPOWTime; // We are now in charge of monitoring this group for the tick, // until destruction of `this` or until we enter a nested event // loop and `iteration_` is incremented. group->acquireStopwatch(iteration_, this); - if (runtime->stopwatch.isEmpty) { + if (runtime_->stopwatch.isEmpty) { // This is the topmost stopwatch on the stack. // It will be in charge of updating the per-process // performance data. - runtime->stopwatch.isEmpty = false; - runtime->stopwatch.performance.ticks++; + runtime_->stopwatch.isEmpty = false; + runtime_->stopwatch.performance.ticks++; isTop_ = true; } } @@ -463,35 +463,32 @@ struct AutoStopwatch final return; } - JSRuntime* runtime = JS_GetRuntime(cx_); - JSCompartment* compartment = cx_->compartment(); + MOZ_ASSERT(!compartment_->scheduledForDestruction); - MOZ_ASSERT(!compartment->scheduledForDestruction); - - if (!runtime->stopwatch.isActive()) { + if (!runtime_->stopwatch.isActive()) { // Monitoring has been stopped while we were // executing the code. Drop everything. return; } - if (iteration_ != runtime->stopwatch.iteration) { + if (iteration_ != runtime_->stopwatch.iteration) { // We have entered a nested event loop at some point. // Any information we may have is obsolete. return; } - PerformanceGroup *group = compartment->performanceMonitoring.getGroup(cx_); + PerformanceGroup* group = compartment_->performanceMonitoring.getGroup(); MOZ_ASSERT(group); // Compute time spent. group->releaseStopwatch(iteration_, this); uint64_t userTimeEnd, systemTimeEnd; - if (!this->getTimes(runtime, &userTimeEnd, &systemTimeEnd)) + if (!this->getTimes(&userTimeEnd, &systemTimeEnd)) return; uint64_t userTimeDelta = userTimeEnd - userTimeStart_; uint64_t systemTimeDelta = systemTimeEnd - systemTimeStart_; - uint64_t CPOWTimeDelta = runtime->stopwatch.performance.totalCPOWTime - CPOWTimeStart_; + uint64_t CPOWTimeDelta = runtime_->stopwatch.performance.totalCPOWTime - CPOWTimeStart_; group->data.totalUserTime += userTimeDelta; group->data.totalSystemTime += systemTimeDelta; group->data.totalCPOWTime += CPOWTimeDelta; @@ -503,10 +500,10 @@ struct AutoStopwatch final if (isTop_) { // This is the topmost stopwatch on the stack. // Record the timing information. - runtime->stopwatch.performance.totalUserTime = userTimeEnd; - runtime->stopwatch.performance.totalSystemTime = systemTimeEnd; - updateDurations(totalTimeDelta, runtime->stopwatch.performance.durations); - runtime->stopwatch.isEmpty = true; + runtime_->stopwatch.performance.totalUserTime = userTimeEnd; + runtime_->stopwatch.performance.totalSystemTime = systemTimeEnd; + updateDurations(totalTimeDelta, runtime_->stopwatch.performance.durations); + runtime_->stopwatch.isEmpty = true; } } @@ -530,7 +527,7 @@ struct AutoStopwatch final // Get the OS-reported time spent in userland/systemland, in // microseconds. On most platforms, this data is per-thread, // but on some platforms we need to fall back to per-process. - bool getTimes(JSRuntime* runtime, uint64_t* userTime, uint64_t* systemTime) const { + bool getTimes(uint64_t* userTime, uint64_t* systemTime) const { MOZ_ASSERT(userTime); MOZ_ASSERT(systemTime); @@ -594,12 +591,12 @@ struct AutoStopwatch final kernelTimeInt.LowPart = kernelFileTime.dwLowDateTime; kernelTimeInt.HighPart = kernelFileTime.dwHighDateTime; // Convert 100 ns to 1 us, make sure that the result is monotonic - *systemTime = runtime->stopwatch.systemTimeFix.monotonize(kernelTimeInt.QuadPart / 10); + *systemTime = runtime_-> stopwatch.systemTimeFix.monotonize(kernelTimeInt.QuadPart / 10); userTimeInt.LowPart = userFileTime.dwLowDateTime; userTimeInt.HighPart = userFileTime.dwHighDateTime; // Convert 100 ns to 1 us, make sure that the result is monotonic - *userTime = runtime->stopwatch.userTimeFix.monotonize(userTimeInt.QuadPart / 10); + *userTime = runtime_-> stopwatch.userTimeFix.monotonize(userTimeInt.QuadPart / 10); #endif // defined(XP_MACOSX) || defined(XP_UNIX) || defined(XP_WIN) @@ -607,9 +604,13 @@ struct AutoStopwatch final } private: - // The context with which this object was initialized. + // The compartment with which this object was initialized. // Non-null. - JSContext* const cx_; + JSCompartment* compartment_; + + // The runtime with which this object was initialized. + // Non-null. + JSRuntime* runtime_; // An indication of the number of times we have entered the event // loop. Used only for comparison. diff --git a/js/src/vm/Runtime.cpp b/js/src/vm/Runtime.cpp index 9672c658cee9..47693c521580 100644 --- a/js/src/vm/Runtime.cpp +++ b/js/src/vm/Runtime.cpp @@ -897,14 +897,15 @@ js::PerformanceGroupHolder::~PerformanceGroupHolder() } void* -js::PerformanceGroupHolder::getHashKey(JSContext* cx) +js::PerformanceGroupHolder::getHashKey() { - if (runtime_->stopwatch.currentPerfGroupCallback) { - return (*runtime_->stopwatch.currentPerfGroupCallback)(cx); - } - - // As a fallback, put everything in the same PerformanceGroup. - return nullptr; + return compartment_->isSystem() ? + (void*)compartment_->addonId : + (void*)JS_GetCompartmentPrincipals(compartment_); + // This key may be `nullptr` if we have `isSystem() == true` + // and `compartment_->addonId`. This is absolutely correct, + // and this represents the `PerformanceGroup` used to track + // the performance of the the platform compartments. } void @@ -925,26 +926,26 @@ js::PerformanceGroupHolder::unlink() JSRuntime::Stopwatch::Groups::Ptr ptr = - runtime_->stopwatch.groups_.lookup(group->key_); + runtime_->stopwatch.groups_.lookup(getHashKey()); MOZ_ASSERT(ptr); runtime_->stopwatch.groups_.remove(ptr); js_delete(group); } PerformanceGroup* -js::PerformanceGroupHolder::getGroup(JSContext* cx) +js::PerformanceGroupHolder::getGroup() { if (group_) return group_; - void* key = getHashKey(cx); + void* key = getHashKey(); JSRuntime::Stopwatch::Groups::AddPtr ptr = runtime_->stopwatch.groups_.lookupForAdd(key); if (ptr) { group_ = ptr->value(); MOZ_ASSERT(group_); } else { - group_ = runtime_->new_(key); + group_ = runtime_->new_(); runtime_->stopwatch.groups_.add(ptr, key, group_); } @@ -958,9 +959,3 @@ js::GetPerformanceData(JSRuntime* rt) { return &rt->stopwatch.performance; } - -void -JS_SetCurrentPerfGroupCallback(JSRuntime *rt, JSCurrentPerfGroupCallback cb) -{ - rt->stopwatch.currentPerfGroupCallback = cb; -} diff --git a/js/src/vm/Runtime.h b/js/src/vm/Runtime.h index 4bf92911c4f2..66e1161983ed 100644 --- a/js/src/vm/Runtime.h +++ b/js/src/vm/Runtime.h @@ -1489,22 +1489,9 @@ struct JSRuntime : public JS::shadow::Runtime, */ js::PerformanceData performance; - /** - * Callback used to ask the embedding to determine in which - * Performance Group the current execution belongs. Typically, this is - * used to regroup JSCompartments from several iframes from the same - * page or from several compartments of the same addon into a single - * Performance Group. - * - * May be `nullptr`, in which case we put all the JSCompartments - * in the same PerformanceGroup. - */ - JSCurrentPerfGroupCallback currentPerfGroupCallback; - Stopwatch() : iteration(0) , isEmpty(true) - , currentPerfGroupCallback(nullptr) , isActive_(false) { } diff --git a/js/xpconnect/src/XPCJSRuntime.cpp b/js/xpconnect/src/XPCJSRuntime.cpp index ae8abdde6a49..5dc8105bbe03 100644 --- a/js/xpconnect/src/XPCJSRuntime.cpp +++ b/js/xpconnect/src/XPCJSRuntime.cpp @@ -1779,20 +1779,6 @@ GetCompartmentName(JSCompartment* c, nsCString& name, int* anonymizeID, } } -extern void -xpc::GetCurrentCompartmentName(JSContext* cx, nsCString& name) -{ - RootedObject global(cx, JS::CurrentGlobalOrNull(cx)); - if (!global) { - name.AssignLiteral("no global"); - return; - } - - JSCompartment* compartment = GetObjectCompartment(global); - int anonymizeID = 0; - GetCompartmentName(compartment, name, &anonymizeID, false); -} - static int64_t JSMainRuntimeGCHeapDistinguishedAmount() { @@ -3307,47 +3293,6 @@ static const JSWrapObjectCallbacks WrapObjectCallbacks = { xpc::WrapperFactory::PrepareForWrapping }; -/** - * Group JSCompartments into PerformanceGroups. - * - * - All JSCompartments from the same add-on belong to the same - * PerformanceGroup. - * - All JSCompartments from the same same webpage (including - * frames) belong to the same PerformanceGroup. - * - All other JSCompartments (normally, system add-ons) - * belong to to a big uncategorized PerformanceGroup. - */ -static void* -GetCurrentPerfGroupCallback(JSContext* cx) { - RootedObject global(cx, CurrentGlobalOrNull(cx)); - if (!global) { - // This can happen for the atom compartments, which is system - // code. - return nullptr; - } - - JSAddonId* addonId = AddonIdOfObject(global); - if (addonId) { - // If this is an add-on, use the id as key. - return addonId; - } - - // If the compartment belongs to a webpage, use the address of the - // topmost scriptable window, hence regrouping all frames of a - // window. - nsRefPtr win = WindowOrNull(global); - if (win) { - nsCOMPtr top; - nsresult rv = win->GetScriptableTop(getter_AddRefs(top)); - NS_ENSURE_SUCCESS(rv, nullptr); - - return top.get(); - } - - // Otherwise, this is platform code, use `nullptr` as key. - return nullptr; -} - XPCJSRuntime::XPCJSRuntime(nsXPConnect* aXPConnect) : CycleCollectedJSRuntime(nullptr, JS::DefaultHeapMaxBytes, JS::DefaultNurseryBytes), mJSContextStack(new XPCJSContextStack(this)), @@ -3528,8 +3473,6 @@ XPCJSRuntime::XPCJSRuntime(nsXPConnect* aXPConnect) // Watch for the JS boolean options. ReloadPrefsCallback(nullptr, this); Preferences::RegisterCallback(ReloadPrefsCallback, JS_OPTIONS_DOT_STR, this); - - JS_SetCurrentPerfGroupCallback(runtime, ::GetCurrentPerfGroupCallback); } // static diff --git a/js/xpconnect/src/xpcpublic.h b/js/xpconnect/src/xpcpublic.h index b063fb45df95..f07f2a46e8a4 100644 --- a/js/xpconnect/src/xpcpublic.h +++ b/js/xpconnect/src/xpcpublic.h @@ -139,6 +139,9 @@ XrayAwareCalleeGlobal(JSObject* fun); void TraceXPCGlobal(JSTracer* trc, JSObject* obj); +uint64_t +GetCompartmentCPOWMicroseconds(JSCompartment* compartment); + } /* namespace xpc */ namespace JS { @@ -529,12 +532,6 @@ void DispatchScriptErrorEvent(nsPIDOMWindow* win, JSRuntime* rt, xpc::ErrorReport* xpcReport, JS::Handle exception); -// Return a name for the compartment. -// This function makes reasonable efforts to make this name both mostly human-readable -// and unique. However, there are no guarantees of either property. -extern void -GetCurrentCompartmentName(JSContext*, nsCString& name); - } // namespace xpc namespace mozilla { diff --git a/toolkit/components/aboutperformance/content/aboutPerformance.js b/toolkit/components/aboutperformance/content/aboutPerformance.js index 664ba14e4b0f..61e4dc31668a 100644 --- a/toolkit/components/aboutperformance/content/aboutPerformance.js +++ b/toolkit/components/aboutperformance/content/aboutPerformance.js @@ -226,7 +226,7 @@ function updateLiveData() { _el.textContent = a ? a.name : _item.name }); } else { - el.textContent = item.title || item.name; + el.textContent = item.name; } } } catch (ex) { diff --git a/toolkit/components/aboutperformance/tests/browser/browser.ini b/toolkit/components/aboutperformance/tests/browser/browser.ini index ae0ee252b4a8..292d14623166 100644 --- a/toolkit/components/aboutperformance/tests/browser/browser.ini +++ b/toolkit/components/aboutperformance/tests/browser/browser.ini @@ -1,9 +1,11 @@ +# This Source Code Form is subject to the terms of the Mozilla Public +# License, v. 2.0. If a copy of the MPL was not distributed with this +# file, You can obtain one at http://mozilla.org/MPL/2.0/. + [DEFAULT] head = head.js -support-files = +support-files = browser_compartments.html - browser_compartments_frame.html - browser_compartments_script.js [browser_aboutperformance.js] skip-if = e10s # Feature not implemented yet – bug 1140310 diff --git a/toolkit/components/aboutperformance/tests/browser/browser_aboutperformance.js b/toolkit/components/aboutperformance/tests/browser/browser_aboutperformance.js index da68b7311f60..79b994d6338b 100644 --- a/toolkit/components/aboutperformance/tests/browser/browser_aboutperformance.js +++ b/toolkit/components/aboutperformance/tests/browser/browser_aboutperformance.js @@ -5,16 +5,15 @@ Cu.import("resource://testing-common/ContentTask.jsm", this); -const SALT = Math.random(); -const URL = "http://example.com/browser/toolkit/components/aboutperformance/tests/browser/browser_compartments.html?test=" + SALT; +const URL = "http://example.com/browser/toolkit/components/aboutperformance/tests/browser/browser_compartments.html?test=" + Math.random(); // This function is injected as source as a frameScript function frameScript() { "use strict"; - addMessageListener("aboutperformance-test:hasItems", ({data: salt}) => { + addMessageListener("aboutperformance-test:hasItems", ({data: url}) => { let hasPlatform = false; - let hasSalt = false; + let hasURL = false; try { let eltData = content.document.getElementById("liveData"); @@ -26,11 +25,10 @@ function frameScript() { hasPlatform = eltData.querySelector("tr.platform") != null; // Find if we have a row for our URL - hasSalt = false; + hasURL = false; for (let eltContent of eltData.querySelectorAll("tr.content td.name")) { - let name = eltContent.textContent; - if (name.contains(salt) && !name.contains("http")) { - hasSalt = true; + if (eltContent.textContent == url) { + hasURL = true; break; } } @@ -39,7 +37,7 @@ function frameScript() { Cu.reportError("Error in content: " + ex); Cu.reportError(ex.stack); } finally { - sendAsyncMessage("aboutperformance-test:hasItems", {hasPlatform, hasSalt}); + sendAsyncMessage("aboutperformance-test:hasItems", {hasPlatform, hasURL}); } }); } @@ -53,10 +51,10 @@ add_task(function* test() { while (true) { yield new Promise(resolve => setTimeout(resolve, 100)); - let {hasPlatform, hasSalt} = (yield promiseContentResponse(tabAboutPerformance.linkedBrowser, "aboutperformance-test:hasItems", SALT)); - info(`Platform: ${hasPlatform}, salt: ${hasSalt}`); - if (hasPlatform && hasSalt) { - Assert.ok(true, "Found a row for and a row for our page"); + let {hasPlatform, hasURL} = (yield promiseContentResponse(tabAboutPerformance.linkedBrowser, "aboutperformance-test:hasItems", URL)); + info(`Platform: ${hasPlatform}, url: ${hasURL}`); + if (hasPlatform && hasURL) { + Assert.ok(true, "Found a row for and a row for our URL"); break; } } diff --git a/toolkit/components/aboutperformance/tests/browser/browser_compartments.html b/toolkit/components/aboutperformance/tests/browser/browser_compartments.html index e0a532f39444..120aeff789ca 100644 --- a/toolkit/components/aboutperformance/tests/browser/browser_compartments.html +++ b/toolkit/components/aboutperformance/tests/browser/browser_compartments.html @@ -2,24 +2,24 @@ - Main frame for test browser_compartments.js + browser_compartments.html - - -Main frame. - - - - + +browser_compartments.html diff --git a/toolkit/components/aboutperformance/tests/browser/browser_compartments_frame.html b/toolkit/components/aboutperformance/tests/browser/browser_compartments_frame.html deleted file mode 100644 index 69edfe871bab..000000000000 --- a/toolkit/components/aboutperformance/tests/browser/browser_compartments_frame.html +++ /dev/null @@ -1,12 +0,0 @@ - - - - - Subframe for test browser_compartments.html (do not change this title) - - - - -Subframe loaded. - - diff --git a/toolkit/components/aboutperformance/tests/browser/browser_compartments_script.js b/toolkit/components/aboutperformance/tests/browser/browser_compartments_script.js deleted file mode 100644 index d588874b3044..000000000000 --- a/toolkit/components/aboutperformance/tests/browser/browser_compartments_script.js +++ /dev/null @@ -1,20 +0,0 @@ - -window.addEventListener("message", e => { - console.log("frame content", "message", e); - if ("title" in e.data) { - document.title = e.data.title; - } -}); - -// Use some CPU. -window.setInterval(() => { - // Compute an arbitrary value, print it out to make sure that the JS - // engine doesn't discard all our computation. - var date = Date.now(); - var array = []; - var i = 0; - while (Date.now() - date <= 100) { - array[i%2] = i++; - } - console.log("Arbitrary value", window.location, array); -}, 300); diff --git a/toolkit/components/perfmonitoring/PerformanceStats.jsm b/toolkit/components/perfmonitoring/PerformanceStats.jsm index bf3e30229d31..47bbb4c0617f 100644 --- a/toolkit/components/perfmonitoring/PerformanceStats.jsm +++ b/toolkit/components/perfmonitoring/PerformanceStats.jsm @@ -25,8 +25,7 @@ let performanceStatsService = const PROPERTIES_NUMBERED = ["totalUserTime", "totalSystemTime", "totalCPOWTime", "ticks"]; -const PROPERTIES_META_IMMUTABLE = ["name", "addonId", "isSystem"]; -const PROPERTIES_META = [...PROPERTIES_META_IMMUTABLE, "windowId", "title"]; +const PROPERTIES_META = ["name", "addonId", "isSystem"]; const PROPERTIES_FLAT = [...PROPERTIES_NUMBERED, ...PROPERTIES_META]; /** @@ -42,15 +41,6 @@ const PROPERTIES_FLAT = [...PROPERTIES_NUMBERED, ...PROPERTIES_META]; * * @field {string} addonId The identifier of the addon (e.g. "myaddon@foo.bar"). * - * @field {string|null} title The title of the webpage to which this code - * belongs. Note that this is the title of the entire webpage (i.e. the tab), - * even if the code is executed in an iframe. Also note that this title may - * change over time. - * - * @field {number} windowId The outer window ID of the top-level nsIDOMWindow - * to which this code belongs. May be 0 if the code doesn't belong to any - * nsIDOMWindow. - * * @field {boolean} isSystem `true` if the component is a system component (i.e. * an add-on or platform-code), `false` otherwise (i.e. a webpage). * diff --git a/toolkit/components/perfmonitoring/moz.build b/toolkit/components/perfmonitoring/moz.build index b1ba6e8094e9..6d53445e3945 100644 --- a/toolkit/components/perfmonitoring/moz.build +++ b/toolkit/components/perfmonitoring/moz.build @@ -28,9 +28,4 @@ EXPORTS += [ 'nsPerformanceStats.h' ] -# We need nsGlobalWindow -LOCAL_INCLUDES += [ - '/dom/base', -] - FINAL_LIBRARY = 'xul' diff --git a/toolkit/components/perfmonitoring/nsIPerformanceStats.idl b/toolkit/components/perfmonitoring/nsIPerformanceStats.idl index 37cc5e58b37d..72f105ea63f3 100644 --- a/toolkit/components/perfmonitoring/nsIPerformanceStats.idl +++ b/toolkit/components/perfmonitoring/nsIPerformanceStats.idl @@ -6,7 +6,6 @@ #include "nsISupports.idl" #include "nsIArray.idl" -#include "nsIDOMWindow.idl" /** * Mechanisms for querying the current process about performance @@ -22,7 +21,7 @@ * All values are monotonic and are updated only when * `nsIPerformanceStatsService.isStopwatchActive` is `true`. */ -[scriptable, uuid(c2e2d494-06b9-40c5-91f7-505086dc4ecd)] +[scriptable, uuid(f015cbad-e16f-4982-a080-67e4e69a5b2e)] interface nsIPerformanceStats: nsISupports { /** * The name of the component: @@ -39,18 +38,6 @@ interface nsIPerformanceStats: nsISupports { */ readonly attribute AString addonId; - /** - * If the component is code executed in a window, the ID of the topmost - * outer window (i.e. the tab), otherwise 0. - */ - readonly attribute uint64_t windowId; - - /** - * If the component is code executed in a window, the title of the topmost - * window (i.e. the tab), otherwise an empty string. - */ - readonly attribute AString title; - /** * Total amount of time spent executing code in this group, in * microseconds. @@ -104,7 +91,7 @@ interface nsIPerformanceSnapshot: nsISupports { nsIPerformanceStats getProcessData(); }; -[scriptable, builtinclass, uuid(3e4bff64-555f-41ec-9e7f-9338c77fd696)] +[scriptable, builtinclass, uuid(5795113a-39a1-4087-ba09-98b7d07d025a)] interface nsIPerformanceStatsService : nsISupports { /** * `true` if we should monitor performance, `false` otherwise. @@ -114,7 +101,7 @@ interface nsIPerformanceStatsService : nsISupports { /** * Capture a snapshot of the performance data. */ - [implicit_jscontext] nsIPerformanceSnapshot getSnapshot(); + nsIPerformanceSnapshot getSnapshot(); }; %{C++ diff --git a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp index bfdfa22edfbf..544fe01210fe 100644 --- a/toolkit/components/perfmonitoring/nsPerformanceStats.cpp +++ b/toolkit/components/perfmonitoring/nsPerformanceStats.cpp @@ -3,37 +3,24 @@ * 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/. */ +#include "jsapi.h" #include "nsPerformanceStats.h" - #include "nsMemory.h" #include "nsLiteralString.h" #include "nsCRTGlue.h" +#include "nsIJSRuntimeService.h" #include "nsServiceManagerUtils.h" - #include "nsCOMArray.h" #include "nsIMutableArray.h" - -#include "jsapi.h" #include "nsJSUtils.h" #include "xpcpublic.h" #include "jspubtd.h" -#include "nsIJSRuntimeService.h" - -#include "nsIDOMWindow.h" -#include "nsGlobalWindow.h" class nsPerformanceStats: public nsIPerformanceStats { public: - nsPerformanceStats(const nsAString& aName, - const nsAString& aAddonId, - const nsAString& aTitle, - const uint64_t aWindowId, - const bool aIsSystem, - const js::PerformanceData& aPerformanceData) + nsPerformanceStats(nsAString& aName, nsAString& aAddonId, bool aIsSystem, js::PerformanceData& aPerformanceData) : mName(aName) , mAddonId(aAddonId) - , mTitle(aTitle) - , mWindowId(aWindowId) , mIsSystem(aIsSystem) , mPerformanceData(aPerformanceData) { @@ -54,24 +41,6 @@ public: return NS_OK; }; - /* readonly attribute uint64_t windowId; */ - NS_IMETHOD GetWindowId(uint64_t *aWindowId) override { - *aWindowId = mWindowId; - return NS_OK; - } - - /* readonly attribute AString title; */ - NS_IMETHOD GetTitle(nsAString & aTitle) override { - aTitle.Assign(mTitle); - return NS_OK; - } - - /* readonly attribute bool isSystem; */ - NS_IMETHOD GetIsSystem(bool *_retval) override { - *_retval = mIsSystem; - return NS_OK; - } - /* readonly attribute unsigned long long totalUserTime; */ NS_IMETHOD GetTotalUserTime(uint64_t *aTotalUserTime) override { *aTotalUserTime = mPerformanceData.totalUserTime; @@ -109,13 +78,16 @@ public: return NS_OK; }; + /* readonly attribute bool isSystem; */ + NS_IMETHOD GetIsSystem(bool *_retval) override { + *_retval = mIsSystem; + return NS_OK; + } + private: nsString mName; nsString mAddonId; - nsString mTitle; - uint64_t mWindowId; bool mIsSystem; - js::PerformanceData mPerformanceData; virtual ~nsPerformanceStats() {} @@ -131,45 +103,15 @@ public: NS_DECL_NSIPERFORMANCESNAPSHOT nsPerformanceSnapshot(); - nsresult Init(JSContext*); + nsresult Init(); private: virtual ~nsPerformanceSnapshot(); /** - * Import a `js::PerformanceStats` as a `nsIPerformanceStats`. - * - * Precondition: this method assumes that we have entered the JSCompartment for which data `c` - * has been collected. - * - * `cx` may be `nullptr` if we are importing the statistics for the - * entire process, rather than the statistics for a specific set of - * compartments. + * Import a `PerformanceStats` as a `nsIPerformanceStats`. */ - already_AddRefed ImportStats(JSContext* cx, const js::PerformanceData& data); + already_AddRefed ImportStats(js::PerformanceStats* c); - /** - * Callbacks for iterating through the `PerformanceStats` of a runtime. - */ - bool IterPerformanceStatsCallbackInternal(JSContext* cx, const js::PerformanceData& stats); - static bool IterPerformanceStatsCallback(JSContext* cx, const js::PerformanceData& stats, void* self); - - // If the context represents a window, extract the title and window ID. - // Otherwise, extract "" and 0. - static void GetWindowData(JSContext*, - nsString& title, - uint64_t* windowId); - - // If the context presents an add-on, extract the addon ID. - // Otherwise, extract "". - static void GetAddonId(JSContext*, - JS::Handle global, - nsAString& addonId); - - // Determine whether a context is part of the system principals. - static bool GetIsSystem(JSContext*, - JS::Handle global); - -private: nsCOMArray mComponentsData; nsCOMPtr mProcessData; }; @@ -184,118 +126,36 @@ nsPerformanceSnapshot::~nsPerformanceSnapshot() { } -/* static */ void -nsPerformanceSnapshot::GetWindowData(JSContext* cx, - nsString& title, - uint64_t* windowId) -{ - MOZ_ASSERT(windowId); - - title.SetIsVoid(true); - *windowId = 0; - - nsCOMPtr win = xpc::CurrentWindowOrNull(cx); - if (!win) { - return; - } - - nsCOMPtr top; - nsresult rv = win->GetTop(getter_AddRefs(top)); - if (!top || NS_FAILED(rv)) { - return; - } - - nsCOMPtr privateTop = do_QueryInterface(top); - if (!privateTop) { - return; - } - - nsCOMPtr doc = privateTop->GetExtantDoc(); - if (!doc) { - return; - } - doc->GetTitle(title); - *windowId = privateTop->WindowID(); -} - -/* static */ void -nsPerformanceSnapshot::GetAddonId(JSContext*, - JS::Handle global, - nsAString& addonId) -{ - addonId.AssignLiteral(""); - - JSAddonId* jsid = AddonIdOfObject(global); - if (!jsid) { - return; - } - AssignJSFlatString(addonId, (JSFlatString*)jsid); -} - -/* static */ bool -nsPerformanceSnapshot::GetIsSystem(JSContext*, - JS::Handle global) -{ - return nsContentUtils::IsSystemPrincipal(nsContentUtils::ObjectPrincipal(global)); -} - already_AddRefed -nsPerformanceSnapshot::ImportStats(JSContext* cx, const js::PerformanceData& performance) { - JS::RootedObject global(cx, JS::CurrentGlobalOrNull(cx)); - - if (!global) { - // While it is possible for a compartment to have no global - // (e.g. atoms), this compartment is not very interesting for us. - return nullptr; - } - +nsPerformanceSnapshot::ImportStats(js::PerformanceStats* c) { nsString addonId; - GetAddonId(cx, global, addonId); - - nsString title; - uint64_t windowId; - GetWindowData(cx, title, &windowId); - - nsAutoString name; - nsAutoCString cname; - xpc::GetCurrentCompartmentName(cx, cname); - name.Assign(NS_ConvertUTF8toUTF16(cname)); - - bool isSystem = GetIsSystem(cx, global); - - nsCOMPtr result = - new nsPerformanceStats(name, addonId, title, windowId, isSystem, performance); + if (c->addonId) { + AssignJSFlatString(addonId, (JSFlatString*)c->addonId); + } + nsCString cname(c->name); + NS_ConvertUTF8toUTF16 name(cname); + nsCOMPtr result = new nsPerformanceStats(name, addonId, c->isSystem, c->performance); return result.forget(); } -/*static*/ bool -nsPerformanceSnapshot::IterPerformanceStatsCallback(JSContext* cx, const js::PerformanceData& stats, void* self) { - return reinterpret_cast(self)->IterPerformanceStatsCallbackInternal(cx, stats); -} - -bool -nsPerformanceSnapshot::IterPerformanceStatsCallbackInternal(JSContext* cx, const js::PerformanceData& stats) { - nsCOMPtr result = ImportStats(cx, stats); - if (result) { - mComponentsData.AppendElement(result); - } - - return true; -} - nsresult -nsPerformanceSnapshot::Init(JSContext* cx) { - js::PerformanceData processStats; - if (!js::IterPerformanceStats(cx, nsPerformanceSnapshot::IterPerformanceStatsCallback, &processStats, this)) { - return NS_ERROR_UNEXPECTED; +nsPerformanceSnapshot::Init() { + JSRuntime* rt; + nsCOMPtr svc(do_GetService("@mozilla.org/js/xpc/RuntimeService;1")); + NS_ENSURE_TRUE(svc, NS_ERROR_FAILURE); + svc->GetRuntime(&rt); + js::PerformanceStats processStats; + js::PerformanceStatsVector componentsStats; + if (!js::GetPerformanceStats(rt, componentsStats, processStats)) { + return NS_ERROR_OUT_OF_MEMORY; } - mProcessData = new nsPerformanceStats(NS_LITERAL_STRING(""), // name - NS_LITERAL_STRING(""), // add-on id - NS_LITERAL_STRING(""), // title - 0, // window id - true, // isSystem - processStats); + size_t num = componentsStats.length(); + for (size_t pos = 0; pos < num; pos++) { + nsCOMPtr stats = ImportStats(&componentsStats[pos]); + mComponentsData.AppendObject(stats); + } + mProcessData = ImportStats(&processStats); return NS_OK; } @@ -349,10 +209,10 @@ NS_IMETHODIMP nsPerformanceStatsService::SetIsStopwatchActive(JSContext* cx, boo } /* readonly attribute nsIPerformanceSnapshot snapshot; */ -NS_IMETHODIMP nsPerformanceStatsService::GetSnapshot(JSContext* cx, nsIPerformanceSnapshot * *aSnapshot) +NS_IMETHODIMP nsPerformanceStatsService::GetSnapshot(nsIPerformanceSnapshot * *aSnapshot) { nsRefPtr snapshot = new nsPerformanceSnapshot(); - nsresult rv = snapshot->Init(cx); + nsresult rv = snapshot->Init(); if (NS_FAILED(rv)) { return rv; } diff --git a/toolkit/components/perfmonitoring/tests/browser/browser.ini b/toolkit/components/perfmonitoring/tests/browser/browser.ini index 4975f2c88d53..fcf0f90c0a4d 100644 --- a/toolkit/components/perfmonitoring/tests/browser/browser.ini +++ b/toolkit/components/perfmonitoring/tests/browser/browser.ini @@ -3,8 +3,6 @@ head = head.js support-files = browser_Addons_sample.xpi browser_compartments.html - browser_compartments_frame.html - browser_compartments_script.js [browser_AddonWatcher.js] [browser_compartments.js] diff --git a/toolkit/components/perfmonitoring/tests/browser/browser_compartments.html b/toolkit/components/perfmonitoring/tests/browser/browser_compartments.html index d7ee6c418910..120aeff789ca 100644 --- a/toolkit/components/perfmonitoring/tests/browser/browser_compartments.html +++ b/toolkit/components/perfmonitoring/tests/browser/browser_compartments.html @@ -2,19 +2,24 @@ - Main frame for test browser_compartments.js + browser_compartments.html + -Main frame. - - - - - +browser_compartments.html diff --git a/toolkit/components/perfmonitoring/tests/browser/browser_compartments.js b/toolkit/components/perfmonitoring/tests/browser/browser_compartments.js index 08ce72f58ab4..dfb02d7e4de1 100644 --- a/toolkit/components/perfmonitoring/tests/browser/browser_compartments.js +++ b/toolkit/components/perfmonitoring/tests/browser/browser_compartments.js @@ -3,17 +3,10 @@ "use strict"; -/** - * Test that we see jank that takes place in a webpage, - * and that jank from several iframes are actually charged - * to the top window. - */ Cu.import("resource://gre/modules/PerformanceStats.jsm", this); Cu.import("resource://testing-common/ContentTask.jsm", this); const URL = "http://example.com/browser/toolkit/components/perfmonitoring/tests/browser/browser_compartments.html?test=" + Math.random(); -const PARENT_TITLE = `Main frame for test browser_compartments.js ${Math.random()}`; -const FRAME_TITLE = `Subframe for test browser_compartments.js ${Math.random()}`; // This function is injected as source as a frameScript function frameScript() { @@ -37,21 +30,6 @@ function frameScript() { Cu.reportError(ex.stack); } }); - - addMessageListener("compartments-test:setTitles", titles => { - try { - console.log("content", "Setting titles", Object.keys(titles)); - content.document.title = titles.data.parent; - for (let i = 0; i < content.frames.length; ++i) { - content.frames[i].postMessage({title: titles.data.frames}, "*"); - } - console.log("content", "Done setting titles", content.document.title); - sendAsyncMessage("compartments-test:setTitles"); - } catch (ex) { - Cu.reportError("Error in content: " + ex); - Cu.reportError(ex.stack); - } - }); } // A variant of `Assert` that doesn't spam the logs @@ -94,29 +72,28 @@ function monotinicity_tester(source, testName) { }; let sanityCheck = function(prev, next) { - let name = `${testName}: ${next.name}`; info(`Sanity check: ${JSON.stringify(next, null, "\t")}`); if (prev == null) { return; } for (let k of ["name", "addonId", "isSystem"]) { - SilentAssert.equal(prev[k], next[k], `Sanity check (${name}): ${k} hasn't changed.`); + SilentAssert.equal(prev[k], next[k], `Sanity check (${testName}): ${k} hasn't changed.`); } for (let k of ["totalUserTime", "totalSystemTime", "totalCPOWTime", "ticks"]) { - SilentAssert.equal(typeof next[k], "number", `Sanity check (${name}): ${k} is a number.`); - SilentAssert.leq(prev[k], next[k], `Sanity check (${name}): ${k} is monotonic.`); - SilentAssert.leq(0, next[k], `Sanity check (${name}): ${k} is >= 0.`) + SilentAssert.equal(typeof next[k], "number", `Sanity check (${testName}): ${k} is a number.`); + SilentAssert.leq(prev[k], next[k], `Sanity check (${testName}): ${k} is monotonic.`); + SilentAssert.leq(0, next[k], `Sanity check (${testName}): ${k} is >= 0.`) } SilentAssert.equal(prev.durations.length, next.durations.length); for (let i = 0; i < next.durations.length; ++i) { SilentAssert.ok(typeof next.durations[i] == "number" && next.durations[i] >= 0, - `Sanity check (${name}): durations[${i}] is a non-negative number.`); + `Sanity check (${testName}): durations[${i}] is a non-negative number.`); SilentAssert.leq(prev.durations[i], next.durations[i], - `Sanity check (${name}): durations[${i}] is monotonic.`) + `Sanity check (${testName}): durations[${i}] is monotonic.`) } for (let i = 0; i < next.durations.length - 1; ++i) { SilentAssert.leq(next.durations[i + 1], next.durations[i], - `Sanity check (${name}): durations[${i}] >= durations[${i + 1}].`) + `Sanity check (${testName}): durations[${i}] >= durations[${i + 1}].`) } }; let iteration = 0; @@ -141,7 +118,7 @@ function monotinicity_tester(source, testName) { let set = new Set(); let keys = []; for (let item of snapshot.componentsData) { - let key = `{name: ${item.name}, addonId: ${item.addonId}, isSystem: ${item.isSystem}, windowId: ${item.windowId}}`; + let key = `{name: ${item.name}, addonId: ${item.addonId}, isSystem: ${item.isSystem}}`; keys.push(key); set.add(key); sanityCheck(previous.componentsMap.get(key), item); @@ -189,40 +166,17 @@ add_task(function* test() { let skipTotalUserTime = hasLowPrecision(); - while (true) { - yield new Promise(resolve => setTimeout(resolve, 100)); - - // We may have race conditions with DOM loading. - // Don't waste too much brainpower here, let's just ask - // repeatedly for the title to be changed, until this works. - info("Setting titles"); - yield promiseContentResponse(browser, "compartments-test:setTitles", { - parent: PARENT_TITLE, - frames: FRAME_TITLE - }); - info("Titles set"); - let stats = (yield promiseContentResponse(browser, "compartments-test:getStatistics", null)); - - // While the webpage consists in three compartments, we should see only - // one `PerformanceData` in `componentsData`. Its `name` is undefined - // (could be either the main frame or one of its subframes), but its - // `title` should be the title of the main frame. - let frame = stats.componentsData.find(stat => stat.title == FRAME_TITLE); - Assert.equal(frame, null, "Searching by title, the frames don't show up in the list of components"); - - let parent = stats.componentsData.find(stat => stat.title == PARENT_TITLE); - if (!parent) { - info("Searching by title, we didn't find the main frame"); - continue; - } - info("Searching by title, we found the main frame"); - - info(`Total user time: ${parent.totalUserTime}`); - if (skipTotalUserTime || parent.totalUserTime > 1000) { + let found = stats.componentsData.find(stat => { + return (stat.name.indexOf(URL) != -1) + && (skipTotalUserTime || stat.totalUserTime > 1000) + }); + if (found) { + info(`Expected totalUserTime > 1000, got ${found.totalUserTime}`); break; } + yield new Promise(resolve => setTimeout(resolve, 100)); } // Cleanup diff --git a/toolkit/components/perfmonitoring/tests/browser/browser_compartments_frame.html b/toolkit/components/perfmonitoring/tests/browser/browser_compartments_frame.html deleted file mode 100644 index 69edfe871bab..000000000000 --- a/toolkit/components/perfmonitoring/tests/browser/browser_compartments_frame.html +++ /dev/null @@ -1,12 +0,0 @@ - - - - - Subframe for test browser_compartments.html (do not change this title) - - - - -Subframe loaded. - - diff --git a/toolkit/components/perfmonitoring/tests/browser/browser_compartments_script.js b/toolkit/components/perfmonitoring/tests/browser/browser_compartments_script.js deleted file mode 100644 index d588874b3044..000000000000 --- a/toolkit/components/perfmonitoring/tests/browser/browser_compartments_script.js +++ /dev/null @@ -1,20 +0,0 @@ - -window.addEventListener("message", e => { - console.log("frame content", "message", e); - if ("title" in e.data) { - document.title = e.data.title; - } -}); - -// Use some CPU. -window.setInterval(() => { - // Compute an arbitrary value, print it out to make sure that the JS - // engine doesn't discard all our computation. - var date = Date.now(); - var array = []; - var i = 0; - while (Date.now() - date <= 100) { - array[i%2] = i++; - } - console.log("Arbitrary value", window.location, array); -}, 300); From 64f650e19f081c2ddc82c199a802ec45767adc11 Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Sat, 16 May 2015 02:10:00 -0400 Subject: [PATCH 19/33] Bug 1136101 - Add an "Add rule" button in the rules view toolbar. r=bgrins, ui-r=shorlander --- .../devtools/shared/widgets/filter-widget.css | 2 +- .../devtools/styleinspector/cssruleview.xhtml | 2 + browser/devtools/styleinspector/rule-view.js | 7 ++- .../test/browser_ruleview_add-rule_01.js | 55 +++++++++++-------- .../test/browser_ruleview_add-rule_02.js | 19 +------ .../test/browser_ruleview_add-rule_03.js | 19 +------ .../browser/devtools/styleinspector.dtd | 5 ++ browser/themes/shared/devtools/images/add.svg | 12 +++- browser/themes/shared/devtools/ruleview.css | 5 ++ .../global/devtools/styleinspector.properties | 7 ++- 10 files changed, 68 insertions(+), 65 deletions(-) diff --git a/browser/devtools/shared/widgets/filter-widget.css b/browser/devtools/shared/widgets/filter-widget.css index 66edf832d280..f139d5b76fa3 100644 --- a/browser/devtools/shared/widgets/filter-widget.css +++ b/browser/devtools/shared/widgets/filter-widget.css @@ -112,7 +112,7 @@ #add-filter { -moz-appearance: none; background: url(chrome://browser/skin/devtools/add.svg); - background-size: 18px; + background-size: cover; border: none; width: 16px; height: 16px; diff --git a/browser/devtools/styleinspector/cssruleview.xhtml b/browser/devtools/styleinspector/cssruleview.xhtml index 429d99a2e152..2b9fb9eb8f84 100644 --- a/browser/devtools/styleinspector/cssruleview.xhtml +++ b/browser/devtools/styleinspector/cssruleview.xhtml @@ -45,6 +45,8 @@ type="search" placeholder="&filterStylesPlaceholder;"/> + + diff --git a/browser/devtools/styleinspector/rule-view.js b/browser/devtools/styleinspector/rule-view.js index 975b361987a0..f9e0e3c34399 100644 --- a/browser/devtools/styleinspector/rule-view.js +++ b/browser/devtools/styleinspector/rule-view.js @@ -1124,6 +1124,7 @@ function CssRuleView(aInspector, aDoc, aStore, aPageStyle) { this._onFilterTextboxContextMenu = this._onFilterTextboxContextMenu.bind(this); this.element = this.doc.getElementById("ruleview-container"); + this.addRuleButton = this.doc.getElementById("ruleview-add-rule-button"); this.searchField = this.doc.getElementById("ruleview-searchbox"); this.searchClearButton = this.doc.getElementById("ruleview-searchinput-clear"); @@ -1131,6 +1132,7 @@ function CssRuleView(aInspector, aDoc, aStore, aPageStyle) { this.element.addEventListener("copy", this._onCopy); this.element.addEventListener("contextmenu", this._onContextMenu); + this.addRuleButton.addEventListener("click", this._onAddRule); this.searchField.addEventListener("input", this._onFilterStyles); this.searchField.addEventListener("keypress", this._onFilterKeyPress); this.searchField.addEventListener("contextmenu", this._onFilterTextboxContextMenu); @@ -1186,8 +1188,8 @@ CssRuleView.prototype = { this._contextmenu.id = "rule-view-context-menu"; this.menuitemAddRule = createMenuItem(this._contextmenu, { - label: "ruleView.contextmenu.addRule", - accesskey: "ruleView.contextmenu.addRule.accessKey", + label: "ruleView.contextmenu.addNewRule", + accesskey: "ruleView.contextmenu.addNewRule.accessKey", command: this._onAddRule }); this.menuitemSelectAll = createMenuItem(this._contextmenu, { @@ -1738,6 +1740,7 @@ CssRuleView.prototype = { // Remove bound listeners this.element.removeEventListener("copy", this._onCopy); this.element.removeEventListener("contextmenu", this._onContextMenu); + this.addRuleButton.removeEventListener("click", this._onAddRule); this.searchField.removeEventListener("input", this._onFilterStyles); this.searchField.removeEventListener("keypress", this._onFilterKeyPress); this.searchField.removeEventListener("contextmenu", diff --git a/browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js index 797276ed5cc8..81fbbfb9df6b 100644 --- a/browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js +++ b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_01.js @@ -27,42 +27,24 @@ const TEST_DATA = [ ]; add_task(function*() { - yield addTab("data:text/html;charset=utf-8,test rule view add rule"); - - info("Creating the test document"); - content.document.body.innerHTML = PAGE_CONTENT; + yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(PAGE_CONTENT)); info("Opening the rule-view"); let {toolbox, inspector, view} = yield openRuleView(); info("Iterating over the test data"); for (let data of TEST_DATA) { - yield runTestData(inspector, view, data); + yield runTestData(inspector, view, data, "context-menu"); + yield runTestData(inspector, view, data, "button"); } }); -function* runTestData(inspector, view, data) { +function* runTestData(inspector, view, data, method) { let {node, expected} = data; info("Selecting the test element"); yield selectNode(node, inspector); - info("Waiting for context menu to be shown"); - let onPopup = once(view._contextmenu, "popupshown"); - let win = view.doc.defaultView; - - EventUtils.synthesizeMouseAtCenter(view.element, - {button: 2, type: "contextmenu"}, win); - yield onPopup; - - ok(!view.menuitemAddRule.hidden, "Add rule is visible"); - - info("Waiting for rule view to change"); - let onRuleViewChanged = once(view, "ruleview-changed"); - - info("Adding the new rule"); - view.menuitemAddRule.click(); - yield onRuleViewChanged; - view._contextmenu.hidePopup(); + yield addNewRule(inspector, view, method); yield testNewRule(view, expected, 1); @@ -70,6 +52,31 @@ function* runTestData(inspector, view, data) { content.document.body.innerHTML = PAGE_CONTENT; } +function* addNewRule(inspector, view, method) { + if (method == "context-menu") { + info("Waiting for context menu to be shown"); + let onPopup = once(view._contextmenu, "popupshown"); + let win = view.doc.defaultView; + + EventUtils.synthesizeMouseAtCenter(view.element, + {button: 2, type: "contextmenu"}, win); + yield onPopup; + + ok(!view.menuitemAddRule.hidden, "Add rule is visible"); + + info("Adding the new rule"); + view.menuitemAddRule.click(); + view._contextmenu.hidePopup(); + } + else { + info("Adding the new rule using the button"); + view.addRuleButton.click(); + } + info("Waiting for rule view to change"); + let onRuleViewChanged = once(view, "ruleview-changed"); + yield onRuleViewChanged; +} + function* testNewRule(view, expected, index) { let idRuleEditor = getRuleViewRuleEditor(view, index); let editor = idRuleEditor.selectorText.ownerDocument.activeElement; @@ -88,4 +95,4 @@ function* testNewRule(view, expected, index) { let lastRule = textProps[textProps.length - 1]; is(lastRule.name, "font-weight", "Last rule name is font-weight"); is(lastRule.value, "bold", "Last rule value is bold"); -} +} \ No newline at end of file diff --git a/browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js index 0db0475e345f..ed5ab33c3943 100644 --- a/browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js +++ b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_02.js @@ -18,10 +18,7 @@ let PAGE_CONTENT = [ ].join("\n"); add_task(function*() { - yield addTab("data:text/html;charset=utf-8,test rule view add rule"); - - info("Creating the test document"); - content.document.body.innerHTML = PAGE_CONTENT; + yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(PAGE_CONTENT)); info("Opening the rule-view"); let {toolbox, inspector, view} = yield openRuleView(); @@ -29,23 +26,13 @@ add_task(function*() { info("Selecting the test element"); yield selectNode("#testid", inspector); - info("Waiting for context menu to be shown"); - let onPopup = once(view._contextmenu, "popupshown"); - let win = view.doc.defaultView; - - EventUtils.synthesizeMouseAtCenter(view.element, - {button: 2, type: "contextmenu"}, win); - yield onPopup; - - ok(!view.menuitemAddRule.hidden, "Add rule is visible"); - info("Waiting for rule view to change"); let onRuleViewChanged = once(view, "ruleview-changed"); info("Adding the new rule"); - view.menuitemAddRule.click(); + view.addRuleButton.click(); + yield onRuleViewChanged; - view._contextmenu.hidePopup(); yield testEditSelector(view, "span"); diff --git a/browser/devtools/styleinspector/test/browser_ruleview_add-rule_03.js b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_03.js index 8fef2484b404..0854e7d731e2 100644 --- a/browser/devtools/styleinspector/test/browser_ruleview_add-rule_03.js +++ b/browser/devtools/styleinspector/test/browser_ruleview_add-rule_03.js @@ -18,10 +18,7 @@ let PAGE_CONTENT = [ ].join("\n"); add_task(function*() { - yield addTab("data:text/html;charset=utf-8,test rule view add rule"); - - info("Creating the test document"); - content.document.body.innerHTML = PAGE_CONTENT; + yield addTab("data:text/html;charset=utf-8," + encodeURIComponent(PAGE_CONTENT)); info("Opening the rule-view"); let {toolbox, inspector, view} = yield openRuleView(); @@ -29,23 +26,13 @@ add_task(function*() { info("Selecting the test element"); yield selectNode("#testid", inspector); - info("Waiting for context menu to be shown"); - let onPopup = once(view._contextmenu, "popupshown"); - let win = view.doc.defaultView; - - EventUtils.synthesizeMouseAtCenter(view.element, - {button: 2, type: "contextmenu"}, win); - yield onPopup; - - ok(!view.menuitemAddRule.hidden, "Add rule is visible"); - info("Waiting for rule view to change"); let onRuleViewChanged = once(view, "ruleview-changed"); info("Adding the new rule"); - view.menuitemAddRule.click(); + view.addRuleButton.click(); + yield onRuleViewChanged; - view._contextmenu.hidePopup(); info("Adding new properties to the new rule"); yield testNewRule(view, "#testid", 1); diff --git a/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd b/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd index ae980d694845..45c26696c685 100644 --- a/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd +++ b/browser/locales/en-US/chrome/browser/devtools/styleinspector.dtd @@ -17,6 +17,11 @@ - the search box when no search term has been entered. --> + + + diff --git a/browser/themes/shared/devtools/images/add.svg b/browser/themes/shared/devtools/images/add.svg index af162e36047b..001343849e42 100644 --- a/browser/themes/shared/devtools/images/add.svg +++ b/browser/themes/shared/devtools/images/add.svg @@ -1,3 +1,9 @@ - - - + + + + + + + \ No newline at end of file diff --git a/browser/themes/shared/devtools/ruleview.css b/browser/themes/shared/devtools/ruleview.css index c247d6191f1c..f20079c6e4f9 100644 --- a/browser/themes/shared/devtools/ruleview.css +++ b/browser/themes/shared/devtools/ruleview.css @@ -264,3 +264,8 @@ .ruleview-selectorhighlighter.highlighted { background-position: -16px 0; } + +#ruleview-add-rule-button::before { + background-image: url("chrome://browser/skin/devtools/add.svg"); + background-size: cover; +} \ No newline at end of file diff --git a/toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties b/toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties index 07b6045f3014..bfcefd9bd9b9 100644 --- a/toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties +++ b/toolkit/locales/en-US/chrome/global/devtools/styleinspector.properties @@ -112,13 +112,14 @@ ruleView.contextmenu.showMdnDocs=Show MDN docs # the rule view context menu "Show MDN docs" entry. ruleView.contextmenu.showMdnDocs.accessKey=D -# LOCALIZATION NOTE (ruleView.contextmenu.addRule): Text displayed in the +# LOCALIZATION NOTE (ruleView.contextmenu.addNewRule): Text displayed in the # rule view context menu for adding a new rule to the element. -ruleView.contextmenu.addRule=Add rule +# This should match addRuleButton.tooltip in styleinspector.dtd +ruleView.contextmenu.addNewRule=Add new rule # LOCALIZATION NOTE (ruleView.contextmenu.addRule.accessKey): Access key for # the rule view context menu "Add rule" entry. -ruleView.contextmenu.addRule.accessKey=R +ruleView.contextmenu.addNewRule.accessKey=R # LOCALIZATION NOTE (computedView.contextmenu.selectAll): Text displayed in the # computed view context menu. From c38e00836762d6d2fafb2cf5d98eec270ec63bdb Mon Sep 17 00:00:00 2001 From: cedric Date: Mon, 11 May 2015 17:43:15 -0700 Subject: [PATCH 20/33] Bug 1152842 - Remove legacy Download Manager support from test_bug383369.html. r=paolo --- .../mixedcontent/test_bug383369.html | 68 ++++--------------- 1 file changed, 14 insertions(+), 54 deletions(-) diff --git a/security/manager/ssl/tests/mochitest/mixedcontent/test_bug383369.html b/security/manager/ssl/tests/mochitest/mixedcontent/test_bug383369.html index 159a65064e50..7fdacd62b1b9 100644 --- a/security/manager/ssl/tests/mochitest/mixedcontent/test_bug383369.html +++ b/security/manager/ssl/tests/mochitest/mixedcontent/test_bug383369.html @@ -44,60 +44,20 @@ var theWindow = window; - var useJSTransfer = false; - try { - // This method throws an exception if the old Download Manager is disabled. - Services.downloads.activeDownloadCount; - } catch (ex) { - useJSTransfer = true; - } - - if (useJSTransfer) { - var Downloads = SpecialPowers.Cu.import("resource://gre/modules/Downloads.jsm").Downloads; - Downloads.getList(Downloads.PUBLIC).then(list => { - list = SpecialPowers.wrap(list); - list.addView({ - onDownloadAdded: function (aDownload) { - list.removeView(this); - SpecialPowers.wrap(aDownload).whenSucceeded().then(() => { - list.removeFinished(); - theWindow.location = "bug383369step2.html"; - }); - }, - }); - window.location = "download.auto"; - }).then(null, SpecialPowers.Cu.reportError); - - return; - } - - var downloadManager = SpecialPowers.Cc["@mozilla.org/download-manager;1"] - .getService(SpecialPowers.Ci.nsIDownloadManager); - var observer = { - observe: function(subject, topic, data) { - switch (topic) { - case "dl-done": - case "dl-failed": - case "dl-blocked": - case "dl-dirty": - downloadManager.cleanUp(); - theWindow.location = "bug383369step2.html"; - observerService.removeObserver(this, "dl-done"); - observerService.removeObserver(this, "dl-failed"); - observerService.removeObserver(this, "dl-blocked"); - observerService.removeObserver(this, "dl-dirty"); - break; - } - } - }; - var observerService = SpecialPowers.Cc["@mozilla.org/observer-service;1"] - .getService(SpecialPowers.Ci.nsIObserverService); - observerService.addObserver(observer, "dl-done", false); - observerService.addObserver(observer, "dl-failed", false); - observerService.addObserver(observer, "dl-blocked", false); - observerService.addObserver(observer, "dl-dirty", false); - - window.location = "download.auto"; + var Downloads = SpecialPowers.Cu.import("resource://gre/modules/Downloads.jsm").Downloads; + Downloads.getList(Downloads.PUBLIC).then(list => { + list = SpecialPowers.wrap(list); + list.addView({ + onDownloadAdded: function (aDownload) { + list.removeView(this); + SpecialPowers.wrap(aDownload).whenSucceeded().then(() => { + list.removeFinished(); + theWindow.location = "bug383369step2.html"; + }); + }, + }); + window.location = "download.auto"; + }).then(null, SpecialPowers.Cu.reportError); } function afterNavigationTest() From 3b82459ced08560f7bfeadc967bb9f306c0fa7d2 Mon Sep 17 00:00:00 2001 From: Jan Keromnes Date: Thu, 14 May 2015 08:30:00 -0400 Subject: [PATCH 21/33] Bug 1158144 - Fix "Copy URL Parameters" line separation on Windows. r=vporof --- browser/devtools/netmonitor/netmonitor-view.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/browser/devtools/netmonitor/netmonitor-view.js b/browser/devtools/netmonitor/netmonitor-view.js index f0136f248eac..24e367b6444a 100644 --- a/browser/devtools/netmonitor/netmonitor-view.js +++ b/browser/devtools/netmonitor/netmonitor-view.js @@ -573,7 +573,7 @@ RequestsMenuView.prototype = Heritage.extend(WidgetMethods, { let selected = this.selectedItem.attachment; let params = nsIURL(selected.url).query.split("&"); let string = params.join(Services.appinfo.OS === "WINNT" ? "\r\n" : "\n"); - clipboardHelper.copyString(params.join("\n"), document); + clipboardHelper.copyString(string, document); }, /** From 58e3cd94d4d3caad4cedb6e181ef4e57e6ba87f8 Mon Sep 17 00:00:00 2001 From: Jan Keromnes Date: Thu, 14 May 2015 08:30:00 -0400 Subject: [PATCH 22/33] Bug 1158144 - Implement "Copy POST Data" context menu item. r=vporof --- .../devtools/netmonitor/netmonitor-view.js | 133 ++++++++++++++---- browser/devtools/netmonitor/netmonitor.xul | 4 + .../test/browser_net_copy_params.js | 58 ++++++-- .../chrome/browser/devtools/netmonitor.dtd | 8 ++ 4 files changed, 157 insertions(+), 46 deletions(-) diff --git a/browser/devtools/netmonitor/netmonitor-view.js b/browser/devtools/netmonitor/netmonitor-view.js index 24e367b6444a..14a68d6b2567 100644 --- a/browser/devtools/netmonitor/netmonitor-view.js +++ b/browser/devtools/netmonitor/netmonitor-view.js @@ -576,6 +576,83 @@ RequestsMenuView.prototype = Heritage.extend(WidgetMethods, { clipboardHelper.copyString(string, document); }, + /** + * Extracts any urlencoded form data sections (e.g. "?foo=bar&baz=42") from a + * POST request. + * + * @param object aHeaders + * The "requestHeaders". + * @param object aUploadHeaders + * The "requestHeadersFromUploadStream". + * @param object aPostData + * The "requestPostData". + * @return array + * A promise that is resolved with the extracted form data. + */ + _getFormDataSections: Task.async(function*(aHeaders, aUploadHeaders, aPostData) { + let formDataSections = []; + + let { headers: requestHeaders } = aHeaders; + let { headers: payloadHeaders } = aUploadHeaders; + let allHeaders = [...payloadHeaders, ...requestHeaders]; + + let contentTypeHeader = allHeaders.find(e => e.name.toLowerCase() == "content-type"); + let contentTypeLongString = contentTypeHeader ? contentTypeHeader.value : ""; + let contentType = yield gNetwork.getString(contentTypeLongString); + + if (contentType.includes("x-www-form-urlencoded")) { + let postDataLongString = aPostData.postData.text; + let postData = yield gNetwork.getString(postDataLongString); + + for (let section of postData.split(/\r\n|\r|\n/)) { + // Before displaying it, make sure this section of the POST data + // isn't a line containing upload stream headers. + if (payloadHeaders.every(header => !section.startsWith(header.name))) { + formDataSections.push(section); + } + } + } + + return formDataSections; + }), + + /** + * Copy the request form data parameters (or raw payload) from the currently selected item. + */ + copyPostData: Task.async(function*() { + let selected = this.selectedItem.attachment; + let view = this; + + // Try to extract any form data parameters. + let formDataSections = yield view._getFormDataSections( + selected.requestHeaders, + selected.requestHeadersFromUploadStream, + selected.requestPostData); + + let params = []; + formDataSections.forEach(section => { + let paramsArray = parseQueryString(section); + if (paramsArray) { + params = [...params, ...paramsArray]; + } + }); + + let string = params + .map(param => param.name + (param.value ? "=" + param.value : "")) + .join(Services.appinfo.OS === "WINNT" ? "\r\n" : "\n"); + + // Fall back to raw payload. + if (!string) { + let postData = selected.requestPostData.postData.text; + string = yield gNetwork.getString(postData); + if (Services.appinfo.OS !== "WINNT") { + string = string.replace(/\r/g, ""); + } + } + + clipboardHelper.copyString(string, document); + }), + /** * Copy a cURL command from the currently selected item. */ @@ -1843,6 +1920,9 @@ RequestsMenuView.prototype = Heritage.extend(WidgetMethods, { let copyUrlParamsElement = $("#request-menu-context-copy-url-params"); copyUrlParamsElement.hidden = !selectedItem || !nsIURL(selectedItem.attachment.url).query; + let copyPostDataElement = $("#request-menu-context-copy-post-data"); + copyPostDataElement.hidden = !selectedItem || !selectedItem.attachment.requestPostData; + let copyAsCurlElement = $("#request-menu-context-copy-as-curl"); copyAsCurlElement.hidden = !selectedItem || !selectedItem.attachment.responseContent; @@ -2450,19 +2530,19 @@ NetworkDetailsView.prototype = { /** * Sets the network request headers shown in this view. * - * @param object aHeadersResponse + * @param object aHeaders * The "requestHeaders" message received from the server. - * @param object aHeadersFromUploadStream + * @param object aUploadHeaders * The "requestHeadersFromUploadStream" inferred from the POST payload. * @return object * A promise that resolves when request headers are set. */ - _setRequestHeaders: Task.async(function*(aHeadersResponse, aHeadersFromUploadStream) { - if (aHeadersResponse && aHeadersResponse.headers.length) { - yield this._addHeaders(this._requestHeaders, aHeadersResponse); + _setRequestHeaders: Task.async(function*(aHeaders, aUploadHeaders) { + if (aHeaders && aHeaders.headers.length) { + yield this._addHeaders(this._requestHeaders, aHeaders); } - if (aHeadersFromUploadStream && aHeadersFromUploadStream.headers.length) { - yield this._addHeaders(this._requestHeadersFromUpload, aHeadersFromUploadStream); + if (aUploadHeaders && aUploadHeaders.headers.length) { + yield this._addHeaders(this._requestHeadersFromUpload, aUploadHeaders); } }), @@ -2590,40 +2670,28 @@ NetworkDetailsView.prototype = { /** * Sets the network request post params shown in this view. * - * @param object aHeadersResponse + * @param object aHeaders * The "requestHeaders" message received from the server. - * @param object aHeadersFromUploadStream + * @param object aUploadHeaders * The "requestHeadersFromUploadStream" inferred from the POST payload. - * @param object aPostDataResponse + * @param object aPostData * The "requestPostData" message received from the server. * @return object * A promise that is resolved when the request post params are set. */ - _setRequestPostParams: Task.async(function*(aHeadersResponse, aHeadersFromUploadStream, aPostDataResponse) { - if (!aHeadersResponse || !aHeadersFromUploadStream || !aPostDataResponse) { + _setRequestPostParams: Task.async(function*(aHeaders, aUploadHeaders, aPostData) { + if (!aHeaders || !aUploadHeaders || !aPostData) { return; } - let { headers: requestHeaders } = aHeadersResponse; - let { headers: payloadHeaders } = aHeadersFromUploadStream; - let allHeaders = [...payloadHeaders, ...requestHeaders]; + let formDataSections = yield RequestsMenuView.prototype._getFormDataSections( + aHeaders, aUploadHeaders, aPostData); - let contentTypeHeader = allHeaders.find(e => e.name.toLowerCase() == "content-type"); - let contentTypeLongString = contentTypeHeader ? contentTypeHeader.value : ""; - let postDataLongString = aPostDataResponse.postData.text; - - let postData = yield gNetwork.getString(postDataLongString); - let contentType = yield gNetwork.getString(contentTypeLongString); - - // Handle query strings (e.g. "?foo=bar&baz=42"). - if (contentType.includes("x-www-form-urlencoded")) { - for (let section of postData.split(/\r\n|\r|\n/)) { - // Before displaying it, make sure this section of the POST data - // isn't a line containing upload stream headers. - if (payloadHeaders.every(header => !section.startsWith(header.name))) { - this._addParams(this._paramsFormData, section); - } - } + // Handle urlencoded form data sections (e.g. "?foo=bar&baz=42"). + if (formDataSections.length > 0) { + formDataSections.forEach(section => { + this._addParams(this._paramsFormData, section); + }); } // Handle actual forms ("multipart/form-data" content type). else { @@ -2637,6 +2705,9 @@ NetworkDetailsView.prototype = { $("#request-post-data-textarea-box").hidden = false; let editor = yield NetMonitorView.editor("#request-post-data-textarea"); + let postDataLongString = aPostData.postData.text; + let postData = yield gNetwork.getString(postDataLongString); + // Most POST bodies are usually JSON, so they can be neatly // syntax highlighted as JS. Otheriwse, fall back to plain text. try { diff --git a/browser/devtools/netmonitor/netmonitor.xul b/browser/devtools/netmonitor/netmonitor.xul index eb478c6a7cb5..37ef9987bc97 100644 --- a/browser/devtools/netmonitor/netmonitor.xul +++ b/browser/devtools/netmonitor/netmonitor.xul @@ -32,6 +32,10 @@ label="&netmonitorUI.context.copyUrlParams;" accesskey="&netmonitorUI.context.copyUrlParams.accesskey;" oncommand="NetMonitorView.RequestsMenu.copyUrlParams();"/> + { + is(SpecialPowers.getClipboardData("text/unicode"), + aPostData, "The post data string copied from the selected item is correct."); + }); } aDebuggee.performRequests(); diff --git a/browser/locales/en-US/chrome/browser/devtools/netmonitor.dtd b/browser/locales/en-US/chrome/browser/devtools/netmonitor.dtd index 95ccfc11acef..cf579fa7e485 100644 --- a/browser/locales/en-US/chrome/browser/devtools/netmonitor.dtd +++ b/browser/locales/en-US/chrome/browser/devtools/netmonitor.dtd @@ -264,6 +264,14 @@ - for the Copy URL Parameters menu item displayed in the context menu for a request --> + + + + + + - - - -