From fe39f5ced4b397f93086951a273e53d851589dd6 Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Wed, 30 Aug 2017 17:32:20 +0200 Subject: [PATCH 01/11] Bug 1395082 - Intermittent toolkit/components/places/tests/browser/browser_visited_notfound.js. r=standard8 MozReview-Commit-ID: GzAZXNIbBCN --HG-- extra : rebase_source : 18e79867846705ad5ab79588a9f97eaa3fd37324 --- .../components/places/tests/PlacesTestUtils.jsm | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/toolkit/components/places/tests/PlacesTestUtils.jsm b/toolkit/components/places/tests/PlacesTestUtils.jsm index 3984f28de457..5892f10d4e67 100644 --- a/toolkit/components/places/tests/PlacesTestUtils.jsm +++ b/toolkit/components/places/tests/PlacesTestUtils.jsm @@ -207,14 +207,15 @@ this.PlacesTestUtils = Object.freeze({ * @resolves Returns the field value. * @rejects JavaScript exception. */ - async fieldInDB(aURI, field) { + fieldInDB(aURI, field) { let url = aURI instanceof Ci.nsIURI ? new URL(aURI.spec) : new URL(aURI); - let db = await PlacesUtils.promiseDBConnection(); - let rows = await db.executeCached( - `SELECT ${field} FROM moz_places - WHERE url_hash = hash(:url) AND url = :url`, - { url: url.href }); - return rows[0].getResultByIndex(0); + return PlacesUtils.withConnectionWrapper("PlacesTestUtils.jsm: fieldInDb", async db => { + let rows = await db.executeCached( + `SELECT ${field} FROM moz_places + WHERE url_hash = hash(:url) AND url = :url`, + { url: url.href }); + return rows[0].getResultByIndex(0); + }); }, /** From b603ba75ee32d16e6c3f116fddcdf13d19ed6f86 Mon Sep 17 00:00:00 2001 From: Markus Stange Date: Tue, 29 Aug 2017 19:13:57 +0200 Subject: [PATCH 02/11] Bug 1393046 - Enable JS sampling on the main thread as soon as possible after the profiler has started. r=njn MozReview-Commit-ID: CIj3GGI3TQo --HG-- extra : rebase_source : 5fe72df1f0433313aefa5870a63423656005cbe8 --- tools/profiler/core/platform.cpp | 57 ++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 11 deletions(-) diff --git a/tools/profiler/core/platform.cpp b/tools/profiler/core/platform.cpp index 92f5925671ae..74f3e174cd50 100644 --- a/tools/profiler/core/platform.cpp +++ b/tools/profiler/core/platform.cpp @@ -2639,6 +2639,46 @@ profiler_get_buffer_info_helper(uint32_t* aCurrentPosition, *aGeneration = ActivePS::Buffer(lock).mGeneration; } +static void +PollJSSamplingForCurrentThread() +{ + MOZ_RELEASE_ASSERT(CorePS::Exists()); + + PSAutoLock lock(gPSMutex); + + ThreadInfo* info = TLSInfo::Info(lock); + if (!info) { + return; + } + + info->PollJSSampling(); +} + +// When the profiler is started on a background thread, we can't synchronously +// call PollJSSampling on the main thread's ThreadInfo. And the next regular +// call to PollJSSampling on the main thread would only happen once the main +// thread triggers a JS interrupt callback. +// This means that all the JS execution between profiler_start() and the first +// JS interrupt would happen with JS sampling disabled, and we wouldn't get any +// JS function information for that period of time. +// So in order to start JS sampling as soon as possible, we dispatch a runnable +// to the main thread which manually calls PollJSSamplingForCurrentThread(). +// In some cases this runnable will lose the race with the next JS interrupt. +// That's fine; PollJSSamplingForCurrentThread() is immune to redundant calls. +static void +TriggerPollJSSamplingOnMainThread() +{ + nsCOMPtr mainThread; + nsresult rv = NS_GetMainThread(getter_AddRefs(mainThread)); + if (NS_SUCCEEDED(rv) && mainThread) { + nsCOMPtr task = + NS_NewRunnableFunction("TriggerPollJSSamplingOnMainThread", []() { + PollJSSamplingForCurrentThread(); + }); + SystemGroup::Dispatch(TaskCategory::Other, task.forget()); + } +} + static void locked_profiler_start(PSLockRef aLock, int aEntries, double aInterval, uint32_t aFeatures, @@ -2689,6 +2729,11 @@ locked_profiler_start(PSLockRef aLock, int aEntries, double aInterval, // We can manually poll the current thread so it starts sampling // immediately. info->PollJSSampling(); + } else if (info->IsMainThread()) { + // Dispatch a runnable to the main thread to call PollJSSampling(), + // so that we don't have wait for the next JS interrupt callback in + // order to start profiling JS. + TriggerPollJSSamplingOnMainThread(); } } } @@ -3076,17 +3121,7 @@ void profiler_js_interrupt_callback() { // This function runs on JS threads being sampled. - - MOZ_RELEASE_ASSERT(CorePS::Exists()); - - PSAutoLock lock(gPSMutex); - - ThreadInfo* info = TLSInfo::Info(lock); - if (!info) { - return; - } - - info->PollJSSampling(); + PollJSSamplingForCurrentThread(); } double From 9e55b57841d4023c08ba23a13951854a10c57ffc Mon Sep 17 00:00:00 2001 From: Luca Greco Date: Mon, 7 Aug 2017 18:54:16 +0200 Subject: [PATCH 03/11] Bug 1388098 - Fix Android options_ui on disable/enable addon. r=mixedpuppy MozReview-Commit-ID: 4z4vJpDxzGB --HG-- extra : rebase_source : e4d13535390579538aefa8cae3643fe112cfbd72 --- mobile/android/chrome/content/aboutAddons.js | 82 ++++++++++------- .../test/mochitest/test_ext_options_ui.html | 88 +++++++++++++++++++ 2 files changed, 138 insertions(+), 32 deletions(-) diff --git a/mobile/android/chrome/content/aboutAddons.js b/mobile/android/chrome/content/aboutAddons.js index 2beab6075d1d..580237ae1a04 100644 --- a/mobile/android/chrome/content/aboutAddons.js +++ b/mobile/android/chrome/content/aboutAddons.js @@ -397,11 +397,7 @@ var Addons = { // Allow the options to use all the available width space. optionsBox.classList.remove("inner"); - // WebExtensions are loaded asynchronously and the optionsURL - // may not be available via listitem when the add-on has just been - // installed, but it is available on the addon if one is set. - detailItem.setAttribute("optionsURL", addon.optionsURL); - this.createWebExtensionOptions(optionsBox, addon.optionsURL, addon.optionsBrowserStyle); + this.createWebExtensionOptions(optionsBox, addon); break; case AddonManager.OPTIONS_TYPE_TAB: // Keep the usual layout for any options related the legacy (or system) add-ons @@ -441,44 +437,58 @@ var Addons = { } button.onclick = async () => { + if (addon.isWebExtension) { + // WebExtensions are loaded asynchronously and the optionsURL + // may not be available until the addon has been started. + await addon.startupPromise; + } + const {optionsURL} = addon; openOptionsInTab(optionsURL); }; }, - createWebExtensionOptions: async function(destination, optionsURL, browserStyle) { - let originalHeight; - let frame = document.createElement("iframe"); - frame.setAttribute("id", "addon-options"); - frame.setAttribute("mozbrowser", "true"); - frame.setAttribute("style", "width: 100%; overflow: hidden;"); + createWebExtensionOptions: async function(destination, addon) { + // WebExtensions are loaded asynchronously and the optionsURL + // may not be available until the addon has been started. + await addon.startupPromise; - // Adjust iframe height to the iframe content (also between navigation of multiple options - // files). - frame.onload = (evt) => { - if (evt.target !== frame) { - return; - } + const {optionsURL, optionsBrowserStyle} = addon; + let frame = destination.querySelector("iframe#addon-options"); - const {document} = frame.contentWindow; - const bodyScrollHeight = document.body && document.body.scrollHeight; - const documentScrollHeight = document.documentElement.scrollHeight; + if (!frame) { + let originalHeight; + frame = document.createElement("iframe"); + frame.setAttribute("id", "addon-options"); + frame.setAttribute("mozbrowser", "true"); + frame.setAttribute("style", "width: 100%; overflow: hidden;"); - // Set the iframe height to the maximum between the body and the document - // scrollHeight values. - frame.style.height = Math.max(bodyScrollHeight, documentScrollHeight) + "px"; + // Adjust iframe height to the iframe content (also between navigation of multiple options + // files). + frame.onload = (evt) => { + if (evt.target !== frame) { + return; + } - // Restore the original iframe height between option page loads, - // so that we don't force the new document to have the same size - // of the previosuly loaded option page. - frame.contentWindow.addEventListener("unload", () => { - frame.style.height = originalHeight + "px"; - }, {once: true}); - }; + const {document} = frame.contentWindow; + const bodyScrollHeight = document.body && document.body.scrollHeight; + const documentScrollHeight = document.documentElement.scrollHeight; - destination.appendChild(frame); + // Set the iframe height to the maximum between the body and the document + // scrollHeight values. + frame.style.height = Math.max(bodyScrollHeight, documentScrollHeight) + "px"; - originalHeight = frame.getBoundingClientRect().height; + // Restore the original iframe height between option page loads, + // so that we don't force the new document to have the same size + // of the previosuly loaded option page. + frame.contentWindow.addEventListener("unload", () => { + frame.style.height = originalHeight + "px"; + }, {once: true}); + }; + + destination.appendChild(frame); + originalHeight = frame.getBoundingClientRect().height; + } // Loading the URL this way prevents the native back // button from applying to the iframe. @@ -585,6 +595,14 @@ var Addons = { detailItem.setAttribute("opType", opType); else detailItem.removeAttribute("opType"); + + // Remove any addon options iframe if the currently selected addon has been disabled. + if (!aValue) { + const addonOptionsIframe = document.querySelector("#addon-options"); + if (addonOptionsIframe) { + addonOptionsIframe.remove(); + } + } } // Sync to the list item diff --git a/mobile/android/components/extensions/test/mochitest/test_ext_options_ui.html b/mobile/android/components/extensions/test/mochitest/test_ext_options_ui.html index a5fb5f0a1187..6bef7bef6163 100644 --- a/mobile/android/components/extensions/test/mochitest/test_ext_options_ui.html +++ b/mobile/android/components/extensions/test/mochitest/test_ext_options_ui.html @@ -84,6 +84,14 @@ function waitAboutAddonsLoaded() { return waitDOMContentLoaded(url => url === "about:addons"); } +function clickAddonDisable() { + content.document.querySelector("#disable-btn").click(); +} + +function clickAddonEnable() { + content.document.querySelector("#enable-btn").click(); +} + add_task(async function test_options_ui_iframe_height() { let addonID = "test-options-ui@mozilla.org"; @@ -406,6 +414,86 @@ add_task(async function test_options_ui_open_in_tab() { await extension.unload(); }); +add_task(async function test_options_ui_on_disable_and_enable() { + let addonID = "test-options-ui-disable-enable@mozilla.org"; + + function optionsScript() { + browser.test.sendMessage("options-page-loaded", window.location.href); + } + + let extension = ExtensionTestUtils.loadExtension({ + useAddonManager: "temporary", + manifest: { + applications: { + gecko: {id: addonID}, + }, + name: "Options UI open addon details Extension", + description: "Longer addon description", + options_ui: { + page: "options.html", + }, + }, + files: { + "options.js": optionsScript, + "options.html": ` + + + + + +

Options page

+ From 07880c5a6cc44967f57a94c7a7eb261f39fe8df4 Mon Sep 17 00:00:00 2001 From: Anthony Ramine Date: Thu, 31 Aug 2017 04:57:12 -0500 Subject: [PATCH 04/11] servo: Merge #18309 - Update the WebIDL parser (from servo:webidl); r=jdm Source-Repo: https://github.com/servo/servo Source-Revision: ed9fb55366845fdcfae9a7418acd73052a0d4757 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 2e81fbb314f2da761d3fc8993ae438bc8d8589e8 --- .../dom/bindings/codegen/parser/WebIDL.py | 39 +++++++++++++++---- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/servo/components/script/dom/bindings/codegen/parser/WebIDL.py b/servo/components/script/dom/bindings/codegen/parser/WebIDL.py index 820dc108fbfe..5045eae6493a 100644 --- a/servo/components/script/dom/bindings/codegen/parser/WebIDL.py +++ b/servo/components/script/dom/bindings/codegen/parser/WebIDL.py @@ -2064,6 +2064,9 @@ class IDLType(IDLObject): def isRecord(self): return False + def isReadableStream(self): + return False + def isArrayBuffer(self): return False @@ -2091,12 +2094,12 @@ class IDLType(IDLObject): def isSpiderMonkeyInterface(self): """ Returns a boolean indicating whether this type is an 'interface' - type that is implemented in Spidermonkey. At the moment, this - only returns true for the types from the TypedArray spec. """ + type that is implemented in SpiderMonkey. """ return self.isInterface() and (self.isArrayBuffer() or self.isArrayBufferView() or self.isSharedArrayBuffer() or - self.isTypedArray()) + self.isTypedArray() or + self.isReadableStream()) def isDictionary(self): return False @@ -2289,6 +2292,9 @@ class IDLNullableType(IDLParametrizedType): def isRecord(self): return self.inner.isRecord() + def isReadableStream(self): + return self.inner.isReadableStream() + def isArrayBuffer(self): return self.inner.isArrayBuffer() @@ -2656,6 +2662,9 @@ class IDLTypedefType(IDLType): def isRecord(self): return self.inner.isRecord() + def isReadableStream(self): + return self.inner.isReadableStream() + def isDictionary(self): return self.inner.isDictionary() @@ -2970,7 +2979,8 @@ class IDLBuiltinType(IDLType): 'Int32Array', 'Uint32Array', 'Float32Array', - 'Float64Array' + 'Float64Array', + 'ReadableStream', ) TagLookup = { @@ -3005,7 +3015,8 @@ class IDLBuiltinType(IDLType): Types.Int32Array: IDLType.Tags.interface, Types.Uint32Array: IDLType.Tags.interface, Types.Float32Array: IDLType.Tags.interface, - Types.Float64Array: IDLType.Tags.interface + Types.Float64Array: IDLType.Tags.interface, + Types.ReadableStream: IDLType.Tags.interface, } def __init__(self, location, name, type): @@ -3052,6 +3063,9 @@ class IDLBuiltinType(IDLType): return (self._typeTag >= IDLBuiltinType.Types.Int8Array and self._typeTag <= IDLBuiltinType.Types.Float64Array) + def isReadableStream(self): + return self._typeTag == IDLBuiltinType.Types.ReadableStream + def isInterface(self): # TypedArray things are interface types per the TypedArray spec, # but we handle them as builtins because SpiderMonkey implements @@ -3059,7 +3073,8 @@ class IDLBuiltinType(IDLType): return (self.isArrayBuffer() or self.isArrayBufferView() or self.isSharedArrayBuffer() or - self.isTypedArray()) + self.isTypedArray() or + self.isReadableStream()) def isNonCallbackInterface(self): # All the interfaces we can be are non-callback @@ -3129,6 +3144,7 @@ class IDLBuiltinType(IDLType): # that's not an ArrayBuffer or a callback interface (self.isArrayBuffer() and not other.isArrayBuffer()) or (self.isSharedArrayBuffer() and not other.isSharedArrayBuffer()) or + (self.isReadableStream() and not other.isReadableStream()) or # ArrayBufferView is distinguishable from everything # that's not an ArrayBufferView or typed array. (self.isArrayBufferView() and not other.isArrayBufferView() and @@ -3238,7 +3254,10 @@ BuiltinTypes = { IDLBuiltinType.Types.Float32Array), IDLBuiltinType.Types.Float64Array: IDLBuiltinType(BuiltinLocation(""), "Float64Array", - IDLBuiltinType.Types.Float64Array) + IDLBuiltinType.Types.Float64Array), + IDLBuiltinType.Types.ReadableStream: + IDLBuiltinType(BuiltinLocation(""), "ReadableStream", + IDLBuiltinType.Types.ReadableStream), } @@ -5287,7 +5306,8 @@ class Tokenizer(object): "maplike": "MAPLIKE", "setlike": "SETLIKE", "iterable": "ITERABLE", - "namespace": "NAMESPACE" + "namespace": "NAMESPACE", + "ReadableStream": "READABLESTREAM", } tokens.extend(keywords.values()) @@ -6475,6 +6495,7 @@ class Parser(Tokenizer): NonAnyType : PrimitiveType Null | ARRAYBUFFER Null | SHAREDARRAYBUFFER Null + | READABLESTREAM Null | OBJECT Null """ if p[1] == "object": @@ -6483,6 +6504,8 @@ class Parser(Tokenizer): type = BuiltinTypes[IDLBuiltinType.Types.ArrayBuffer] elif p[1] == "SharedArrayBuffer": type = BuiltinTypes[IDLBuiltinType.Types.SharedArrayBuffer] + elif p[1] == "ReadableStream": + type = BuiltinTypes[IDLBuiltinType.Types.ReadableStream] else: type = BuiltinTypes[p[1]] From b8355b02c79ffa7157dbd0d0950d016cdab9d6e3 Mon Sep 17 00:00:00 2001 From: Kartikaya Gupta Date: Wed, 30 Aug 2017 14:51:19 -0400 Subject: [PATCH 05/11] Bug 1395212 - Hoist the scroll layer deduplication code out of bindings.rs into wr::DisplayListBuilder. r=mstange This also splits the wr_dp_push_scroll_layer function in bindings.rs into two separate functions. This makes the API consistent with clipping, and also allows for optimizations in the upcoming patches. MozReview-Commit-ID: IXnOZK0dZm --HG-- extra : rebase_source : aa28875433a03ee9d6c388750f022958958d05e9 --- gfx/layers/wr/ScrollingLayersHelper.cpp | 2 +- gfx/webrender_bindings/WebRenderAPI.cpp | 23 +++++++++------ gfx/webrender_bindings/WebRenderAPI.h | 6 ++-- gfx/webrender_bindings/src/bindings.rs | 28 +++++++++---------- .../webrender_ffi_generated.h | 11 ++++++-- 5 files changed, 41 insertions(+), 29 deletions(-) diff --git a/gfx/layers/wr/ScrollingLayersHelper.cpp b/gfx/layers/wr/ScrollingLayersHelper.cpp index bfa77e89eef2..dd0ac0125822 100644 --- a/gfx/layers/wr/ScrollingLayersHelper.cpp +++ b/gfx/layers/wr/ScrollingLayersHelper.cpp @@ -177,7 +177,7 @@ ScrollingLayersHelper::DefineAndPushScrollLayers(nsDisplayItem* aItem, DefineAndPushChain(asrClippedBy, aBuilder, aStackingContext, aAppUnitsPerDevPixel, aCache); // Finally, push the ASR itself as a scroll layer. Note that the - // implementation of wr_push_scroll_layer in bindings.rs makes sure the + // implementation of PushScrollLayer in DisplayListBuilder makes sure the // scroll layer doesn't get defined multiple times so we don't need to worry // about that here. if (PushScrollLayer(metadata->GetMetrics(), aStackingContext)) { diff --git a/gfx/webrender_bindings/WebRenderAPI.cpp b/gfx/webrender_bindings/WebRenderAPI.cpp index f81fcc0b1ff5..95f398d93979 100644 --- a/gfx/webrender_bindings/WebRenderAPI.cpp +++ b/gfx/webrender_bindings/WebRenderAPI.cpp @@ -685,14 +685,21 @@ DisplayListBuilder::PushScrollLayer(const layers::FrameMetrics::ViewID& aScrollI { WRDL_LOG("PushScrollLayer id=%" PRIu64 " co=%s cl=%s\n", mWrState, aScrollId, Stringify(aContentRect).c_str(), Stringify(aClipRect).c_str()); - wr_dp_push_scroll_layer(mWrState, aScrollId, aContentRect, aClipRect); - if (!mScrollIdStack.empty()) { - auto it = mScrollParents.insert({aScrollId, mScrollIdStack.back()}); - if (!it.second) { // aScrollId was already a key in mScrollParents - // so check that the parent value is the same. - MOZ_ASSERT(it.first->second == mScrollIdStack.back()); - } + + Maybe parent = + mScrollIdStack.empty() ? Nothing() : Some(mScrollIdStack.back()); + auto it = mScrollParents.insert({aScrollId, parent}); + if (it.second) { + // An insertion took place, which means we haven't defined aScrollId before. + // So let's define it now. + wr_dp_define_scroll_layer(mWrState, aScrollId, aContentRect, aClipRect); + } else { + // aScrollId was already a key in mScrollParents so check that the parent + // value is the same. + MOZ_ASSERT(it.first->second == parent); } + + wr_dp_push_scroll_layer(mWrState, aScrollId); mScrollIdStack.push_back(aScrollId); } @@ -1011,7 +1018,7 @@ Maybe DisplayListBuilder::ParentScrollIdFor(layers::FrameMetrics::ViewID aScrollId) { auto it = mScrollParents.find(aScrollId); - return (it == mScrollParents.end() ? Nothing() : Some(it->second)); + return (it == mScrollParents.end() ? Nothing() : it->second); } } // namespace wr diff --git a/gfx/webrender_bindings/WebRenderAPI.h b/gfx/webrender_bindings/WebRenderAPI.h index 32a0543f6604..be653b1062dd 100644 --- a/gfx/webrender_bindings/WebRenderAPI.h +++ b/gfx/webrender_bindings/WebRenderAPI.h @@ -348,8 +348,10 @@ protected: std::vector mClipIdStack; std::vector mScrollIdStack; - // Track the parent scroll id of each scroll id that we encountered. - std::unordered_map mScrollParents; + // Track the parent scroll id of each scroll id that we encountered. A + // Nothing() value indicates a root scroll id. We also use this structure to + // ensure that we don't define a particular scroll layer multiple times. + std::unordered_map> mScrollParents; friend class WebRenderAPI; }; diff --git a/gfx/webrender_bindings/src/bindings.rs b/gfx/webrender_bindings/src/bindings.rs index abcfb638aaac..6adff1246f05 100644 --- a/gfx/webrender_bindings/src/bindings.rs +++ b/gfx/webrender_bindings/src/bindings.rs @@ -1,4 +1,3 @@ -use std::collections::HashSet; use std::ffi::CString; use std::{mem, slice}; use std::path::PathBuf; @@ -926,7 +925,6 @@ pub unsafe extern "C" fn wr_api_get_namespace(dh: &mut DocumentHandle) -> WrIdNa pub struct WebRenderFrameBuilder { pub root_pipeline_id: WrPipelineId, pub dl_builder: webrender_api::DisplayListBuilder, - pub scroll_clips_defined: HashSet, } impl WebRenderFrameBuilder { @@ -935,7 +933,6 @@ impl WebRenderFrameBuilder { WebRenderFrameBuilder { root_pipeline_id: root_pipeline_id, dl_builder: webrender_api::DisplayListBuilder::new(root_pipeline_id, content_size), - scroll_clips_defined: HashSet::new(), } } } @@ -1102,21 +1099,22 @@ pub extern "C" fn wr_dp_pop_clip(state: &mut WrState) { } #[no_mangle] -pub extern "C" fn wr_dp_push_scroll_layer(state: &mut WrState, - scroll_id: u64, - content_rect: LayoutRect, - clip_rect: LayoutRect) { +pub extern "C" fn wr_dp_define_scroll_layer(state: &mut WrState, + scroll_id: u64, + content_rect: LayoutRect, + clip_rect: LayoutRect) { assert!(unsafe { is_in_main_thread() }); let clip_id = ClipId::new(scroll_id, state.pipeline_id); - // Avoid defining multiple scroll clips with the same clip id, as that - // results in undefined behaviour or assertion failures. - if !state.frame_builder.scroll_clips_defined.contains(&clip_id) { + state.frame_builder.dl_builder.define_scroll_frame( + Some(clip_id), content_rect, clip_rect, vec![], None, + ScrollSensitivity::Script); +} - state.frame_builder.dl_builder.define_scroll_frame( - Some(clip_id), content_rect, clip_rect, vec![], None, - ScrollSensitivity::Script); - state.frame_builder.scroll_clips_defined.insert(clip_id); - } +#[no_mangle] +pub extern "C" fn wr_dp_push_scroll_layer(state: &mut WrState, + scroll_id: u64) { + assert!(unsafe { is_in_main_thread() }); + let clip_id = ClipId::new(scroll_id, state.pipeline_id); state.frame_builder.dl_builder.push_clip_id(clip_id); } diff --git a/gfx/webrender_bindings/webrender_ffi_generated.h b/gfx/webrender_bindings/webrender_ffi_generated.h index c5bcbfa6d3fa..a6c3d94ebb43 100644 --- a/gfx/webrender_bindings/webrender_ffi_generated.h +++ b/gfx/webrender_bindings/webrender_ffi_generated.h @@ -860,6 +860,13 @@ uint64_t wr_dp_define_clip(WrState *aState, const WrImageMask *aMask) WR_FUNC; +WR_INLINE +void wr_dp_define_scroll_layer(WrState *aState, + uint64_t aScrollId, + LayoutRect aContentRect, + LayoutRect aClipRect) +WR_FUNC; + WR_INLINE void wr_dp_end(WrState *aState) WR_FUNC; @@ -1027,9 +1034,7 @@ WR_FUNC; WR_INLINE void wr_dp_push_scroll_layer(WrState *aState, - uint64_t aScrollId, - LayoutRect aContentRect, - LayoutRect aClipRect) + uint64_t aScrollId) WR_FUNC; WR_INLINE From 543988e10613cb5b8d255420ae5cf4adc793085a Mon Sep 17 00:00:00 2001 From: Kartikaya Gupta Date: Wed, 30 Aug 2017 14:51:19 -0400 Subject: [PATCH 06/11] Bug 1395212 - Update wr::DisplayListBuilder to expose separate APIs for defining and pushing scroll layers. r=mstange This makes the DisplayListBuilder scrolling API more consistent with the clipping API, and allows for more optimization at the call site (in the next patch). MozReview-Commit-ID: LdCA7wkXDwF --HG-- extra : rebase_source : 1fe934e778c597f6a639ad2ecbda46995f8fd09e --- gfx/layers/wr/ScrollingLayersHelper.cpp | 13 +++++++------ gfx/layers/wr/ScrollingLayersHelper.h | 4 ++-- gfx/webrender_bindings/WebRenderAPI.cpp | 13 +++++++++---- gfx/webrender_bindings/WebRenderAPI.h | 7 ++++--- 4 files changed, 22 insertions(+), 15 deletions(-) diff --git a/gfx/layers/wr/ScrollingLayersHelper.cpp b/gfx/layers/wr/ScrollingLayersHelper.cpp index dd0ac0125822..c3b544c94756 100644 --- a/gfx/layers/wr/ScrollingLayersHelper.cpp +++ b/gfx/layers/wr/ScrollingLayersHelper.cpp @@ -51,7 +51,7 @@ ScrollingLayersHelper::ScrollingLayersHelper(WebRenderLayer* aLayer, PushLayerLocalClip(aStackingContext); } - PushScrollLayer(fm, aStackingContext); + DefineAndPushScrollLayer(fm, aStackingContext); } // The scrolled clip on the layer is "inside" all of the scrollable metadatas @@ -177,10 +177,10 @@ ScrollingLayersHelper::DefineAndPushScrollLayers(nsDisplayItem* aItem, DefineAndPushChain(asrClippedBy, aBuilder, aStackingContext, aAppUnitsPerDevPixel, aCache); // Finally, push the ASR itself as a scroll layer. Note that the - // implementation of PushScrollLayer in DisplayListBuilder makes sure the + // implementation of DefineScrollLayer in DisplayListBuilder makes sure the // scroll layer doesn't get defined multiple times so we don't need to worry // about that here. - if (PushScrollLayer(metadata->GetMetrics(), aStackingContext)) { + if (DefineAndPushScrollLayer(metadata->GetMetrics(), aStackingContext)) { mPushedClips.push_back(wr::ScrollOrClipId(scrollId)); } } @@ -228,8 +228,8 @@ ScrollingLayersHelper::DefineAndPushChain(const DisplayItemClipChain* aChain, } bool -ScrollingLayersHelper::PushScrollLayer(const FrameMetrics& aMetrics, - const StackingContextHelper& aStackingContext) +ScrollingLayersHelper::DefineAndPushScrollLayer(const FrameMetrics& aMetrics, + const StackingContextHelper& aStackingContext) { if (!aMetrics.IsScrollable()) { return false; @@ -251,9 +251,10 @@ ScrollingLayersHelper::PushScrollLayer(const FrameMetrics& aMetrics, // WebRender at all. Instead, we take the position from the composition // bounds. contentRect.MoveTo(clipBounds.TopLeft()); - mBuilder->PushScrollLayer(aMetrics.GetScrollId(), + mBuilder->DefineScrollLayer(aMetrics.GetScrollId(), aStackingContext.ToRelativeLayoutRect(contentRect), aStackingContext.ToRelativeLayoutRect(clipBounds)); + mBuilder->PushScrollLayer(aMetrics.GetScrollId()); return true; } diff --git a/gfx/layers/wr/ScrollingLayersHelper.h b/gfx/layers/wr/ScrollingLayersHelper.h index a9dd31b5b174..95b757554c7d 100644 --- a/gfx/layers/wr/ScrollingLayersHelper.h +++ b/gfx/layers/wr/ScrollingLayersHelper.h @@ -49,8 +49,8 @@ private: const StackingContextHelper& aStackingContext, int32_t aAppUnitsPerDevPixel, WebRenderLayerManager::ClipIdMap& aCache); - bool PushScrollLayer(const FrameMetrics& aMetrics, - const StackingContextHelper& aStackingContext); + bool DefineAndPushScrollLayer(const FrameMetrics& aMetrics, + const StackingContextHelper& aStackingContext); void PushLayerLocalClip(const StackingContextHelper& aStackingContext); void PushLayerClip(const LayerClip& aClip, const StackingContextHelper& aSc); diff --git a/gfx/webrender_bindings/WebRenderAPI.cpp b/gfx/webrender_bindings/WebRenderAPI.cpp index 95f398d93979..1b400b4e9d68 100644 --- a/gfx/webrender_bindings/WebRenderAPI.cpp +++ b/gfx/webrender_bindings/WebRenderAPI.cpp @@ -679,11 +679,11 @@ DisplayListBuilder::PushBuiltDisplayList(BuiltDisplayList &dl) } void -DisplayListBuilder::PushScrollLayer(const layers::FrameMetrics::ViewID& aScrollId, - const wr::LayoutRect& aContentRect, - const wr::LayoutRect& aClipRect) +DisplayListBuilder::DefineScrollLayer(const layers::FrameMetrics::ViewID& aScrollId, + const wr::LayoutRect& aContentRect, + const wr::LayoutRect& aClipRect) { - WRDL_LOG("PushScrollLayer id=%" PRIu64 " co=%s cl=%s\n", mWrState, + WRDL_LOG("DefineScrollLayer id=%" PRIu64 " co=%s cl=%s\n", mWrState, aScrollId, Stringify(aContentRect).c_str(), Stringify(aClipRect).c_str()); Maybe parent = @@ -698,7 +698,12 @@ DisplayListBuilder::PushScrollLayer(const layers::FrameMetrics::ViewID& aScrollI // value is the same. MOZ_ASSERT(it.first->second == parent); } +} +void +DisplayListBuilder::PushScrollLayer(const layers::FrameMetrics::ViewID& aScrollId) +{ + WRDL_LOG("PushScrollLayer id=%" PRIu64 "\n", mWrState, aScrollId); wr_dp_push_scroll_layer(mWrState, aScrollId); mScrollIdStack.push_back(aScrollId); } diff --git a/gfx/webrender_bindings/WebRenderAPI.h b/gfx/webrender_bindings/WebRenderAPI.h index be653b1062dd..305c4c4efb14 100644 --- a/gfx/webrender_bindings/WebRenderAPI.h +++ b/gfx/webrender_bindings/WebRenderAPI.h @@ -195,9 +195,10 @@ public: void PushBuiltDisplayList(wr::BuiltDisplayList &dl); - void PushScrollLayer(const layers::FrameMetrics::ViewID& aScrollId, - const wr::LayoutRect& aContentRect, // TODO: We should work with strongly typed rects - const wr::LayoutRect& aClipRect); + void DefineScrollLayer(const layers::FrameMetrics::ViewID& aScrollId, + const wr::LayoutRect& aContentRect, // TODO: We should work with strongly typed rects + const wr::LayoutRect& aClipRect); + void PushScrollLayer(const layers::FrameMetrics::ViewID& aScrollId); void PopScrollLayer(); void PushClipAndScrollInfo(const layers::FrameMetrics::ViewID& aScrollId, From 21ac463b1bef69fdfb810f56d2f8e8da41f4ddaa Mon Sep 17 00:00:00 2001 From: Kartikaya Gupta Date: Wed, 30 Aug 2017 14:51:19 -0400 Subject: [PATCH 07/11] Bug 1395212 - Avoid computing the entire ScrollMetadata for cases where we just need the scroll id. r=mstange MozReview-Commit-ID: BEYfq2EqIel --HG-- extra : rebase_source : 7315123162b80758be347c243375e28462352e06 --- gfx/layers/wr/ScrollingLayersHelper.cpp | 23 ++++++++++++++--------- gfx/webrender_bindings/WebRenderAPI.cpp | 6 ++++++ gfx/webrender_bindings/WebRenderAPI.h | 1 + 3 files changed, 21 insertions(+), 9 deletions(-) diff --git a/gfx/layers/wr/ScrollingLayersHelper.cpp b/gfx/layers/wr/ScrollingLayersHelper.cpp index c3b544c94756..1fae723fc785 100644 --- a/gfx/layers/wr/ScrollingLayersHelper.cpp +++ b/gfx/layers/wr/ScrollingLayersHelper.cpp @@ -146,10 +146,7 @@ ScrollingLayersHelper::DefineAndPushScrollLayers(nsDisplayItem* aItem, if (!aAsr) { return; } - Maybe metadata = aAsr->mScrollableFrame->ComputeScrollMetadata( - nullptr, aItem->ReferenceFrame(), ContainerLayerParameters(), nullptr); - MOZ_ASSERT(metadata); - FrameMetrics::ViewID scrollId = metadata->GetMetrics().GetScrollId(); + FrameMetrics::ViewID scrollId = nsLayoutUtils::ViewIDForASR(aAsr); if (aBuilder.TopmostScrollId() == scrollId) { // it's already been pushed, so we don't need to recurse any further. return; @@ -176,11 +173,19 @@ ScrollingLayersHelper::DefineAndPushScrollLayers(nsDisplayItem* aItem, // push exactly what we want. DefineAndPushChain(asrClippedBy, aBuilder, aStackingContext, aAppUnitsPerDevPixel, aCache); - // Finally, push the ASR itself as a scroll layer. Note that the - // implementation of DefineScrollLayer in DisplayListBuilder makes sure the - // scroll layer doesn't get defined multiple times so we don't need to worry - // about that here. - if (DefineAndPushScrollLayer(metadata->GetMetrics(), aStackingContext)) { + // Finally, push the ASR itself as a scroll layer. If it's already defined + // we can skip the expensive step of computing the ScrollMetadata. + bool pushed = false; + if (mBuilder->IsScrollLayerDefined(scrollId)) { + mBuilder->PushScrollLayer(scrollId); + pushed = true; + } else { + Maybe metadata = aAsr->mScrollableFrame->ComputeScrollMetadata( + nullptr, aItem->ReferenceFrame(), ContainerLayerParameters(), nullptr); + MOZ_ASSERT(metadata); + pushed = DefineAndPushScrollLayer(metadata->GetMetrics(), aStackingContext); + } + if (pushed) { mPushedClips.push_back(wr::ScrollOrClipId(scrollId)); } } diff --git a/gfx/webrender_bindings/WebRenderAPI.cpp b/gfx/webrender_bindings/WebRenderAPI.cpp index 1b400b4e9d68..578996111901 100644 --- a/gfx/webrender_bindings/WebRenderAPI.cpp +++ b/gfx/webrender_bindings/WebRenderAPI.cpp @@ -678,6 +678,12 @@ DisplayListBuilder::PushBuiltDisplayList(BuiltDisplayList &dl) &dl.dl.inner); } +bool +DisplayListBuilder::IsScrollLayerDefined(layers::FrameMetrics::ViewID aScrollId) const +{ + return mScrollParents.find(aScrollId) != mScrollParents.end(); +} + void DisplayListBuilder::DefineScrollLayer(const layers::FrameMetrics::ViewID& aScrollId, const wr::LayoutRect& aContentRect, diff --git a/gfx/webrender_bindings/WebRenderAPI.h b/gfx/webrender_bindings/WebRenderAPI.h index 305c4c4efb14..b1ff4d063c81 100644 --- a/gfx/webrender_bindings/WebRenderAPI.h +++ b/gfx/webrender_bindings/WebRenderAPI.h @@ -195,6 +195,7 @@ public: void PushBuiltDisplayList(wr::BuiltDisplayList &dl); + bool IsScrollLayerDefined(layers::FrameMetrics::ViewID aScrollId) const; void DefineScrollLayer(const layers::FrameMetrics::ViewID& aScrollId, const wr::LayoutRect& aContentRect, // TODO: We should work with strongly typed rects const wr::LayoutRect& aClipRect); From dffd9ec2a98d647585a88b8b6e9485352f801fd4 Mon Sep 17 00:00:00 2001 From: Tom Tromey Date: Wed, 30 Aug 2017 12:05:41 -0600 Subject: [PATCH 08/11] Bug 1395262 - use plain console.error in devtools promise catches; r=bgrins MozReview-Commit-ID: C8IhVPckQJ7 --HG-- extra : rebase_source : bd5f46a0bdff992cc0334250a16557eb710af5de --- .../animation-controller.js | 6 ++-- .../animationinspector/animation-panel.js | 14 +++++----- devtools/client/canvasdebugger/callslist.js | 2 +- .../client/inspector/boxmodel/box-model.js | 6 ++-- .../client/inspector/computed/computed.js | 2 +- .../client/inspector/inspector-commands.js | 4 +-- devtools/client/inspector/inspector-search.js | 2 +- devtools/client/inspector/inspector.js | 28 +++++++++---------- devtools/client/inspector/markup/markup.js | 2 +- devtools/client/inspector/rules/rules.js | 2 +- .../inspector/rules/views/rule-editor.js | 2 +- .../inspector/shared/dom-node-preview.js | 10 +++---- .../inspector/shared/highlighters-overlay.js | 2 +- devtools/client/responsive.html/manager.js | 2 +- devtools/client/scratchpad/scratchpad.js | 2 +- devtools/client/shadereditor/shadereditor.js | 2 +- .../client/shared/widgets/FilterWidget.js | 12 ++++---- .../tooltip/SwatchColorPickerTooltip.js | 2 +- devtools/client/sourceeditor/autocomplete.js | 2 +- devtools/client/styleeditor/StyleEditorUI.jsm | 6 ++-- .../client/styleeditor/StyleSheetEditor.jsm | 6 ++-- devtools/client/webconsole/console-output.js | 4 +-- ...nsole_block_mixedcontent_securityerrors.js | 2 +- .../client/webide/content/runtimedetails.js | 6 ++-- .../docs/backend/backward-compatibility.md | 2 +- .../server/actors/highlighters/eye-dropper.js | 2 +- devtools/shared/protocol.js | 2 +- 27 files changed, 67 insertions(+), 67 deletions(-) diff --git a/devtools/client/animationinspector/animation-controller.js b/devtools/client/animationinspector/animation-controller.js index af86e26cae13..f37ea8d8f122 100644 --- a/devtools/client/animationinspector/animation-controller.js +++ b/devtools/client/animationinspector/animation-controller.js @@ -64,10 +64,10 @@ var shutdown = Task.async(function* () { // This is what makes the sidebar widget able to load/unload the panel. function setPanel(panel) { - return startup(panel).catch(e => console.error(e)); + return startup(panel).catch(console.error); } function destroy() { - return shutdown().catch(e => console.error(e)); + return shutdown().catch(console.error); } /** @@ -261,7 +261,7 @@ var AnimationsController = { return this.animationsFront.toggleAll() .then(() => this.emit(this.ALL_ANIMATIONS_TOGGLED_EVENT, this)) - .catch(e => console.error(e)); + .catch(console.error); }, /** diff --git a/devtools/client/animationinspector/animation-panel.js b/devtools/client/animationinspector/animation-panel.js index a6a4e7e6cd10..272da274923c 100644 --- a/devtools/client/animationinspector/animation-panel.js +++ b/devtools/client/animationinspector/animation-panel.js @@ -179,9 +179,9 @@ var AnimationsPanel = { // the page if the selected node does not have any animation on it. if (event.keyCode === KeyCodes.DOM_VK_SPACE) { if (AnimationsController.animationPlayers.length > 0) { - this.playPauseTimeline().catch(ex => console.error(ex)); + this.playPauseTimeline().catch(console.error); } else { - this.toggleAll().catch(ex => console.error(ex)); + this.toggleAll().catch(console.error); } event.preventDefault(); } @@ -208,7 +208,7 @@ var AnimationsPanel = { }, onToggleAllClicked: function () { - this.toggleAll().catch(ex => console.error(ex)); + this.toggleAll().catch(console.error); }, /** @@ -221,7 +221,7 @@ var AnimationsPanel = { }), onTimelinePlayClicked: function () { - this.playPauseTimeline().catch(ex => console.error(ex)); + this.playPauseTimeline().catch(console.error); }, /** @@ -241,7 +241,7 @@ var AnimationsPanel = { }, onTimelineRewindClicked: function () { - this.rewindTimeline().catch(ex => console.error(ex)); + this.rewindTimeline().catch(console.error); }, /** @@ -263,7 +263,7 @@ var AnimationsPanel = { onRateChanged: function (e, rate) { AnimationsController.setPlaybackRateAll(rate) .then(() => this.refreshAnimationsStateAndUI()) - .catch(ex => console.error(ex)); + .catch(console.error); }, onTabNavigated: function () { @@ -289,7 +289,7 @@ var AnimationsPanel = { if (isUserDrag && !this.setCurrentTimeAllPromise) { this.setCurrentTimeAllPromise = AnimationsController.setCurrentTimeAll(time, true) - .catch(error => console.error(error)) + .catch(console.error) .then(() => { this.setCurrentTimeAllPromise = null; }); diff --git a/devtools/client/canvasdebugger/callslist.js b/devtools/client/canvasdebugger/callslist.js index dbe6acba25f1..990243338f6d 100644 --- a/devtools/client/canvasdebugger/callslist.js +++ b/devtools/client/canvasdebugger/callslist.js @@ -288,7 +288,7 @@ var CallsListView = Heritage.extend(WidgetMethods, { frameSnapshot.generateScreenshotFor(functionCall).then(screenshot => { this.showScreenshot(screenshot); this.highlightedThumbnail = screenshot.index; - }).catch(e => console.error(e)); + }).catch(console.error); }); }, diff --git a/devtools/client/inspector/boxmodel/box-model.js b/devtools/client/inspector/boxmodel/box-model.js index 88cc6395b85c..9d90a7228547 100644 --- a/devtools/client/inspector/boxmodel/box-model.js +++ b/devtools/client/inspector/boxmodel/box-model.js @@ -302,14 +302,14 @@ BoxModel.prototype = { properties[0].name = property.substring(9); } - session.setProperties(properties).catch(e => console.error(e)); + session.setProperties(properties).catch(console.error); }, done: (value, commit) => { editor.elt.parentNode.classList.remove("boxmodel-editing"); if (!commit) { session.revert().then(() => { session.destroy(); - }, e => console.error(e)); + }, console.error); return; } @@ -322,7 +322,7 @@ BoxModel.prototype = { autoMargins: true, }).then(layout => { this.store.dispatch(updateLayout(layout)); - }, e => console.error(e)); + }, console.error); }, cssProperties: getCssProperties(this.inspector.toolbox) }, event); diff --git a/devtools/client/inspector/computed/computed.js b/devtools/client/inspector/computed/computed.js index f5dd5d0ab2bc..ba87aa7976fb 100644 --- a/devtools/client/inspector/computed/computed.js +++ b/devtools/client/inspector/computed/computed.js @@ -509,7 +509,7 @@ CssComputedView.prototype = { ); this._refreshProcess.schedule(); }); - }).catch((err) => console.error(err)); + }).catch(console.error); }, /** diff --git a/devtools/client/inspector/inspector-commands.js b/devtools/client/inspector/inspector-commands.js index ff26e4b94919..9c1e0cfd0ef1 100644 --- a/devtools/client/inspector/inspector-commands.js +++ b/devtools/client/inspector/inspector-commands.js @@ -58,7 +58,7 @@ exports.items = [{ }], exec: function* (args, context) { if (args.hide) { - context.updateExec("eyedropper_server_hide").catch(e => console.error(e)); + context.updateExec("eyedropper_server_hide").catch(console.error); return; } @@ -74,7 +74,7 @@ exports.items = [{ let telemetry = new Telemetry(); telemetry.toolOpened(args.frommenu ? "menueyedropper" : "eyedropper"); - context.updateExec("eyedropper_server").catch(e => console.error(e)); + context.updateExec("eyedropper_server").catch(console.error); } }, { item: "command", diff --git a/devtools/client/inspector/inspector-search.js b/devtools/client/inspector/inspector-search.js index 242235f17939..95571f72d237 100644 --- a/devtools/client/inspector/inspector-search.js +++ b/devtools/client/inspector/inspector-search.js @@ -72,7 +72,7 @@ InspectorSearch.prototype = { _onSearch: function (reverse = false) { this.doFullTextSearch(this.searchBox.value, reverse) - .catch(e => console.error(e)); + .catch(console.error); }, doFullTextSearch: Task.async(function* (query, reverse) { diff --git a/devtools/client/inspector/inspector.js b/devtools/client/inspector/inspector.js index 1fd676f12fa8..3954fd5fdcae 100644 --- a/devtools/client/inspector/inspector.js +++ b/devtools/client/inspector/inspector.js @@ -224,13 +224,13 @@ Inspector.prototype = { return promise.all([ this._target.actorHasMethod("domwalker", "duplicateNode").then(value => { this._supportsDuplicateNode = value; - }).catch(e => console.error(e)), + }).catch(console.error), this._target.actorHasMethod("domnode", "scrollIntoView").then(value => { this._supportsScrollIntoView = value; - }).catch(e => console.error(e)), + }).catch(console.error), this._target.actorHasMethod("inspector", "resolveRelativeURL").then(value => { this._supportsResolveRelativeURL = value; - }).catch(e => console.error(e)), + }).catch(console.error), ]); }); }, @@ -1627,7 +1627,7 @@ Inspector.prototype = { this.eyeDropperButton.classList.add("checked"); this.startEyeDropperListeners(); return this.inspector.pickColorFromPage(this.toolbox, {copyOnSelect: true}) - .catch(e => console.error(e)); + .catch(console.error); }, /** @@ -1644,7 +1644,7 @@ Inspector.prototype = { this.eyeDropperButton.classList.remove("checked"); this.stopEyeDropperListeners(); return this.inspector.cancelPickColorFromPage() - .catch(e => console.error(e)); + .catch(console.error); }, /** @@ -1839,7 +1839,7 @@ Inspector.prototype = { _copyLongString: function (longStringActorPromise) { return this._getLongString(longStringActorPromise).then(string => { clipboardHelper.copyString(string); - }).catch(e => console.error(e)); + }).catch(console.error); }, /** @@ -1851,10 +1851,10 @@ Inspector.prototype = { _getLongString: function (longStringActorPromise) { return longStringActorPromise.then(longStringActor => { return longStringActor.string().then(string => { - longStringActor.release().catch(e => console.error(e)); + longStringActor.release().catch(console.error); return string; }); - }).catch(e => console.error(e)); + }).catch(console.error); }, /** @@ -1868,7 +1868,7 @@ Inspector.prototype = { this.telemetry.toolOpened("copyuniquecssselector"); this.selection.nodeFront.getUniqueSelector().then(selector => { clipboardHelper.copyString(selector); - }).catch(e => console.error); + }).catch(console.error); }, /** @@ -1882,7 +1882,7 @@ Inspector.prototype = { this.telemetry.toolOpened("copyfullcssselector"); this.selection.nodeFront.getCssPath().then(path => { clipboardHelper.copyString(path); - }).catch(e => console.error); + }).catch(console.error); }, /** @@ -1896,7 +1896,7 @@ Inspector.prototype = { this.telemetry.toolOpened("copyxpath"); this.selection.nodeFront.getXPath().then(path => { clipboardHelper.copyString(path); - }).catch(e => console.error); + }).catch(console.error); }, /** @@ -1942,7 +1942,7 @@ Inspector.prototype = { selection.isPseudoElementNode()) { return; } - this.walker.duplicateNode(selection.nodeFront).catch(e => console.error(e)); + this.walker.duplicateNode(selection.nodeFront).catch(console.error); }, /** @@ -2043,7 +2043,7 @@ Inspector.prototype = { return this.toolbox.viewSourceInDebugger(url); } return null; - }).catch(e => console.error(e)); + }).catch(console.error); } else if (type == "idref") { // Select the node in the same document. this.walker.document(this.selection.nodeFront).then(doc => { @@ -2054,7 +2054,7 @@ Inspector.prototype = { } this.selection.setNodeFront(node); }); - }).catch(e => console.error(e)); + }).catch(console.error); } }, diff --git a/devtools/client/inspector/markup/markup.js b/devtools/client/inspector/markup/markup.js index 6f8e032caf0b..e297db415df7 100644 --- a/devtools/client/inspector/markup/markup.js +++ b/devtools/client/inspector/markup/markup.js @@ -184,7 +184,7 @@ MarkupView.prototype = { _onToolboxPickerHover: function (event, nodeFront) { this.showNode(nodeFront).then(() => { this._showContainerAsHovered(nodeFront); - }, e => console.error(e)); + }, console.error); }, /** diff --git a/devtools/client/inspector/rules/rules.js b/devtools/client/inspector/rules/rules.js index 0a6eb5a47c55..fa04e3be933c 100644 --- a/devtools/client/inspector/rules/rules.js +++ b/devtools/client/inspector/rules/rules.js @@ -897,7 +897,7 @@ CssRuleView.prototype = { // Notify anyone that cares that we refreshed. return onEditorsReady.then(() => { this.emit("ruleview-refreshed"); - }, e => console.error(e)); + }, console.error); }).catch(promiseWarn); }, diff --git a/devtools/client/inspector/rules/views/rule-editor.js b/devtools/client/inspector/rules/views/rule-editor.js index b6e6a64d5018..37612ac0e94a 100644 --- a/devtools/client/inspector/rules/views/rule-editor.js +++ b/devtools/client/inspector/rules/views/rule-editor.js @@ -275,7 +275,7 @@ RuleEditor.prototype = { this.rule.getOriginalSourceStrings().then((strings) => { sourceLabel.textContent = strings.short; sourceLabel.setAttribute("title", strings.full); - }, e => console.error(e)).then(() => { + }, console.error).then(() => { this.emit("source-link-updated"); }); } else { diff --git a/devtools/client/inspector/shared/dom-node-preview.js b/devtools/client/inspector/shared/dom-node-preview.js index 981802dcca87..3feec7971afc 100644 --- a/devtools/client/inspector/shared/dom-node-preview.js +++ b/devtools/client/inspector/shared/dom-node-preview.js @@ -196,7 +196,7 @@ DomNodePreview.prototype = { }, destroy: function () { - HighlighterLock.unhighlight().catch(e => console.error(e)); + HighlighterLock.unhighlight().catch(console.error); this.stopListeners(); @@ -218,7 +218,7 @@ DomNodePreview.prototype = { return; } this.highlighterUtils.highlightNodeFront(this.nodeFront) - .catch(e => console.error(e)); + .catch(console.error); }, onPreviewMouseOut: function () { @@ -226,7 +226,7 @@ DomNodePreview.prototype = { return; } this.highlighterUtils.unhighlight() - .catch(e => console.error(e)); + .catch(console.error); }, onSelectElClick: function () { @@ -246,12 +246,12 @@ DomNodePreview.prototype = { classList.remove("selected"); HighlighterLock.unhighlight().then(() => { this.emit("target-highlighter-unlocked"); - }, error => console.error(error)); + }, console.error); } else { classList.add("selected"); HighlighterLock.highlight(this).then(() => { this.emit("target-highlighter-locked"); - }, error => console.error(error)); + }, console.error); } }, diff --git a/devtools/client/inspector/shared/highlighters-overlay.js b/devtools/client/inspector/shared/highlighters-overlay.js index 9016ccade521..dcb0e52ebd1e 100644 --- a/devtools/client/inspector/shared/highlighters-overlay.js +++ b/devtools/client/inspector/shared/highlighters-overlay.js @@ -535,7 +535,7 @@ HighlightersOverlay.prototype = { // whether the result is truthy before installing the handler. let onHidden = this.highlighters[this.hoveredHighlighterShown].hide(); if (onHidden) { - onHidden.catch(e => console.error(e)); + onHidden.catch(console.error); } this.hoveredHighlighterShown = null; diff --git a/devtools/client/responsive.html/manager.js b/devtools/client/responsive.html/manager.js index 82abfd17a20a..ff602c5ea6cf 100644 --- a/devtools/client/responsive.html/manager.js +++ b/devtools/client/responsive.html/manager.js @@ -189,7 +189,7 @@ const ResponsiveUIManager = exports.ResponsiveUIManager = { break; default: } - completed.catch(e => console.error(e)); + completed.catch(console.error); }, handleMenuCheck({target}) { diff --git a/devtools/client/scratchpad/scratchpad.js b/devtools/client/scratchpad/scratchpad.js index 336929ad4f12..2ec6cb72ad31 100644 --- a/devtools/client/scratchpad/scratchpad.js +++ b/devtools/client/scratchpad/scratchpad.js @@ -1736,7 +1736,7 @@ var Scratchpad = { this.populateRecentFilesMenu(); PreferenceObserver.init(); CloseObserver.init(); - }).catch((err) => console.error(err)); + }).catch(console.error); this._setupCommandListeners(); this._updateViewMenuItems(); this._setupPopupShowingListeners(); diff --git a/devtools/client/shadereditor/shadereditor.js b/devtools/client/shadereditor/shadereditor.js index cb2cf1a3f81c..16c714031bf6 100644 --- a/devtools/client/shadereditor/shadereditor.js +++ b/devtools/client/shadereditor/shadereditor.js @@ -314,7 +314,7 @@ var ShadersListView = Heritage.extend(WidgetMethods, { getShaders() .then(getSources) .then(showSources) - .catch(e => console.error(e)); + .catch(console.error); }, /** diff --git a/devtools/client/shared/widgets/FilterWidget.js b/devtools/client/shared/widgets/FilterWidget.js index 013d504ec8aa..93de0f8cd163 100644 --- a/devtools/client/shared/widgets/FilterWidget.js +++ b/devtools/client/shared/widgets/FilterWidget.js @@ -575,7 +575,7 @@ CSSFilterEditorWidget.prototype = { // If the click happened on the remove button. presets.splice(id, 1); this.setPresets(presets).then(this.renderPresets, - ex => console.error(ex)); + console.error); } else { // Or if the click happened on a preset. let p = presets[id]; @@ -583,7 +583,7 @@ CSSFilterEditorWidget.prototype = { this.setCssValue(p.value); this.addPresetInput.value = p.name; } - }, ex => console.error(ex)); + }, console.error); }, _togglePresets: function () { @@ -612,8 +612,8 @@ CSSFilterEditorWidget.prototype = { } this.setPresets(presets).then(this.renderPresets, - ex => console.error(ex)); - }, ex => console.error(ex)); + console.error); + }, console.error); }, /** @@ -952,12 +952,12 @@ CSSFilterEditorWidget.prototype = { } return presets; - }, e => console.error(e)); + }, console.error); }, setPresets: function (presets) { return asyncStorage.setItem("cssFilterPresets", presets) - .catch(e => console.error(e)); + .catch(console.error); } }; diff --git a/devtools/client/shared/widgets/tooltip/SwatchColorPickerTooltip.js b/devtools/client/shared/widgets/tooltip/SwatchColorPickerTooltip.js index 9606e4a25556..c52c7dddc408 100644 --- a/devtools/client/shared/widgets/tooltip/SwatchColorPickerTooltip.js +++ b/devtools/client/shared/widgets/tooltip/SwatchColorPickerTooltip.js @@ -171,7 +171,7 @@ SwatchColorPickerTooltip.prototype = extend(SwatchBasedEditorTooltip.prototype, this.hide(); this.tooltip.emit("eyedropper-opened"); - }, e => console.error(e)); + }, console.error); inspector.once("color-picked", color => { toolbox.win.focus(); diff --git a/devtools/client/sourceeditor/autocomplete.js b/devtools/client/sourceeditor/autocomplete.js index 84ed40967c50..341214628cd9 100644 --- a/devtools/client/sourceeditor/autocomplete.js +++ b/devtools/client/sourceeditor/autocomplete.js @@ -237,7 +237,7 @@ function autoComplete({ ed, cm }) { }); popup.openPopup(cursorElement, -1 * left, 0); autocompleteOpts.suggestionInsertedOnce = false; - }).catch(e => console.error(e)); + }).catch(console.error); } /** diff --git a/devtools/client/styleeditor/StyleEditorUI.jsm b/devtools/client/styleeditor/StyleEditorUI.jsm index e7ff43e4c4ef..946133e1ae48 100644 --- a/devtools/client/styleeditor/StyleEditorUI.jsm +++ b/devtools/client/styleeditor/StyleEditorUI.jsm @@ -234,7 +234,7 @@ StyleEditorUI.prototype = { _onNewDocument: function () { this._debuggee.getStyleSheets().then((styleSheets) => { return this._resetStyleSheetList(styleSheets); - }).catch(e => console.error(e)); + }).catch(console.error); }, /** @@ -634,7 +634,7 @@ StyleEditorUI.prototype = { this.emit("error", { key: "error-compressed", level: "info" }); } } - }.bind(this)).catch(e => console.error(e)); + }.bind(this)).catch(console.error); } }); }, @@ -919,7 +919,7 @@ StyleEditorUI.prototype = { sidebar.hidden = !showSidebar || !inSource; this.emit("media-list-changed", editor); - }.bind(this)).catch(e => console.error(e)); + }.bind(this)).catch(console.error); }, /** diff --git a/devtools/client/styleeditor/StyleSheetEditor.jsm b/devtools/client/styleeditor/StyleSheetEditor.jsm index 5d4761c07a99..141b231ae694 100644 --- a/devtools/client/styleeditor/StyleSheetEditor.jsm +++ b/devtools/client/styleeditor/StyleSheetEditor.jsm @@ -127,7 +127,7 @@ function StyleSheetEditor(styleSheet, win, file, isNew, walker, highlighter) { this.mediaRules = []; if (this.cssSheet.getMediaRules) { this.cssSheet.getMediaRules().then(this._onMediaRulesChanged, - e => console.error(e)); + console.error); } this.cssSheet.on("media-rules-changed", this._onMediaRulesChanged); this.cssSheet.on("style-applied", this._onStyleApplied); @@ -518,7 +518,7 @@ StyleSheetEditor.prototype = { * Toggled the disabled state of the underlying stylesheet. */ toggleDisabled: function () { - this.styleSheet.toggleDisabled().catch(e => console.error(e)); + this.styleSheet.toggleDisabled().catch(console.error); }, /** @@ -560,7 +560,7 @@ StyleSheetEditor.prototype = { this._isUpdating = true; this.styleSheet.update(this._state.text, this.transitionsEnabled) - .catch(e => console.error(e)); + .catch(console.error); }, /** diff --git a/devtools/client/webconsole/console-output.js b/devtools/client/webconsole/console-output.js index 9bfd579adfae..ede99a32b91e 100644 --- a/devtools/client/webconsole/console-output.js +++ b/devtools/client/webconsole/console-output.js @@ -3072,7 +3072,7 @@ Widgets.ObjectRenderers.add({ // the message is destroyed. this.message.widgets.add(this); - this.linkToInspector().catch(e => console.error(e)); + this.linkToInspector().catch(console.error); }, /** @@ -3160,7 +3160,7 @@ Widgets.ObjectRenderers.add({ unhighlightDomNode: function () { return this.linkToInspector().then(() => { return this.toolbox.highlighterUtils.unhighlight(); - }).catch(e => console.error(e)); + }).catch(console.error); }, /** diff --git a/devtools/client/webconsole/test/browser_webconsole_block_mixedcontent_securityerrors.js b/devtools/client/webconsole/test/browser_webconsole_block_mixedcontent_securityerrors.js index be579ab9a15d..22340a6d8ec0 100644 --- a/devtools/client/webconsole/test/browser_webconsole_block_mixedcontent_securityerrors.js +++ b/devtools/client/webconsole/test/browser_webconsole_block_mixedcontent_securityerrors.js @@ -97,7 +97,7 @@ function mixedContentOverrideTest2(hud, browser) { objects: true, }, ], - }).then(msgs => deferred.resolve(msgs), e => console.error(e)); + }).then(msgs => deferred.resolve(msgs), console.error); return deferred.promise; } diff --git a/devtools/client/webide/content/runtimedetails.js b/devtools/client/webide/content/runtimedetails.js index 80f54e5ff804..2e696027b6c3 100644 --- a/devtools/client/webide/content/runtimedetails.js +++ b/devtools/client/webide/content/runtimedetails.js @@ -102,7 +102,7 @@ function CheckLockState() { adbCheckResult.textContent = sNo; adbRootAction.removeAttribute("hidden"); } - }, e => console.error(e)); + }, console.error); } else { adbCheckResult.textContent = sUnknown; } @@ -120,7 +120,7 @@ function CheckLockState() { } else { devtoolsCheckResult.textContent = sYes; } - }, e => console.error(e)); + }, console.error); } catch (e) { // Exception. pref actor is only accessible if forbird-certified-apps is false devtoolsCheckResult.textContent = sNo; @@ -147,5 +147,5 @@ function EnableCertApps() { function RootADB() { let device = AppManager.selectedRuntime.device; - device.summonRoot().then(CheckLockState, (e) => console.error(e)); + device.summonRoot().then(CheckLockState, console.error); } diff --git a/devtools/docs/backend/backward-compatibility.md b/devtools/docs/backend/backward-compatibility.md index 7bd30a29e36a..13064a9f307c 100644 --- a/devtools/docs/backend/backward-compatibility.md +++ b/devtools/docs/backend/backward-compatibility.md @@ -43,7 +43,7 @@ The `hasActor` method returns a boolean synchronously. ```js toolbox.target.actorHasMethod("domwalker", "duplicateNode").then(hasMethod => { -}).catch(e => console.error(e)); +}).catch(console.error); ``` The `actorHasMethod` returns a promise that resolves to a boolean. diff --git a/devtools/server/actors/highlighters/eye-dropper.js b/devtools/server/actors/highlighters/eye-dropper.js index a5d6be2e17a7..e914f936d927 100644 --- a/devtools/server/actors/highlighters/eye-dropper.js +++ b/devtools/server/actors/highlighters/eye-dropper.js @@ -383,7 +383,7 @@ EyeDropper.prototype = { } this.emit("selected", toColorString(this.centerColor, this.format)); - onColorSelected.then(() => this.hide(), e => console.error(e)); + onColorSelected.then(() => this.hide(), console.error); }, /** diff --git a/devtools/shared/protocol.js b/devtools/shared/protocol.js index bd562c790913..b6d0a6e660d0 100644 --- a/devtools/shared/protocol.js +++ b/devtools/shared/protocol.js @@ -1251,7 +1251,7 @@ Front.prototype = extend(Pool.prototype, { this.actor().then(actorID => { packet.to = actorID; this.conn._transport.send(packet); - }).catch(e => console.error(e)); + }).catch(console.error); } }, From f85102e87276343f219280d9b49f6b4da815b760 Mon Sep 17 00:00:00 2001 From: Rob Wood Date: Tue, 29 Aug 2017 11:44:58 -0400 Subject: [PATCH 09/11] Bug 1390084 - Fix enabling gecko profiling on try; r=jmaher MozReview-Commit-ID: 8Cm9zH7lxGF --HG-- extra : rebase_source : 052c2c06a8c345878353770f4d576ea2cba415f9 --- .../mozharness/mozilla/testing/talos.py | 31 +++++++++++-------- .../mozharness/mozilla/testing/try_tools.py | 10 ++++-- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/testing/mozharness/mozharness/mozilla/testing/talos.py b/testing/mozharness/mozharness/mozilla/testing/talos.py index d15bb558baae..b41fa24265dc 100755 --- a/testing/mozharness/mozharness/mozilla/testing/talos.py +++ b/testing/mozharness/mozharness/mozilla/testing/talos.py @@ -217,19 +217,24 @@ class Talos(TestingMixin, MercurialScript, BlobUploadMixin, TooltoolMixin, opts = None if opts: - # In the case of a multi-line commit message, only examine - # the first line for mozharness options - opts = opts.split('\n')[0] - opts = re.sub(r'\w+:.*', '', opts).strip().split(' ') - if "--geckoProfile" in opts: - # overwrite whatever was set here. - self.gecko_profile = True - try: - idx = opts.index('--geckoProfileInterval') - if len(opts) > idx + 1: - self.gecko_profile_interval = opts[idx + 1] - except ValueError: - pass + # In the case of a multi-line commit message, only examine + # the first line for mozharness options + opts = opts.split('\n')[0] + opts = re.sub(r'\w+:.*', '', opts).strip().split(' ') + if "--geckoProfile" in opts: + # overwrite whatever was set here. + self.gecko_profile = True + try: + idx = opts.index('--geckoProfileInterval') + if len(opts) > idx + 1: + self.gecko_profile_interval = opts[idx + 1] + except ValueError: + pass + else: + # no opts, check for '--geckoProfile' in try message text directly + if self.try_message_has_flag('geckoProfile'): + self.gecko_profile = True + # finally, if gecko_profile is set, we add that to the talos options if self.gecko_profile: gecko_results.append('--geckoProfile') diff --git a/testing/mozharness/mozharness/mozilla/testing/try_tools.py b/testing/mozharness/mozharness/mozilla/testing/try_tools.py index 58efc0ecf1af..583ba5b217ed 100644 --- a/testing/mozharness/mozharness/mozilla/testing/try_tools.py +++ b/testing/mozharness/mozharness/mozilla/testing/try_tools.py @@ -163,8 +163,14 @@ class TryToolsMixin(TransferMixin): repo_path = None if self.buildbot_config and 'properties' in self.buildbot_config: repo_path = self.buildbot_config['properties'].get('branch') - return (self.config.get('branch', repo_path) == 'try' or - 'TRY_COMMIT_MSG' in os.environ) + get_branch = self.config.get('branch', repo_path) + if get_branch is not None: + on_try = ('try' in get_branch or 'Try' in get_branch) + elif os.environ is not None: + on_try = ('TRY_COMMIT_MSG' in os.environ) + else: + on_try = False + return on_try @PostScriptAction('download-and-extract') def set_extra_try_arguments(self, action, success=None): From 7b0b1977594894bf3fbfe27dcff47eb7a152a478 Mon Sep 17 00:00:00 2001 From: Andreas Tolfsen Date: Tue, 29 Aug 2017 17:33:38 +0100 Subject: [PATCH 10/11] Bug 1394849 - Export pprint separately. r=automatedtester pprint is currently exposed twice: once on the error namespace and once separately. We only want to expose it once, and since there are only a handful "error.pprint" usages left, we can go ahead and make this change. When we move transition to use "require" in the future, like devtools does, it will be possible to use both "error.pprint" and "pprint" styles without export duplication. MozReview-Commit-ID: CAnPDWn9Vr7 --HG-- extra : rebase_source : 05a05460d710eb96fa7b20cb94477be0282809de --- testing/marionette/action.js | 4 +-- testing/marionette/assert.js | 22 +++++++-------- testing/marionette/cookie.js | 4 +-- testing/marionette/element.js | 4 +-- testing/marionette/error.js | 15 ++++++---- testing/marionette/interaction.js | 3 +- testing/marionette/test_error.js | 46 +++++++++++++++++++++++++------ 7 files changed, 63 insertions(+), 35 deletions(-) diff --git a/testing/marionette/action.js b/testing/marionette/action.js index 4c905bbde6e3..cdc540e68f62 100644 --- a/testing/marionette/action.js +++ b/testing/marionette/action.js @@ -11,7 +11,7 @@ const {classes: Cc, interfaces: Ci, utils: Cu} = Components; Cu.import("chrome://marionette/content/assert.js"); Cu.import("chrome://marionette/content/element.js"); const { - error, + pprint, InvalidArgumentError, MoveTargetOutOfBoundsError, UnsupportedOperationError, @@ -21,8 +21,6 @@ Cu.import("chrome://marionette/content/interaction.js"); this.EXPORTED_SYMBOLS = ["action"]; -const {pprint} = error; - // TODO? With ES 2016 and Symbol you can make a safer approximation // to an enum e.g. https://gist.github.com/xmlking/e86e4f15ec32b12c4689 /** diff --git a/testing/marionette/assert.js b/testing/marionette/assert.js index 53ec83c11025..074ef12b7211 100644 --- a/testing/marionette/assert.js +++ b/testing/marionette/assert.js @@ -11,10 +11,10 @@ Cu.import("resource://gre/modules/Preferences.jsm"); Cu.import("resource://gre/modules/Services.jsm"); const { - error, InvalidArgumentError, InvalidSessionIDError, NoSuchWindowError, + pprint, UnexpectedAlertOpenError, UnsupportedOperationError, } = Cu.import("chrome://marionette/content/error.js", {}); @@ -174,7 +174,7 @@ assert.noUserPrompt = function(dialog, msg = "") { * If |obj| is not defined. */ assert.defined = function(obj, msg = "") { - msg = msg || error.pprint`Expected ${obj} to be defined`; + msg = msg || pprint`Expected ${obj} to be defined`; return assert.that(o => typeof o != "undefined", msg)(obj); }; @@ -193,7 +193,7 @@ assert.defined = function(obj, msg = "") { * If |obj| is not a number. */ assert.number = function(obj, msg = "") { - msg = msg || error.pprint`Expected ${obj} to be finite number`; + msg = msg || pprint`Expected ${obj} to be finite number`; return assert.that(Number.isFinite, msg)(obj); }; @@ -212,7 +212,7 @@ assert.number = function(obj, msg = "") { * If |obj| is not callable. */ assert.callable = function(obj, msg = "") { - msg = msg || error.pprint`${obj} is not callable`; + msg = msg || pprint`${obj} is not callable`; return assert.that(o => typeof o == "function", msg)(obj); }; @@ -231,7 +231,7 @@ assert.callable = function(obj, msg = "") { * If |obj| is not an integer. */ assert.integer = function(obj, msg = "") { - msg = msg || error.pprint`Expected ${obj} to be an integer`; + msg = msg || pprint`Expected ${obj} to be an integer`; return assert.that(Number.isInteger, msg)(obj); }; @@ -251,7 +251,7 @@ assert.integer = function(obj, msg = "") { */ assert.positiveInteger = function(obj, msg = "") { assert.integer(obj, msg); - msg = msg || error.pprint`Expected ${obj} to be >= 0`; + msg = msg || pprint`Expected ${obj} to be >= 0`; return assert.that(n => n >= 0, msg)(obj); }; @@ -270,7 +270,7 @@ assert.positiveInteger = function(obj, msg = "") { * If |obj| is not a boolean. */ assert.boolean = function(obj, msg = "") { - msg = msg || error.pprint`Expected ${obj} to be boolean`; + msg = msg || pprint`Expected ${obj} to be boolean`; return assert.that(b => typeof b == "boolean", msg)(obj); }; @@ -289,7 +289,7 @@ assert.boolean = function(obj, msg = "") { * If |obj| is not a string. */ assert.string = function(obj, msg = "") { - msg = msg || error.pprint`Expected ${obj} to be a string`; + msg = msg || pprint`Expected ${obj} to be a string`; return assert.that(s => typeof s == "string", msg)(obj); }; @@ -308,7 +308,7 @@ assert.string = function(obj, msg = "") { * If |obj| is not an object. */ assert.object = function(obj, msg = "") { - msg = msg || error.pprint`Expected ${obj} to be an object`; + msg = msg || pprint`Expected ${obj} to be an object`; return assert.that(o => { // unable to use instanceof because LHS and RHS may come from // different globals @@ -335,7 +335,7 @@ assert.object = function(obj, msg = "") { */ assert.in = function(prop, obj, msg = "") { assert.object(obj, msg); - msg = msg || error.pprint`Expected ${prop} in ${obj}`; + msg = msg || pprint`Expected ${prop} in ${obj}`; assert.that(p => obj.hasOwnProperty(p), msg)(prop); return obj[prop]; }; @@ -355,7 +355,7 @@ assert.in = function(prop, obj, msg = "") { * If |obj| is not an Array. */ assert.array = function(obj, msg = "") { - msg = msg || error.pprint`Expected ${obj} to be an Array`; + msg = msg || pprint`Expected ${obj} to be an Array`; return assert.that(Array.isArray, msg)(obj); }; diff --git a/testing/marionette/cookie.js b/testing/marionette/cookie.js index 1252a0678453..219c52dcaab2 100644 --- a/testing/marionette/cookie.js +++ b/testing/marionette/cookie.js @@ -10,8 +10,8 @@ Cu.import("resource://gre/modules/Services.jsm"); Cu.import("chrome://marionette/content/assert.js"); const { - error, InvalidCookieDomainError, + pprint, } = Cu.import("chrome://marionette/content/error.js", {}); this.EXPORTED_SYMBOLS = ["cookie"]; @@ -53,7 +53,7 @@ this.cookie = { cookie.fromJSON = function(json) { let newCookie = {}; - assert.object(json, error.pprint`Expected cookie object, got ${json}`); + assert.object(json, pprint`Expected cookie object, got ${json}`); newCookie.name = assert.string(json.name, "Cookie name must be string"); newCookie.value = assert.string(json.value, "Cookie value must be string"); diff --git a/testing/marionette/element.js b/testing/marionette/element.js index cfd64190c80f..0ee217fd3613 100644 --- a/testing/marionette/element.js +++ b/testing/marionette/element.js @@ -12,10 +12,10 @@ Cu.import("resource://gre/modules/Log.jsm"); Cu.import("chrome://marionette/content/assert.js"); Cu.import("chrome://marionette/content/atom.js"); const { - error, InvalidSelectorError, JavaScriptError, NoSuchElementError, + pprint, StaleElementReferenceError, } = Cu.import("chrome://marionette/content/error.js", {}); Cu.import("chrome://marionette/content/wait.js"); @@ -180,7 +180,7 @@ element.Store = class { if (element.isStale(el)) { throw new StaleElementReferenceError( - error.pprint`The element reference of ${el} stale; ` + + pprint`The element reference of ${el} stale; ` + "either the element is no longer attached to the DOM " + "or the document has been refreshed"); } diff --git a/testing/marionette/error.js b/testing/marionette/error.js index 995a26a4950a..25a6d0bc6da9 100644 --- a/testing/marionette/error.js +++ b/testing/marionette/error.js @@ -45,7 +45,10 @@ const BUILTIN_ERRORS = new Set([ "URIError", ]); -this.EXPORTED_SYMBOLS = ["error", "error.pprint"].concat(Array.from(ERRORS)); +this.EXPORTED_SYMBOLS = [ + "error", + "pprint", +].concat(Array.from(ERRORS)); /** @namespace */ this.error = {}; @@ -158,7 +161,7 @@ error.stringify = function(err) { * pprint`Expected element ${htmlElement}`; * => 'Expected element ' */ -error.pprint = function(ss, ...values) { +this.pprint = function(ss, ...values) { function prettyObject(obj) { let proto = Object.prototype.toString.call(obj); let s = ""; @@ -305,17 +308,17 @@ class ElementClickInterceptedError extends WebDriverError { switch (obscuredEl.style.pointerEvents) { case "none": - msg = error.pprint`Element ${obscuredEl} is not clickable ` + + msg = pprint`Element ${obscuredEl} is not clickable ` + `at point (${coords.x},${coords.y}) ` + `because it does not have pointer events enabled, ` + - error.pprint`and element ${overlayingEl} ` + + pprint`and element ${overlayingEl} ` + `would receive the click instead`; break; default: - msg = error.pprint`Element ${obscuredEl} is not clickable ` + + msg = pprint`Element ${obscuredEl} is not clickable ` + `at point (${coords.x},${coords.y}) ` + - error.pprint`because another element ${overlayingEl} ` + + pprint`because another element ${overlayingEl} ` + `obscures it`; break; } diff --git a/testing/marionette/interaction.js b/testing/marionette/interaction.js index d296a7c105e8..7ee69c40d387 100644 --- a/testing/marionette/interaction.js +++ b/testing/marionette/interaction.js @@ -11,7 +11,6 @@ Cu.import("chrome://marionette/content/atom.js"); const { ElementClickInterceptedError, ElementNotInteractableError, - error, InvalidArgument, InvalidArgumentError, InvalidElementStateError, @@ -178,7 +177,7 @@ async function webdriverClickElement(el, a11y) { // there is no point in checking if it is pointer-interactable if (!element.isInView(containerEl)) { throw new ElementNotInteractableError( - error.pprint`Element ${el} could not be scrolled into view`); + pprint`Element ${el} could not be scrolled into view`); } // step 7 diff --git a/testing/marionette/test_error.js b/testing/marionette/test_error.js index 279b5dd89829..3fe04565a923 100644 --- a/testing/marionette/test_error.js +++ b/testing/marionette/test_error.js @@ -4,7 +4,35 @@ const {utils: Cu} = Components; -Cu.import("chrome://marionette/content/error.js"); +const { + ElementClickInterceptedError, + ElementNotAccessibleError, + ElementNotInteractableError, + error, + InsecureCertificateError, + InvalidArgumentError, + InvalidCookieDomainError, + InvalidElementStateError, + InvalidSelectorError, + InvalidSessionIDError, + JavaScriptError, + MoveTargetOutOfBoundsError, + NoAlertOpenError, + NoSuchElementError, + NoSuchFrameError, + NoSuchWindowError, + pprint, + ScriptTimeoutError, + SessionNotCreatedError, + StaleElementReferenceError, + TimeoutError, + UnableToSetCookieError, + UnexpectedAlertOpenError, + UnknownCommandError, + UnknownError, + UnsupportedOperationError, + WebDriverError, +} = Cu.import("chrome://marionette/content/error.js", {}); function notok(condition) { ok(!(condition)); @@ -90,19 +118,19 @@ add_test(function test_stringify() { }); add_test(function test_pprint() { - equal('[object Object] {"foo":"bar"}', error.pprint`${{foo: "bar"}}`); + equal('[object Object] {"foo":"bar"}', pprint`${{foo: "bar"}}`); - equal("[object Number] 42", error.pprint`${42}`); - equal("[object Boolean] true", error.pprint`${true}`); - equal("[object Undefined] undefined", error.pprint`${undefined}`); - equal("[object Null] null", error.pprint`${null}`); + equal("[object Number] 42", pprint`${42}`); + equal("[object Boolean] true", pprint`${true}`); + equal("[object Undefined] undefined", pprint`${undefined}`); + equal("[object Null] null", pprint`${null}`); let complexObj = {toJSON: () => "foo"}; - equal('[object Object] "foo"', error.pprint`${complexObj}`); + equal('[object Object] "foo"', pprint`${complexObj}`); let cyclic = {}; cyclic.me = cyclic; - equal("[object Object] ", error.pprint`${cyclic}`); + equal("[object Object] ", pprint`${cyclic}`); let el = { nodeType: 1, @@ -111,7 +139,7 @@ add_test(function test_pprint() { classList: {length: 1}, className: "bar baz", }; - equal('', error.pprint`${el}`); + equal('', pprint`${el}`); run_next_test(); }); From 482df6f9bff0f516b970b1c1eabaaa235a897664 Mon Sep 17 00:00:00 2001 From: Andreas Tolfsen Date: Tue, 29 Aug 2017 17:34:37 +0100 Subject: [PATCH 11/11] Bug 1394849 - Add error.stack to create stacktraces. r=automatedtester This patch introduces a new error.stack function as a shorthand for creating stacktraces. It is equivalent to calling new Error().stack and removing the first line of the stack. Removing the first line is needed to make it appear as if the error originated from the caller's position in the program. MozReview-Commit-ID: DpSSWU5vPDm --HG-- extra : rebase_source : 52697e348367b2b7dbeb28a711a9bd1fdef076ec --- testing/marionette/error.js | 9 +++++++++ testing/marionette/test_error.js | 9 +++++++++ 2 files changed, 18 insertions(+) diff --git a/testing/marionette/error.js b/testing/marionette/error.js index 25a6d0bc6da9..243e21374558 100644 --- a/testing/marionette/error.js +++ b/testing/marionette/error.js @@ -48,6 +48,7 @@ const BUILTIN_ERRORS = new Set([ this.EXPORTED_SYMBOLS = [ "error", "pprint", + "stack", ].concat(Array.from(ERRORS)); /** @namespace */ @@ -215,6 +216,14 @@ this.pprint = function(ss, ...values) { return res.join(""); }; +/** Create a stacktrace to the current line in the program. */ +this.stack = function() { + let trace = new Error().stack; + let sa = trace.split("\n"); + sa = sa.slice(1); + return "stacktrace:\n" + sa.join("\n"); +}; + /** * WebDriverError is the prototypal parent of all WebDriver errors. * It should not be used directly, as it does not correspond to a real diff --git a/testing/marionette/test_error.js b/testing/marionette/test_error.js index 3fe04565a923..cd8923c9f128 100644 --- a/testing/marionette/test_error.js +++ b/testing/marionette/test_error.js @@ -24,6 +24,7 @@ const { pprint, ScriptTimeoutError, SessionNotCreatedError, + stack, StaleElementReferenceError, TimeoutError, UnableToSetCookieError, @@ -144,6 +145,14 @@ add_test(function test_pprint() { run_next_test(); }); +add_test(function test_stack() { + equal("string", typeof stack()); + ok(stack().includes("test_stack")); + ok(!stack().includes("add_test")); + + run_next_test(); +}); + add_test(function test_toJSON() { let e0 = new WebDriverError(); let e0s = e0.toJSON();