From d58da1866e6b6b9d221518d5661c1152384f97a1 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Tue, 21 Aug 2018 09:53:23 +0200 Subject: [PATCH 01/26] Bug 1033916 followup - Fix SpiderMonkey Rust bindings by including jsapi.h in wrapper.hpp instead of relying on jsfriendapi.h doing it. r=me --HG-- extra : rebase_source : 0d2fd16c5d5125df28bb366a9b4032a448e741f4 --- js/rust/etc/wrapper.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/js/rust/etc/wrapper.hpp b/js/rust/etc/wrapper.hpp index b25153d037cd..e42749e32841 100644 --- a/js/rust/etc/wrapper.hpp +++ b/js/rust/etc/wrapper.hpp @@ -11,6 +11,7 @@ typedef uint32_t HashNumber; +#include "jsapi.h" #include "jsfriendapi.h" #include "js/Conversions.h" #include "js/Initialization.h" From eb3df477a894d3e523c1657762dca146964816fc Mon Sep 17 00:00:00 2001 From: "Nicolas B. Pierron" Date: Wed, 25 Jul 2018 12:32:34 +0000 Subject: [PATCH 02/26] Bug 1456392 - In deterministic builds, disable baselineCompile when --fuzzing-safe is provided. r=jorendorff --- js/src/builtin/TestingFunctions.cpp | 12 +++++++++++- js/src/jit-test/tests/self-test/baselineCompile.js | 5 +++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp index d9adf5921e3a..570aaf323adb 100644 --- a/js/src/builtin/TestingFunctions.cpp +++ b/js/src/builtin/TestingFunctions.cpp @@ -5333,6 +5333,15 @@ BaselineCompile(JSContext* cx, unsigned argc, Value* vp) const char* returnedStr = nullptr; do { +#ifdef JS_MORE_DETERMINISTIC + // In order to check for differential behaviour, baselineCompile should have + // the same output whether --no-baseline is used or not. + if (fuzzingSafe) { + returnedStr = "skipped (fuzzing-safe)"; + break; + } +#endif + AutoRealm ar(cx, script); if (script->hasBaselineScript()) { if (forceDebug && !script->baselineScript()->hasDebugInstrumentation()) { @@ -6056,7 +6065,8 @@ gc::ZealModeHelpText), " that extra boilerplate is needed afterwards to cause the VM to start\n" " running the jitcode rather than staying in the interpreter:\n" " baselineCompile(); for (var i=0; i<1; i++) {} ...\n" -" The interpreter will enter the new jitcode at the loop header.\n"), +" The interpreter will enter the new jitcode at the loop header unless\n" +" baselineCompile returned a string or threw an error.\n"), JS_FS_HELP_END }; diff --git a/js/src/jit-test/tests/self-test/baselineCompile.js b/js/src/jit-test/tests/self-test/baselineCompile.js index 6ac66697458d..18da9dbf12fe 100644 --- a/js/src/jit-test/tests/self-test/baselineCompile.js +++ b/js/src/jit-test/tests/self-test/baselineCompile.js @@ -1,9 +1,10 @@ +// |jit-test| test-also=--fuzzing-safe // Check that the help text for baselineCompile() is accurate. if (typeof inJit == "function" && typeof baselineCompile == "function") { if (!inJit()) { - baselineCompile(); // compile the current script + var res = baselineCompile(); // compile the current script assertEq(inJit(), false, "We have compiled this script to baseline jitcode, but shouldn't " + @@ -13,7 +14,7 @@ if (typeof inJit == "function" && typeof baselineCompile == "function") { for (var i=0; i<1; i++) {} // exact boilerplate suggested by the help text - assertEq(inJit(), true, + assertEq(typeof res != "string" ? inJit() : true, true, "help text in TestingFunctions.cpp claims the above loop causes " + "the interpreter to start running the new baseline jitcode"); } From 8eb6087f2aaa9f252aef9b7be141040b14b2d45d Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Mon, 20 Aug 2018 21:47:21 +0200 Subject: [PATCH 03/26] Bug 1447449 - [wdspec] Re-enable mouse_pause_dblclick.py. r=ato --HG-- extra : rebase_source : 1f464998aec00cd70f2ef2d326b7a170def298c2 --- .../meta/webdriver/tests/actions/mouse_pause_dblclick.py.ini | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 testing/web-platform/meta/webdriver/tests/actions/mouse_pause_dblclick.py.ini diff --git a/testing/web-platform/meta/webdriver/tests/actions/mouse_pause_dblclick.py.ini b/testing/web-platform/meta/webdriver/tests/actions/mouse_pause_dblclick.py.ini deleted file mode 100644 index 67fd9e9a3678..000000000000 --- a/testing/web-platform/meta/webdriver/tests/actions/mouse_pause_dblclick.py.ini +++ /dev/null @@ -1,3 +0,0 @@ -[mouse_pause_dblclick.py] - disabled: - if os == "linux": https://bugzilla.mozilla.org/show_bug.cgi?id=1447449 From 3fd5e7dcbbe516aebf24ee5b448ccfda5f5160aa Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Mon, 20 Aug 2018 21:56:24 +0200 Subject: [PATCH 04/26] Bug 1447844 - [wdspec] Reenable actions/modifier_click.py. --HG-- extra : rebase_source : e65dbc53ebecf6956b276e310ba96bd73c007fb9 --- .../meta/webdriver/tests/actions/modifier_click.py.ini | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 testing/web-platform/meta/webdriver/tests/actions/modifier_click.py.ini diff --git a/testing/web-platform/meta/webdriver/tests/actions/modifier_click.py.ini b/testing/web-platform/meta/webdriver/tests/actions/modifier_click.py.ini deleted file mode 100644 index b8f3ccef4ce6..000000000000 --- a/testing/web-platform/meta/webdriver/tests/actions/modifier_click.py.ini +++ /dev/null @@ -1,3 +0,0 @@ -[modifier_click.py] - disabled: - if os == "linux": https://bugzilla.mozilla.org/show_bug.cgi?id=1447844 From cd598f0aaad45eb4ca1d70795cc0849f239c1a0e Mon Sep 17 00:00:00 2001 From: Jorg K Date: Tue, 21 Aug 2018 00:28:00 +0300 Subject: [PATCH 05/26] Bug 1484809 - Put class nsRelativeFilePref into its own include file. r=njn --HG-- extra : rebase_source : c775cdde24c8aa7134a7dd121abaf9233c8e2729 --- modules/libpref/Preferences.cpp | 16 +------------ modules/libpref/moz.build | 1 + modules/libpref/nsRelativeFilePref.h | 34 ++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 modules/libpref/nsRelativeFilePref.h diff --git a/modules/libpref/Preferences.cpp b/modules/libpref/Preferences.cpp index 7338a55e11aa..65e1c0d6b5b2 100644 --- a/modules/libpref/Preferences.cpp +++ b/modules/libpref/Preferences.cpp @@ -71,6 +71,7 @@ #include "nsQuickSort.h" #include "nsReadableUtils.h" #include "nsRefPtrHashtable.h" +#include "nsRelativeFilePref.h" #include "nsString.h" #include "nsTArray.h" #include "nsThreadUtils.h" @@ -2309,21 +2310,6 @@ private: nsCOMPtr mUnicodeString; }; -class nsRelativeFilePref final : public nsIRelativeFilePref -{ -public: - NS_DECL_ISUPPORTS - NS_DECL_NSIRELATIVEFILEPREF - - nsRelativeFilePref(); - -private: - virtual ~nsRelativeFilePref(); - - nsCOMPtr mFile; - nsCString mRelativeToKey; -}; - //---------------------------------------------------------------------------- // nsPrefBranch //---------------------------------------------------------------------------- diff --git a/modules/libpref/moz.build b/modules/libpref/moz.build index 017dd86591c6..5ad7e787edf2 100644 --- a/modules/libpref/moz.build +++ b/modules/libpref/moz.build @@ -26,6 +26,7 @@ XPIDL_MODULE = 'pref' EXPORTS.mozilla += [ 'init/StaticPrefList.h', + 'nsRelativeFilePref.h', 'Preferences.h', 'StaticPrefs.h', ] diff --git a/modules/libpref/nsRelativeFilePref.h b/modules/libpref/nsRelativeFilePref.h new file mode 100644 index 000000000000..73d05b786747 --- /dev/null +++ b/modules/libpref/nsRelativeFilePref.h @@ -0,0 +1,34 @@ +/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* 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/. */ + +#ifndef mozilla_nsRelativeFilePref_h +#define mozilla_nsRelativeFilePref_h + +#include "nsCOMPtr.h" +#include "nsIFile.h" +#include "nsString.h" + +// Note: This class is in its own file because it is needed by Mailnews. + +namespace mozilla { + +class nsRelativeFilePref final : public nsIRelativeFilePref +{ +public: + NS_DECL_ISUPPORTS + NS_DECL_NSIRELATIVEFILEPREF + + nsRelativeFilePref(); + +private: + virtual ~nsRelativeFilePref(); + + nsCOMPtr mFile; + nsCString mRelativeToKey; +}; + +} // namespace mozilla + +#endif // mozilla_nsRelativeFilePref_h From 0d407d478e37ec03a31ac0aae3f7bf04b7dc9c09 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 7 Aug 2018 16:04:58 +0200 Subject: [PATCH 06/26] Bug 1437065: Have LookupCode reset the codeRange out-param if it didn't find a Code; r=luke --HG-- extra : rebase_source : a27b87eddcf9c65aeddc80688da7e81ba148a3f9 extra : histedit_source : 14ec6236c078a7e767de45720eef151c63884029 --- js/src/wasm/WasmProcess.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/js/src/wasm/WasmProcess.cpp b/js/src/wasm/WasmProcess.cpp index b1ed99a2c6b5..5214fb28e15a 100644 --- a/js/src/wasm/WasmProcess.cpp +++ b/js/src/wasm/WasmProcess.cpp @@ -229,23 +229,26 @@ wasm::UnregisterCodeSegment(const CodeSegment* cs) } const CodeSegment* -wasm::LookupCodeSegment(const void* pc, const CodeRange** cr /*= nullptr */) +wasm::LookupCodeSegment(const void* pc, const CodeRange** codeRange /*= nullptr */) { if (const CodeSegment* found = processCodeSegmentMap.lookup(pc)) { - if (cr) { - *cr = found->isModule() - ? found->asModule()->lookupRange(pc) - : found->asLazyStub()->lookupRange(pc); + if (codeRange) { + *codeRange = found->isModule() + ? found->asModule()->lookupRange(pc) + : found->asLazyStub()->lookupRange(pc); } return found; } + if (codeRange) + *codeRange = nullptr; return nullptr; } const Code* -wasm::LookupCode(const void* pc, const CodeRange** cr /* = nullptr */) +wasm::LookupCode(const void* pc, const CodeRange** codeRange /* = nullptr */) { - const CodeSegment* found = LookupCodeSegment(pc, cr); + const CodeSegment* found = LookupCodeSegment(pc, codeRange); + MOZ_ASSERT_IF(!found && codeRange, !*codeRange); return found ? &found->code() : nullptr; } From f39704cd607e99d6330f3864ae1b283ff1e21793 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 20 Aug 2018 15:23:01 +0200 Subject: [PATCH 07/26] Bug 1437065: Drive-by fixes; r=jandem --HG-- extra : rebase_source : 23f246fa48bd2dcc85c3918609630f50aa1d0837 extra : histedit_source : a33a5be54c7a340999e1b4b5b9316ae37f7cc374 --- js/src/jit/CodeGenerator.cpp | 2 +- js/src/jit/JSJitFrameIter.cpp | 2 +- js/src/jit/JitFrames.h | 7 +++---- js/src/jit/LIR.h | 5 +++++ js/src/jit/MCallOptimize.cpp | 2 +- js/src/jit/MIR.h | 4 ++-- js/src/jit/shared/Lowering-shared-inl.h | 1 - js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp | 3 --- js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp | 2 +- js/src/wasm/WasmInstance.h | 2 +- 10 files changed, 15 insertions(+), 15 deletions(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index a207e597a3ef..34ff48056732 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -4563,7 +4563,7 @@ CodeGenerator::visitCallDOMNative(LCallDOMNative* call) MOZ_ASSERT(obj == argObj); // Push a Value containing the callee object: natives are allowed to access their callee before - // setitng the return value. After this the StackPointer points to &vp[0]. + // setting the return value. After this the StackPointer points to &vp[0]. masm.Push(ObjectValue(*target->rawJSFunction())); // Now compute the argv value. Since StackPointer is pointing to &vp[0] and diff --git a/js/src/jit/JSJitFrameIter.cpp b/js/src/jit/JSJitFrameIter.cpp index 90aaceb24e31..b1182f1da64c 100644 --- a/js/src/jit/JSJitFrameIter.cpp +++ b/js/src/jit/JSJitFrameIter.cpp @@ -93,7 +93,7 @@ JSJitFrameIter::callee() const JSFunction* JSJitFrameIter::maybeCallee() const { - if (isScripted() && (isFunctionFrame())) + if (isScripted() && isFunctionFrame()) return callee(); return nullptr; } diff --git a/js/src/jit/JitFrames.h b/js/src/jit/JitFrames.h index 2ed3e9afdbb2..5aa8fecbf037 100644 --- a/js/src/jit/JitFrames.h +++ b/js/src/jit/JitFrames.h @@ -190,6 +190,7 @@ class OsiIndex // [ frame size | has cached saved frame bit | frame header size | frame type ] // < highest - - - - - - - - - - - - - - lowest > static const uintptr_t FRAMETYPE_BITS = 4; +static const uintptr_t FRAMETYPE_MASK = (1 << FRAMETYPE_BITS) - 1; static const uintptr_t FRAME_HEADER_SIZE_SHIFT = FRAMETYPE_BITS; static const uintptr_t FRAME_HEADER_SIZE_BITS = 3; static const uintptr_t FRAME_HEADER_SIZE_MASK = (1 << FRAME_HEADER_SIZE_BITS) - 1; @@ -342,8 +343,6 @@ class CommonFrameLayout uint8_t* returnAddress_; uintptr_t descriptor_; - static const uintptr_t FrameTypeMask = (1 << FRAMETYPE_BITS) - 1; - public: static size_t offsetOfDescriptor() { return offsetof(CommonFrameLayout, descriptor_); @@ -355,10 +354,10 @@ class CommonFrameLayout return offsetof(CommonFrameLayout, returnAddress_); } FrameType prevType() const { - return FrameType(descriptor_ & FrameTypeMask); + return FrameType(descriptor_ & FRAMETYPE_MASK); } void changePrevType(FrameType type) { - descriptor_ &= ~FrameTypeMask; + descriptor_ &= ~FRAMETYPE_MASK; descriptor_ |= type; } size_t prevFrameLocalSize() const { diff --git a/js/src/jit/LIR.h b/js/src/jit/LIR.h index 04c03473d130..c988f79d4d68 100644 --- a/js/src/jit/LIR.h +++ b/js/src/jit/LIR.h @@ -658,6 +658,8 @@ class LSafepoint; class LInstruction; class LElementVisitor; +constexpr size_t MaxNumLInstructionOperands = 63; + // The common base class for LPhi and LInstruction. class LNode { @@ -672,9 +674,12 @@ class LNode // Bitfields below are all uint32_t to make sure MSVC packs them correctly. uint32_t op_ : 10; uint32_t isCall_ : 1; + // LPhi::numOperands() may not fit in this bitfield, so we only use this // field for LInstruction. uint32_t nonPhiNumOperands_ : 6; + static_assert((1 << 6) - 1 == MaxNumLInstructionOperands, "packing constraints"); + // For LInstruction, the first operand is stored at offset // sizeof(LInstruction) + nonPhiOperandsOffset_ * sizeof(uintptr_t). uint32_t nonPhiOperandsOffset_ : 5; diff --git a/js/src/jit/MCallOptimize.cpp b/js/src/jit/MCallOptimize.cpp index 1310f522c3bd..ae317ed4a0d7 100644 --- a/js/src/jit/MCallOptimize.cpp +++ b/js/src/jit/MCallOptimize.cpp @@ -2656,7 +2656,7 @@ IonBuilder::inlineGuardToClass(CallInfo& callInfo, const Class* clasp) { return InliningStatus_NotInlined; } - + TemporaryTypeSet* types = callInfo.getArg(0)->resultTypeSet(); const Class* knownClass = types ? types->getKnownClass(constraints()) : nullptr; diff --git a/js/src/jit/MIR.h b/js/src/jit/MIR.h index 1b1f467a4510..33a033bbdc2a 100644 --- a/js/src/jit/MIR.h +++ b/js/src/jit/MIR.h @@ -239,8 +239,8 @@ class MUse : public TempObject, public InlineListNode #ifdef DEBUG // Return the operand index of this MUse in its consumer. This is DEBUG-only - // as normal code should instead to call indexOf on the casted consumer - // directly, to allow it to be devirtualized and inlined. + // as normal code should instead call indexOf on the cast consumer directly, + // to allow it to be devirtualized and inlined. size_t index() const; #endif }; diff --git a/js/src/jit/shared/Lowering-shared-inl.h b/js/src/jit/shared/Lowering-shared-inl.h index 03a684b963ca..766b99f1644c 100644 --- a/js/src/jit/shared/Lowering-shared-inl.h +++ b/js/src/jit/shared/Lowering-shared-inl.h @@ -828,7 +828,6 @@ LIRGeneratorShared::useBoxOrTyped(MDefinition* mir) if (mir->type() == MIRType::Value) return useBox(mir); - #if defined(JS_NUNBOX32) return LBoxAllocation(useRegister(mir), LAllocation()); #else diff --git a/js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp b/js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp index 8442ff802781..13990d2a93fd 100644 --- a/js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp +++ b/js/src/jit/x86-shared/CodeGenerator-x86-shared.cpp @@ -301,9 +301,6 @@ CodeGenerator::visitWasmStackArg(LWasmStackArg* ins) case MIRType::Float32: masm.storeFloat32(ToFloatRegister(ins->arg()), dst); return; - case MIRType::Int32x4: - case MIRType::Bool32x4: - case MIRType::Float32x4: default: break; } diff --git a/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp b/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp index eedb2a557ab2..b5c0efe44614 100644 --- a/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp +++ b/js/src/jit/x86-shared/MacroAssembler-x86-shared.cpp @@ -335,7 +335,7 @@ MacroAssembler::flexibleDivMod32(Register rhs, Register lhsOutput, Register remO MOZ_ASSERT(lhsOutput != remOutput); // Choose a register that is not edx, or eax to hold the rhs; - // ebx is chosen arbitrarily, and will be preserved if necessary. + // ebx is chosen arbitrarily, and will be preserved if necessary. Register regForRhs = (rhs == eax || rhs == edx) ? ebx : rhs; // Add registers we will be clobbering as live, but diff --git a/js/src/wasm/WasmInstance.h b/js/src/wasm/WasmInstance.h index fe8a40cceb7f..701d6bd391dd 100644 --- a/js/src/wasm/WasmInstance.h +++ b/js/src/wasm/WasmInstance.h @@ -118,7 +118,7 @@ class Instance #endif // This method returns a pointer to the GC object that owns this Instance. - // Instances may be reached via weak edges (e.g., Compartment::instances_) + // Instances may be reached via weak edges (e.g., Realm::instances_) // so this perform a read-barrier on the returned object unless the barrier // is explicitly waived. From bb271da0eeeed7e38272334f4ed120c4dabcf513 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Mon, 20 Aug 2018 15:38:36 +0200 Subject: [PATCH 08/26] Bug 1437065: Rename interpCodeRangeIndex to funcCodeRangeIndex; r=luke --HG-- extra : rebase_source : e4f93e725474295d9008a1dfc1db6785d4ff56c7 extra : histedit_source : df644cdfd5fdc5dc9a1ee72ec0a6d0730e951364 --- js/src/wasm/WasmCode.cpp | 4 ++-- js/src/wasm/WasmCode.h | 22 +++++++++++----------- js/src/wasm/WasmGenerator.cpp | 2 +- js/src/wasm/WasmJS.cpp | 4 ++-- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/js/src/wasm/WasmCode.cpp b/js/src/wasm/WasmCode.cpp index 023b691d8d33..c18eaeeffe19 100644 --- a/js/src/wasm/WasmCode.cpp +++ b/js/src/wasm/WasmCode.cpp @@ -648,7 +648,7 @@ LazyStubTier::createMany(HasGcTypes gcTypesEnabled, const Uint32Vector& funcExpo const FuncExport& fe = funcExports[funcExportIndex]; numExpectedRanges += fe.funcType().temporarilyUnsupportedAnyRef() ? 1 : 2; void* calleePtr = moduleSegmentBase + - moduleRanges[fe.interpCodeRangeIndex()].funcNormalEntry(); + moduleRanges[fe.funcCodeRangeIndex()].funcNormalEntry(); Maybe callee; callee.emplace(calleePtr, ImmPtr::NoCheckToken()); if (!GenerateEntryStubs(masm, funcExportIndex, fe, callee, /* asmjs */ false, @@ -803,7 +803,7 @@ LazyStubTier::lookupInterpEntry(uint32_t funcIndex) const &match)); const LazyFuncExport& fe = exports_[match]; const LazyStubSegment& stub = *stubSegments_[fe.lazyStubSegmentIndex]; - return stub.base() + stub.codeRanges()[fe.interpCodeRangeIndex].begin(); + return stub.base() + stub.codeRanges()[fe.funcCodeRangeIndex].begin(); } void diff --git a/js/src/wasm/WasmCode.h b/js/src/wasm/WasmCode.h index a89d053e6dd6..ad18996abd8e 100644 --- a/js/src/wasm/WasmCode.h +++ b/js/src/wasm/WasmCode.h @@ -190,7 +190,7 @@ class FuncExport FuncType funcType_; MOZ_INIT_OUTSIDE_CTOR struct CacheablePod { uint32_t funcIndex_; - uint32_t interpCodeRangeIndex_; + uint32_t funcCodeRangeIndex_; uint32_t eagerInterpEntryOffset_; // Machine code offset bool hasEagerStubs_; } pod; @@ -201,7 +201,7 @@ class FuncExport : funcType_(std::move(funcType)) { pod.funcIndex_ = funcIndex; - pod.interpCodeRangeIndex_ = UINT32_MAX; + pod.funcCodeRangeIndex_ = UINT32_MAX; pod.eagerInterpEntryOffset_ = UINT32_MAX; pod.hasEagerStubs_ = hasEagerStubs; } @@ -210,9 +210,9 @@ class FuncExport MOZ_ASSERT(hasEagerStubs()); pod.eagerInterpEntryOffset_ = entryOffset; } - void initInterpCodeRangeIndex(uint32_t codeRangeIndex) { - MOZ_ASSERT(pod.interpCodeRangeIndex_ == UINT32_MAX); - pod.interpCodeRangeIndex_ = codeRangeIndex; + void initFuncCodeRangeIndex(uint32_t codeRangeIndex) { + MOZ_ASSERT(pod.funcCodeRangeIndex_ == UINT32_MAX); + pod.funcCodeRangeIndex_ = codeRangeIndex; } bool hasEagerStubs() const { @@ -224,9 +224,9 @@ class FuncExport uint32_t funcIndex() const { return pod.funcIndex_; } - uint32_t interpCodeRangeIndex() const { - MOZ_ASSERT(pod.interpCodeRangeIndex_ != UINT32_MAX); - return pod.interpCodeRangeIndex_; + uint32_t funcCodeRangeIndex() const { + MOZ_ASSERT(pod.funcCodeRangeIndex_ != UINT32_MAX); + return pod.funcCodeRangeIndex_; } uint32_t eagerInterpEntryOffset() const { MOZ_ASSERT(pod.eagerInterpEntryOffset_ != UINT32_MAX); @@ -532,11 +532,11 @@ struct LazyFuncExport { size_t funcIndex; size_t lazyStubSegmentIndex; - size_t interpCodeRangeIndex; - LazyFuncExport(size_t funcIndex, size_t lazyStubSegmentIndex, size_t interpCodeRangeIndex) + size_t funcCodeRangeIndex; + LazyFuncExport(size_t funcIndex, size_t lazyStubSegmentIndex, size_t funcCodeRangeIndex) : funcIndex(funcIndex), lazyStubSegmentIndex(lazyStubSegmentIndex), - interpCodeRangeIndex(interpCodeRangeIndex) + funcCodeRangeIndex(funcCodeRangeIndex) {} }; diff --git a/js/src/wasm/WasmGenerator.cpp b/js/src/wasm/WasmGenerator.cpp index 7d5d7392a606..ecf85e262e85 100644 --- a/js/src/wasm/WasmGenerator.cpp +++ b/js/src/wasm/WasmGenerator.cpp @@ -862,7 +862,7 @@ ModuleGenerator::finishMetadata(const ShareableBytes& bytecode) // now that every function has a code range. for (FuncExport& fe : metadataTier_->funcExports) - fe.initInterpCodeRangeIndex(funcToCodeRange_[fe.funcIndex()]); + fe.initFuncCodeRangeIndex(funcToCodeRange_[fe.funcIndex()]); for (ElemSegment& elems : env_->elemSegments) { Uint32Vector& codeRangeIndices = elems.elemCodeRangeIndices(tier()); diff --git a/js/src/wasm/WasmJS.cpp b/js/src/wasm/WasmJS.cpp index 9c3d2d374380..790757740610 100644 --- a/js/src/wasm/WasmJS.cpp +++ b/js/src/wasm/WasmJS.cpp @@ -1384,7 +1384,7 @@ WasmInstanceObject::getExportedFunctionCodeRange(HandleFunction fun, Tier tier) uint32_t funcIndex = ExportedFunctionToFuncIndex(fun); MOZ_ASSERT(exports().lookup(funcIndex)->value() == fun); const FuncExport& funcExport = instance().metadata(tier).lookupFuncExport(funcIndex); - return instance().metadata(tier).codeRanges[funcExport.interpCodeRangeIndex()]; + return instance().metadata(tier).codeRanges[funcExport.funcCodeRangeIndex()]; } /* static */ WasmInstanceScope* @@ -2051,7 +2051,7 @@ WasmTableObject::setImpl(JSContext* cx, const CallArgs& args) Tier tier = instance.code().bestTier(); const MetadataTier& metadata = instance.metadata(tier); const FuncExport& funcExport = metadata.lookupFuncExport(funcIndex); - const CodeRange& codeRange = metadata.codeRanges[funcExport.interpCodeRangeIndex()]; + const CodeRange& codeRange = metadata.codeRanges[funcExport.funcCodeRangeIndex()]; void* code = instance.codeBase(tier) + codeRange.funcTableEntry(); table.set(index, code, instance); } else { From 7fc2bdaae12521c2c7dd50b7335a65f4ca135821 Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Fri, 17 Aug 2018 17:56:28 +0900 Subject: [PATCH 09/26] Bug 1484110 - part 1: Create HTMLEditor::RefereshEditingUI() for internal use of nsIHTMLEditor::CheckSelectionStateForAnonymousButtons() r=m_kato HTMLEditor::CheckSelectionStateForAnonymousButtons() is called a lot internally. Especially, its virtual call cost may make damage to our performance since it's called from a selection listener. So, we should create non-virtual method, RefereshEditingUI() for internal use. --- editor/libeditor/EditorBase.cpp | 6 +-- editor/libeditor/HTMLAbsPositionEditor.cpp | 6 ++- editor/libeditor/HTMLAnonymousNodeEditor.cpp | 39 ++++++++++++++------ editor/libeditor/HTMLEditor.cpp | 9 +++-- editor/libeditor/HTMLEditor.h | 7 ++++ editor/nsIHTMLEditor.idl | 11 ++++-- 6 files changed, 55 insertions(+), 23 deletions(-) diff --git a/editor/libeditor/EditorBase.cpp b/editor/libeditor/EditorBase.cpp index 97cb62d037a0..b1622ce4e81d 100644 --- a/editor/libeditor/EditorBase.cpp +++ b/editor/libeditor/EditorBase.cpp @@ -4200,10 +4200,8 @@ EditorBase::EndUpdateViewBatch() return; } - DebugOnly rv = - htmlEditor->CheckSelectionStateForAnonymousButtons(selection); - NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), - "CheckSelectionStateForAnonymousButtons() failed"); + DebugOnly rv = htmlEditor->RefereshEditingUI(*selection); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "RefereshEditingUI() failed"); } TextComposition* diff --git a/editor/libeditor/HTMLAbsPositionEditor.cpp b/editor/libeditor/HTMLAbsPositionEditor.cpp index 89726dd86a49..424cf76d3638 100644 --- a/editor/libeditor/HTMLAbsPositionEditor.cpp +++ b/editor/libeditor/HTMLAbsPositionEditor.cpp @@ -418,7 +418,11 @@ HTMLEditor::EndMoving() if (!selection) { return NS_ERROR_NOT_INITIALIZED; } - return CheckSelectionStateForAnonymousButtons(selection); + nsresult rv = RefereshEditingUI(*selection); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; } nsresult HTMLEditor::SetFinalPosition(int32_t aX, diff --git a/editor/libeditor/HTMLAnonymousNodeEditor.cpp b/editor/libeditor/HTMLAnonymousNodeEditor.cpp index 55553479568a..9ac7faed26fd 100644 --- a/editor/libeditor/HTMLAnonymousNodeEditor.cpp +++ b/editor/libeditor/HTMLAnonymousNodeEditor.cpp @@ -282,21 +282,26 @@ HTMLEditor::DeleteRefToAnonymousNode(ManualNACPtr aContent, // The ManualNACPtr destructor will invoke UnbindFromTree. } -// The following method is mostly called by a selection listener. When a -// selection change is notified, the method is called to check if resizing -// handles, a grabber and/or inline table editing UI need to be displayed -// or refreshed NS_IMETHODIMP HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection) { if (NS_WARN_IF(!aSelection)) { return NS_ERROR_INVALID_ARG; } + nsresult rv = RefereshEditingUI(*aSelection); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } + return NS_OK; +} +nsresult +HTMLEditor::RefereshEditingUI(Selection& aSelection) +{ // early way out if all contextual UI extensions are disabled - if (NS_WARN_IF(!IsObjectResizerEnabled() && - !IsAbsolutePositionEditorEnabled() && - !IsInlineTableEditorEnabled())) { + if (!IsObjectResizerEnabled() && + !IsAbsolutePositionEditorEnabled() && + !IsInlineTableEditorEnabled()) { return NS_OK; } @@ -306,7 +311,7 @@ HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection) } // let's get the containing element of the selection - RefPtr focusElement = GetSelectionContainerElement(*aSelection); + RefPtr focusElement = GetSelectionContainerElement(aSelection); if (NS_WARN_IF(!focusElement)) { return NS_OK; } @@ -331,7 +336,7 @@ HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection) // Resizing or Inline Table Editing is enabled, we need to check if the // selection is contained in a table cell cellElement = - GetElementOrParentByTagNameAtSelection(*aSelection, *nsGkAtoms::td); + GetElementOrParentByTagNameAtSelection(aSelection, *nsGkAtoms::td); } if (IsObjectResizerEnabled() && cellElement) { @@ -342,6 +347,9 @@ HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection) if (nsGkAtoms::img != focusTagAtom) { // the element container of the selection is not an image, so we'll show // the resizers around the table + // XXX There may be a bug. cellElement may be not in in invalid + // tree. So, perhaps, GetEnclosingTable() returns nullptr, we should + // not set focusTagAtom to nsGkAtoms::table. focusElement = GetEnclosingTable(cellElement); focusTagAtom = nsGkAtoms::table; } @@ -369,15 +377,24 @@ HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection) if (IsObjectResizerEnabled() && mResizedObject && mResizedObject != focusElement) { + // Perhaps, even if HideResizers() failed, we should try to hide inline + // table editing UI. However, it returns error only when we cannot do + // anything. So, it's okay for now. nsresult rv = HideResizers(); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } NS_ASSERTION(!mResizedObject, "HideResizers failed"); } if (mIsInlineTableEditingEnabled && mInlineEditedCell && mInlineEditedCell != cellElement) { + // XXX HideInlineTableEditingUI() won't return error. Should be change it + // void later. nsresult rv = HideInlineTableEditingUI(); - NS_ENSURE_SUCCESS(rv, rv); + if (NS_WARN_IF(NS_FAILED(rv))) { + return rv; + } NS_ASSERTION(!mInlineEditedCell, "HideInlineTableEditingUI failed"); } diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index 6ca98ca34dd3..56628814a93e 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -434,9 +434,9 @@ HTMLEditor::NotifySelectionChanged(nsIDocument* aDocument, typeInState->OnSelectionChange(*aSelection); // We used a class which derived from nsISelectionListener to call - // HTMLEditor::CheckSelectionStateForAnonymousButtons(). The lifetime of - // the class was exactly same as mTypeInState. So, call it only when - // mTypeInState is not nullptr. + // HTMLEditor::RefereshEditingUI(). The lifetime of the class was + // exactly same as mTypeInState. So, call it only when mTypeInState + // is not nullptr. if ((aReason & (nsISelectionListener::MOUSEDOWN_REASON | nsISelectionListener::KEYPRESS_REASON | nsISelectionListener::SELECTALL_REASON)) && aSelection) { @@ -445,7 +445,8 @@ HTMLEditor::NotifySelectionChanged(nsIDocument* aDocument, // FYI: This is an XPCOM method. So, the caller, Selection, guarantees // the lifetime of this instance. So, don't need to grab this with // local variable. - CheckSelectionStateForAnonymousButtons(aSelection); + DebugOnly rv = RefereshEditingUI(*aSelection); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "RefereshEditingUI() failed"); } } diff --git a/editor/libeditor/HTMLEditor.h b/editor/libeditor/HTMLEditor.h index 22bae73e013e..9574ae68ad08 100644 --- a/editor/libeditor/HTMLEditor.h +++ b/editor/libeditor/HTMLEditor.h @@ -1510,6 +1510,13 @@ protected: // Shouldn't be used by friend classes void DeleteRefToAnonymousNode(ManualNACPtr aContent, nsIPresShell* aShell); + /** + * RefereshEditingUI() may refresh editing UIs for current Selection, focus, + * etc. If this shows or hides some UIs, it causes reflow. So, this is + * not safe method. + */ + nsresult RefereshEditingUI(Selection& aSelection); + nsresult ShowResizersInner(Element& aResizedElement); /** diff --git a/editor/nsIHTMLEditor.idl b/editor/nsIHTMLEditor.idl index 9db1729fa187..00f518bd0afb 100644 --- a/editor/nsIHTMLEditor.idl +++ b/editor/nsIHTMLEditor.idl @@ -397,9 +397,14 @@ interface nsIHTMLEditor : nsISupports attribute boolean isCSSEnabled; /** - * Checks if the anonymous nodes created by the HTML editor have to be - * refreshed or hidden depending on a possible new state of the selection - * @param aSelection [IN] a selection + * checkSelectionStateForAnonymousButtons() may refresh editing UI such as + * resizers, inline-table-editing UI, absolute positioning UI for current + * Selection and focus state. When this method shows or hides UI, the + * editor (and/or its document/window) could be broken by mutation observers. + * FYI: Current user in script is only BlueGriffon. + * + * @param aSelection Selection instance for the normal selection of the + * document. */ void checkSelectionStateForAnonymousButtons(in Selection aSelection); From a24cf41019605f5ba31fcd7d436c5ceab8c4c20f Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Fri, 17 Aug 2018 18:23:13 +0900 Subject: [PATCH 10/26] Bug 1484110 - part 2: Rewrite HTMLEditor::HideInlineTableEditingUI() r=m_kato First, HTMLEditor::HideInlineTableEditingUI() always returns NS_OK. So, we can change its return type to void. Additionally, it removes each UI from the DOM tree one by one. However, each mutation could cause showing same UI again. In such case, ShowInlineTableEditingUI() overwrites each UI with newly created element. Then, HTMLEditor cannot remove the old UI anymore. Therefore, this patch moves all members of the UI into local variables first. --- editor/libeditor/HTMLAnonymousNodeEditor.cpp | 5 +--- editor/libeditor/HTMLEditor.h | 2 +- editor/libeditor/HTMLInlineTableEditor.cpp | 25 +++++++++++++------- 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/editor/libeditor/HTMLAnonymousNodeEditor.cpp b/editor/libeditor/HTMLAnonymousNodeEditor.cpp index 9ac7faed26fd..9b84cada2faf 100644 --- a/editor/libeditor/HTMLAnonymousNodeEditor.cpp +++ b/editor/libeditor/HTMLAnonymousNodeEditor.cpp @@ -391,10 +391,7 @@ HTMLEditor::RefereshEditingUI(Selection& aSelection) mInlineEditedCell != cellElement) { // XXX HideInlineTableEditingUI() won't return error. Should be change it // void later. - nsresult rv = HideInlineTableEditingUI(); - if (NS_WARN_IF(NS_FAILED(rv))) { - return rv; - } + HideInlineTableEditingUI(); NS_ASSERTION(!mInlineEditedCell, "HideInlineTableEditingUI failed"); } diff --git a/editor/libeditor/HTMLEditor.h b/editor/libeditor/HTMLEditor.h index 9574ae68ad08..9556329f9dfc 100644 --- a/editor/libeditor/HTMLEditor.h +++ b/editor/libeditor/HTMLEditor.h @@ -1619,7 +1619,7 @@ protected: // Shouldn't be used by friend classes /** * Hide all inline table editing UI */ - nsresult HideInlineTableEditingUI(); + void HideInlineTableEditingUI(); /** * IsEmptyTextNode() returns true if aNode is a text node and does not have diff --git a/editor/libeditor/HTMLInlineTableEditor.cpp b/editor/libeditor/HTMLInlineTableEditor.cpp index e601bb4c8408..64a80750181e 100644 --- a/editor/libeditor/HTMLInlineTableEditor.cpp +++ b/editor/libeditor/HTMLInlineTableEditor.cpp @@ -94,7 +94,7 @@ HTMLEditor::ShowInlineTableEditingUI(Element* aCell) return RefreshInlineTableEditingUI(); } -nsresult +void HTMLEditor::HideInlineTableEditingUI() { mInlineEditedCell = nullptr; @@ -112,14 +112,23 @@ HTMLEditor::HideInlineTableEditingUI() // are no document observers to notify, but we still want to // UnbindFromTree. - DeleteRefToAnonymousNode(std::move(mAddColumnBeforeButton), ps); - DeleteRefToAnonymousNode(std::move(mRemoveColumnButton), ps); - DeleteRefToAnonymousNode(std::move(mAddColumnAfterButton), ps); - DeleteRefToAnonymousNode(std::move(mAddRowBeforeButton), ps); - DeleteRefToAnonymousNode(std::move(mRemoveRowButton), ps); - DeleteRefToAnonymousNode(std::move(mAddRowAfterButton), ps); + // Calling DeleteRefToAnonymousNode() may cause showing the UI again. + // Therefore, we should forget all anonymous contents first. + // Otherwise, we could leak the old content because of overwritten by + // ShowInlineTableEditingUI(). + ManualNACPtr addColumnBeforeButton(std::move(mAddColumnBeforeButton)); + ManualNACPtr removeColumnButton(std::move(mRemoveColumnButton)); + ManualNACPtr addColumnAfterButton(std::move(mAddColumnAfterButton)); + ManualNACPtr addRowBeforeButton(std::move(mAddRowBeforeButton)); + ManualNACPtr removeRowButton(std::move(mRemoveRowButton)); + ManualNACPtr addRowAfterButton(std::move(mAddRowAfterButton)); - return NS_OK; + DeleteRefToAnonymousNode(std::move(addColumnBeforeButton), ps); + DeleteRefToAnonymousNode(std::move(removeColumnButton), ps); + DeleteRefToAnonymousNode(std::move(addColumnAfterButton), ps); + DeleteRefToAnonymousNode(std::move(addRowBeforeButton), ps); + DeleteRefToAnonymousNode(std::move(removeRowButton), ps); + DeleteRefToAnonymousNode(std::move(addRowAfterButton), ps); } nsresult From d33fe8cd6d972737f83ef84e79e17e4fa63acedd Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Fri, 17 Aug 2018 19:03:02 +0900 Subject: [PATCH 11/26] Bug 1484110 - part 3: HTMLEditor::RefereshEditingUI() should refresh UIs when one of them is changed to enabled or disabled r=m_kato HTMLEditor::RefereshEditingUI() works only with enabled UIs. Therefore, if UI is disabled while it's visible, it keeps shown. This is too bad if web apps tries to disable the Gecko specific UIs after we show some of them. This patch adds HTMLEditor::HideAnonymousEditingUIsIfUnnecessary() to hide unnecessary UIs and makes RefereshEditingUI() call it always. --- editor/libeditor/HTMLAnonymousNodeEditor.cpp | 53 ++++++++++++++++++- editor/libeditor/HTMLEditor.cpp | 14 ----- editor/libeditor/HTMLEditor.h | 35 ++++++++++++ .../tests/test_abs_positioner_appearance.html | 10 ++++ .../tests/test_resizers_appearance.html | 9 ++++ 5 files changed, 105 insertions(+), 16 deletions(-) diff --git a/editor/libeditor/HTMLAnonymousNodeEditor.cpp b/editor/libeditor/HTMLAnonymousNodeEditor.cpp index 9b84cada2faf..16d216d69768 100644 --- a/editor/libeditor/HTMLAnonymousNodeEditor.cpp +++ b/editor/libeditor/HTMLAnonymousNodeEditor.cpp @@ -282,6 +282,51 @@ HTMLEditor::DeleteRefToAnonymousNode(ManualNACPtr aContent, // The ManualNACPtr destructor will invoke UnbindFromTree. } +void +HTMLEditor::HideAnonymousEditingUIs() +{ + if (mAbsolutelyPositionedObject) { + HideGrabber(); + NS_ASSERTION(!mAbsolutelyPositionedObject, "HideGrabber failed"); + } + if (mInlineEditedCell) { + HideInlineTableEditingUI(); + NS_ASSERTION(!mInlineEditedCell, "HideInlineTableEditingUI failed"); + } + if (mResizedObject) { + DebugOnly rv = HideResizers(); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "HideResizers() failed"); + NS_ASSERTION(!mResizedObject, "HideResizers failed"); + } +} + +void +HTMLEditor::HideAnonymousEditingUIsIfUnnecessary() +{ + // XXX Perhaps, this is wrong approach to hide multiple UIs because + // hiding one UI may causes overwriting existing UI with newly + // created one. In such case, we will leak ovewritten UI. + if (!IsAbsolutePositionEditorEnabled() && mAbsolutelyPositionedObject) { + // XXX If we're moving something, we need to cancel or commit the + // operation now. + HideGrabber(); + NS_ASSERTION(!mAbsolutelyPositionedObject, "HideGrabber failed"); + } + if (!IsInlineTableEditorEnabled() && mInlineEditedCell) { + // XXX If we're resizing a table element, we need to cancel or commit the + // operation now. + HideInlineTableEditingUI(); + NS_ASSERTION(!mInlineEditedCell, "HideInlineTableEditingUI failed"); + } + if (!IsObjectResizerEnabled() && mResizedObject) { + // XXX If we're resizing something, we need to cancel or commit the + // operation now. + DebugOnly rv = HideResizers(); + NS_WARNING_ASSERTION(NS_SUCCEEDED(rv), "HideResizers() failed"); + NS_ASSERTION(!mResizedObject, "HideResizers failed"); + } +} + NS_IMETHODIMP HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection) { @@ -298,6 +343,10 @@ HTMLEditor::CheckSelectionStateForAnonymousButtons(Selection* aSelection) nsresult HTMLEditor::RefereshEditingUI(Selection& aSelection) { + // First, we need to remove unnecessary editing UI now since some of them + // may be disabled while them are visible. + HideAnonymousEditingUIsIfUnnecessary(); + // early way out if all contextual UI extensions are disabled if (!IsObjectResizerEnabled() && !IsAbsolutePositionEditorEnabled() && @@ -387,7 +436,7 @@ HTMLEditor::RefereshEditingUI(Selection& aSelection) NS_ASSERTION(!mResizedObject, "HideResizers failed"); } - if (mIsInlineTableEditingEnabled && mInlineEditedCell && + if (IsInlineTableEditorEnabled() && mInlineEditedCell && mInlineEditedCell != cellElement) { // XXX HideInlineTableEditingUI() won't return error. Should be change it // void later. @@ -431,7 +480,7 @@ HTMLEditor::RefereshEditingUI(Selection& aSelection) } } - if (mIsInlineTableEditingEnabled && cellElement && + if (IsInlineTableEditorEnabled() && cellElement && IsModifiableNode(*cellElement) && cellElement != hostContent) { if (mInlineEditedCell) { nsresult rv = RefreshInlineTableEditingUI(); diff --git a/editor/libeditor/HTMLEditor.cpp b/editor/libeditor/HTMLEditor.cpp index 56628814a93e..a36e7b4ff3fd 100644 --- a/editor/libeditor/HTMLEditor.cpp +++ b/editor/libeditor/HTMLEditor.cpp @@ -258,20 +258,6 @@ HTMLEditor::~HTMLEditor() } } -void -HTMLEditor::HideAnonymousEditingUIs() -{ - if (mAbsolutelyPositionedObject) { - HideGrabber(); - } - if (mInlineEditedCell) { - HideInlineTableEditingUI(); - } - if (mResizedObject) { - HideResizers(); - } -} - NS_IMPL_CYCLE_COLLECTION_CLASS(HTMLEditor) NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED(HTMLEditor, TextEditor) diff --git a/editor/libeditor/HTMLEditor.h b/editor/libeditor/HTMLEditor.h index 9556329f9dfc..973625105388 100644 --- a/editor/libeditor/HTMLEditor.h +++ b/editor/libeditor/HTMLEditor.h @@ -227,7 +227,15 @@ public: */ void EnableObjectResizer(bool aEnable) { + if (mIsObjectResizingEnabled == aEnable) { + return; + } mIsObjectResizingEnabled = aEnable; + RefPtr selection = GetSelection(); + if (NS_WARN_IF(!selection)) { + return; + } + RefereshEditingUI(*selection); } bool IsObjectResizerEnabled() const { @@ -240,7 +248,15 @@ public: */ void EnableInlineTableEditor(bool aEnable) { + if (mIsInlineTableEditingEnabled == aEnable) { + return; + } mIsInlineTableEditingEnabled = aEnable; + RefPtr selection = GetSelection(); + if (NS_WARN_IF(!selection)) { + return; + } + RefereshEditingUI(*selection); } bool IsInlineTableEditorEnabled() const { @@ -254,7 +270,15 @@ public: */ void EnableAbsolutePositionEditor(bool aEnable) { + if (mIsAbsolutelyPositioningEnabled == aEnable) { + return; + } mIsAbsolutelyPositioningEnabled = aEnable; + RefPtr selection = GetSelection(); + if (NS_WARN_IF(!selection)) { + return; + } + RefereshEditingUI(*selection); } bool IsAbsolutePositionEditorEnabled() const { @@ -1577,8 +1601,19 @@ protected: // Shouldn't be used by friend classes void SetFinalSize(int32_t aX, int32_t aY); void SetResizeIncrements(int32_t aX, int32_t aY, int32_t aW, int32_t aH, bool aPreserveRatio); + + /** + * HideAnonymousEditingUIs() forcibly hides all editing UIs (resizers, + * inline-table-editing UI, absolute positioning UI). + */ void HideAnonymousEditingUIs(); + /** + * HideAnonymousEditingUIsIfUnnecessary() hides all editing UIs if some of + * visible UIs are now unnecessary. + */ + void HideAnonymousEditingUIsIfUnnecessary(); + /** * sets the z-index of an element. * @param aElement [IN] the element diff --git a/editor/libeditor/tests/test_abs_positioner_appearance.html b/editor/libeditor/tests/test_abs_positioner_appearance.html index b7fd8e840b02..6f8042622397 100644 --- a/editor/libeditor/tests/test_abs_positioner_appearance.html +++ b/editor/libeditor/tests/test_abs_positioner_appearance.html @@ -78,6 +78,16 @@ SimpleTest.waitForFocus(async function() { is(target.hasAttribute("_moz_abspos"), kTest.movable, kDescription + (kTest.movable ? "While enableAbsolutePositionEditing is enabled, positioner should appear" : "Even while enableAbsolutePositionEditing is enabled, positioner shouldn't appear")); + + document.execCommand("enableAbsolutePositionEditing", false, false); + ok(!target.hasAttribute("_moz_abspos"), + kDescription + "When enableAbsolutePositionEditing is disabled even while positioner is visible, positioner should disappear"); + + document.execCommand("enableAbsolutePositionEditing", false, true); + is(target.hasAttribute("_moz_abspos"), kTest.movable, + kDescription + (kTest.movable ? + "When enableAbsolutePositionEditing is enabled when absolute positioned element is selected, positioner should appear" : + "Even if enableAbsolutePositionEditing is enabled when static positioned element is selected, positioner shouldn't appear")); } } diff --git a/editor/libeditor/tests/test_resizers_appearance.html b/editor/libeditor/tests/test_resizers_appearance.html index 19e25fc72f0b..1ffd780cfed6 100644 --- a/editor/libeditor/tests/test_resizers_appearance.html +++ b/editor/libeditor/tests/test_resizers_appearance.html @@ -89,6 +89,15 @@ SimpleTest.waitForFocus(async function() { is(target.hasAttribute("_moz_resizing"), kResizable, kDescription + (kResizable ? "While enableObjectResizing is enabled, resizers should appear" : "Even while enableObjectResizing is enabled, resizers shouldn't appear")); + + document.execCommand("enableObjectResizing", false, false); + ok(!target.hasAttribute("_moz_resizing"), + kDescription + "enableObjectResizing is disabled even while resizers are visible, resizers should disappear"); + + document.execCommand("enableObjectResizing", false, true); + is(target.hasAttribute("_moz_resizing"), kResizable, + kDescription + (kResizable ? "enableObjectResizing is enabled when resizable object is selected, resizers should appear" : + "Even if enableObjectResizing is enabled when non-resizable object is selected, resizers shouldn't appear")); } } From 6eaa3a47e0214a2b0f9b22a807f6f970232c8b78 Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Fri, 17 Aug 2018 10:22:42 +0100 Subject: [PATCH 12/26] Bug 1443561 - Part 0: Workaround bug 1482157, set -moz-appearance:toolbox on #navigator-toolbox::after. r=dao MozReview-Commit-ID: CuJIYn9ioPO --HG-- extra : rebase_source : f76753bcf524697d5ab9a9adb05e1fa55e289f8b --- browser/themes/osx/browser.css | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/browser/themes/osx/browser.css b/browser/themes/osx/browser.css index 244399e87450..eaa2ecdf1d46 100644 --- a/browser/themes/osx/browser.css +++ b/browser/themes/osx/browser.css @@ -47,9 +47,25 @@ } #navigator-toolbox { + -moz-appearance: none; --tabs-border-color: rgba(0,0,0,.3); } +/* + This is a workaround for Bug 1482157 + -moz-appearance: toolbox; makes the macOS sheets attached to the element's + bottom border. We cannot put this property on the toolbox itself as it + cancels all backgrounds that are there, so we set it on the toolbox bottom + border. +*/ +#navigator-toolbox::after { + -moz-appearance: toolbox; + height: 1px; + /* use inset box-shadow instead of border because -moz-appearance hides the border */ + border: none; + box-shadow: inset 0 -1px var(--toolbox-border-bottom-color); +} + #tabbrowser-tabs { --tab-line-color: #0a84ff; } From 29e63a034bd5940098eedd8ac3fe4f0a9e17afb4 Mon Sep 17 00:00:00 2001 From: Vivek Dhingra Date: Fri, 17 Aug 2018 09:57:25 +0100 Subject: [PATCH 13/26] Bug 1443561 - Part 1: Make additional backgrounds alignment relative to toolbox. r=jaws MozReview-Commit-ID: 5gjrzZiLx0Q --HG-- extra : rebase_source : df858d25b76a1f96ba08538f011b527a1c00d75c --- browser/base/content/browser.css | 7 +- browser/themes/shared/browser.inc.css | 9 ++ .../components/extensions/parent/ext-theme.js | 9 +- .../extensions/test/browser/browser.ini | 1 + ...themes_additional_backgrounds_alignment.js | 105 ++++++++++++++++++ ...browser_ext_themes_multiple_backgrounds.js | 76 ++++++++----- 6 files changed, 164 insertions(+), 43 deletions(-) create mode 100644 toolkit/components/extensions/test/browser/browser_ext_themes_additional_backgrounds_alignment.js diff --git a/browser/base/content/browser.css b/browser/base/content/browser.css index bb1361604962..9966a9482d08 100644 --- a/browser/base/content/browser.css +++ b/browser/base/content/browser.css @@ -8,7 +8,6 @@ :root { --panelui-subview-transition-duration: 150ms; --lwt-additional-images: none; - --lwt-background-alignment: right top; --lwt-background-tiling: no-repeat; } @@ -18,13 +17,11 @@ :root:-moz-lwtheme { background-color: var(--lwt-accent-color) !important; - background-image: var(--lwt-additional-images) !important; - background-position: var(--lwt-background-alignment) !important; - background-repeat: var(--lwt-background-tiling) !important; } :root:-moz-lwtheme[lwtheme-image] { - background-image: var(--lwt-header-image), var(--lwt-additional-images) !important; + background-image: var(--lwt-header-image) !important; + background-position: right top !important; } :root:-moz-lwtheme:-moz-window-inactive { diff --git a/browser/themes/shared/browser.inc.css b/browser/themes/shared/browser.inc.css index 848e315bb004..2fb5fc3ef010 100644 --- a/browser/themes/shared/browser.inc.css +++ b/browser/themes/shared/browser.inc.css @@ -24,10 +24,19 @@ } /* Increase contrast of UI links on dark themes */ + :root[lwt-popup-brighttext] panel .text-link { color: @lwtPopupBrighttextLinkColor@; } +/* Set additional backgrounds alignment relative to toolbox*/ + +#navigator-toolbox:-moz-lwtheme { + background-image: var(--lwt-additional-images); + background-position: var(--lwt-background-alignment); + background-repeat: var(--lwt-background-tiling); +} + /* Toolbar / content area border */ #navigator-toolbox::after { diff --git a/toolkit/components/extensions/parent/ext-theme.js b/toolkit/components/extensions/parent/ext-theme.js index 8a2dfc324a81..25b2b0dee2e4 100644 --- a/toolkit/components/extensions/parent/ext-theme.js +++ b/toolkit/components/extensions/parent/ext-theme.js @@ -302,11 +302,7 @@ class Theme { break; } - let alignment = []; - if (this.lwtStyles.headerURL) { - alignment.push("right top"); - } - this.lwtStyles.backgroundsAlignment = alignment.concat(val).join(","); + this.lwtStyles.backgroundsAlignment = val.join(","); break; } case "additional_backgrounds_tiling": { @@ -315,9 +311,6 @@ class Theme { } let tiling = []; - if (this.lwtStyles.headerURL) { - tiling.push("no-repeat"); - } for (let i = 0, l = this.lwtStyles.additionalBackgrounds.length; i < l; ++i) { tiling.push(val[i] || "no-repeat"); } diff --git a/toolkit/components/extensions/test/browser/browser.ini b/toolkit/components/extensions/test/browser/browser.ini index dd5db6b24716..adbc22daabfe 100644 --- a/toolkit/components/extensions/test/browser/browser.ini +++ b/toolkit/components/extensions/test/browser/browser.ini @@ -4,6 +4,7 @@ support-files = [browser_ext_management_themes.js] skip-if = verify +[browser_ext_themes_additional_backgrounds_alignment.js] [browser_ext_themes_alpha_accentcolor.js] [browser_ext_themes_chromeparity.js] [browser_ext_themes_dynamic_getCurrent.js] diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_additional_backgrounds_alignment.js b/toolkit/components/extensions/test/browser/browser_ext_themes_additional_backgrounds_alignment.js new file mode 100644 index 000000000000..e1c67d2ff926 --- /dev/null +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_additional_backgrounds_alignment.js @@ -0,0 +1,105 @@ +"use strict"; + +/* globals InspectorUtils */ + +// Case 1 - When there is a headerURL image and additional_backgrounds_alignment is not specified. +// So background-position should default to "left top" +add_task(async function test_default_additional_backgrounds_alignment() { + const LEFT_TOP = "0% 0%"; + const RIGHT_TOP = "100% 0%"; + + let extension = ExtensionTestUtils.loadExtension({ + manifest: { + "theme": { + "images": { + "headerURL": "image1.png", + "additional_backgrounds": ["image1.png", "image1.png"], + }, + "colors": { + "accentcolor": ACCENT_COLOR, + "textcolor": TEXT_COLOR, + }, + }, + }, + files: { + "image1.png": BACKGROUND, + }, + + }); + + await extension.startup(); + + let docEl = document.documentElement; + let rootCS = window.getComputedStyle(docEl); + + Assert.equal( + rootCS.getPropertyValue("background-position"), + RIGHT_TOP, + "root only contains headerURL alignment property" + ); + + + let toolbox = document.querySelector("#navigator-toolbox"); + let toolboxCS = window.getComputedStyle(toolbox); + + Assert.equal( + toolboxCS.getPropertyValue("background-position"), + LEFT_TOP, + toolbox.id + " only contains default additional backgrounds alignment property" + ); + + await extension.unload(); +}); + + +// Case 2 - When there is a headerURL image and additional_backgrounds_alignment is specified. +add_task(async function test_additional_backgrounds_alignment() { + const LEFT_BOTTOM = "0% 100%"; + const CENTER_CENTER = "50% 50%"; + const RIGHT_TOP = "100% 0%"; + + let extension = ExtensionTestUtils.loadExtension({ + manifest: { + "theme": { + "images": { + "headerURL": "image1.png", + "additional_backgrounds": ["image1.png", "image1.png", "image1.png"], + }, + "colors": { + "accentcolor": ACCENT_COLOR, + "textcolor": TEXT_COLOR, + }, + "properties": { + additional_backgrounds_alignment: ["left bottom", "center center", "right top"], + }, + }, + }, + files: { + "image1.png": BACKGROUND, + }, + + }); + + await extension.startup(); + + let docEl = document.documentElement; + let rootCS = window.getComputedStyle(docEl); + + Assert.equal( + rootCS.getPropertyValue("background-position"), + RIGHT_TOP, + "root only contains headerURL alignment property" + ); + + + let toolbox = document.querySelector("#navigator-toolbox"); + let toolboxCS = window.getComputedStyle(toolbox); + + Assert.equal( + toolboxCS.getPropertyValue("background-position"), + LEFT_BOTTOM + ", " + CENTER_CENTER + ", " + RIGHT_TOP, + toolbox.id + " contains additional backgrounds alignment properties" + ); + + await extension.unload(); +}); diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js b/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js index 4b2e82ad018f..f58545a094f9 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js @@ -5,8 +5,8 @@ add_task(async function test_support_backgrounds_position() { manifest: { "theme": { "images": { - "headerURL": "face.png", - "additional_backgrounds": ["face.png", "face.png", "face.png"], + "headerURL": "face1.png", + "additional_backgrounds": ["face2.png", "face2.png", "face2.png"], }, "colors": { "accentcolor": `rgb(${FRAME_COLOR.join(",")})`, @@ -18,37 +18,45 @@ add_task(async function test_support_backgrounds_position() { }, }, files: { - "face.png": imageBufferFromDataURI(ENCODED_IMAGE_DATA), + "face1.png": imageBufferFromDataURI(ENCODED_IMAGE_DATA), + "face2.png": imageBufferFromDataURI(ENCODED_IMAGE_DATA), }, }); await extension.startup(); let docEl = window.document.documentElement; + let toolbox = document.querySelector("#navigator-toolbox"); Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set"); Assert.equal(docEl.getAttribute("lwthemetextcolor"), "bright", "LWT text color attribute should be set"); - let style = window.getComputedStyle(docEl); - let bgImage = style.backgroundImage.split(",")[0].trim(); - Assert.ok(bgImage.includes("face.png"), - `The backgroundImage should use face.png. Actual value is: ${bgImage}`); - Assert.equal(Array(4).fill(bgImage).join(", "), style.backgroundImage, - "The backgroundImage should use face.png four times."); - Assert.equal(style.backgroundPosition, "100% 0%, 0% 0%, 50% 0%, 100% 100%", - "The backgroundPosition should use the four values provided."); - Assert.equal(style.backgroundRepeat, "no-repeat", + let toolboxCS = window.getComputedStyle(toolbox); + let rootCS = window.getComputedStyle(docEl); + let rootBgImage = rootCS.backgroundImage.split(",")[0].trim(); + let bgImage = toolboxCS.backgroundImage.split(",")[0].trim(); + Assert.ok(rootBgImage.includes("face1.png"), + `The backgroundImage should use face1.png. Actual value is: ${rootBgImage}`); + Assert.equal(toolboxCS.backgroundImage, Array(3).fill(bgImage).join(", "), + "The backgroundImage should use face2.png three times."); + Assert.equal(toolboxCS.backgroundPosition, "0% 0%, 50% 0%, 100% 100%", + "The backgroundPosition should use the three values provided."); + Assert.equal(toolboxCS.backgroundRepeat, "no-repeat", "The backgroundPosition should use the default value."); await extension.unload(); Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set"); - style = window.getComputedStyle(docEl); + toolboxCS = window.getComputedStyle(toolbox); + // Styles should've reverted to their initial values. - Assert.equal(style.backgroundImage, "none"); - Assert.equal(style.backgroundPosition, "0% 0%"); - Assert.equal(style.backgroundRepeat, "repeat"); + Assert.equal(rootCS.backgroundImage, "none"); + Assert.equal(rootCS.backgroundPosition, "0% 0%"); + Assert.equal(rootCS.backgroundRepeat, "repeat"); + Assert.equal(toolboxCS.backgroundImage, "none"); + Assert.equal(toolboxCS.backgroundPosition, "0% 0%"); + Assert.equal(toolboxCS.backgroundRepeat, "repeat"); }); add_task(async function test_support_backgrounds_repeat() { @@ -79,21 +87,27 @@ add_task(async function test_support_backgrounds_repeat() { await extension.startup(); let docEl = window.document.documentElement; + let toolbox = document.querySelector("#navigator-toolbox"); Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set"); Assert.equal(docEl.getAttribute("lwthemetextcolor"), "bright", "LWT text color attribute should be set"); - let style = window.getComputedStyle(docEl); - let bgImage = style.backgroundImage.split(",")[0].trim(); + let rootCS = window.getComputedStyle(docEl); + let toolboxCS = window.getComputedStyle(toolbox); + let bgImage = rootCS.backgroundImage.split(",")[0].trim(); Assert.ok(bgImage.includes("face0.png"), `The backgroundImage should use face.png. Actual value is: ${bgImage}`); - Assert.equal([0, 1, 2, 3].map(num => bgImage.replace(/face[\d]*/, `face${num}`)).join(", "), - style.backgroundImage, "The backgroundImage should use face.png four times."); - Assert.equal(style.backgroundPosition, "100% 0%", - "The backgroundPosition should use the default value."); - Assert.equal(style.backgroundRepeat, "no-repeat, repeat-x, repeat-y, repeat", - "The backgroundPosition should use the four values provided."); + Assert.equal([1, 2, 3].map(num => bgImage.replace(/face[\d]*/, `face${num}`)).join(", "), + toolboxCS.backgroundImage, "The backgroundImage should use face.png three times."); + Assert.equal(rootCS.backgroundPosition, "100% 0%", + "The backgroundPosition should use the default value for root."); + Assert.equal(toolboxCS.backgroundPosition, "0% 0%", + "The backgroundPosition should use the default value for navigator-toolbox."); + Assert.equal(rootCS.backgroundRepeat, "no-repeat", + "The backgroundRepeat should use the default values for root."); + Assert.equal(toolboxCS.backgroundRepeat, "repeat-x, repeat-y, repeat", + "The backgroundRepeat should use the three values provided for navigator-toolbox."); await extension.unload(); @@ -124,20 +138,22 @@ add_task(async function test_additional_images_check() { await extension.startup(); let docEl = window.document.documentElement; + let toolbox = document.querySelector("#navigator-toolbox"); Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set"); Assert.equal(docEl.getAttribute("lwthemetextcolor"), "bright", "LWT text color attribute should be set"); - let style = window.getComputedStyle(docEl); - let bgImage = style.backgroundImage.split(",")[0]; + let rootCS = window.getComputedStyle(docEl); + let toolboxCS = window.getComputedStyle(toolbox); + let bgImage = rootCS.backgroundImage.split(",")[0]; Assert.ok(bgImage.includes("face.png"), `The backgroundImage should use face.png. Actual value is: ${bgImage}`); - Assert.equal(bgImage + ", none", style.backgroundImage, - "The backgroundImage should use face.png only once."); - Assert.equal(style.backgroundPosition, "100% 0%", + Assert.equal("none", toolboxCS.backgroundImage, + "The backgroundImage should not use face.png."); + Assert.equal(rootCS.backgroundPosition, "100% 0%", "The backgroundPosition should use the default value."); - Assert.equal(style.backgroundRepeat, "no-repeat", + Assert.equal(rootCS.backgroundRepeat, "no-repeat", "The backgroundPosition should use only one (default) value."); await extension.unload(); From 136f2e234014aa5c2be5d22c09309d7a3f366dc9 Mon Sep 17 00:00:00 2001 From: Tim Nguyen Date: Sun, 19 Aug 2018 10:34:33 +0100 Subject: [PATCH 14/26] Bug 1484512 - Split up places.css and only load relevant parts of it. r=dao --HG-- rename : browser/themes/linux/places/places.css => browser/themes/linux/places/sidebar.css rename : browser/themes/osx/places/places.css => browser/themes/osx/places/sidebar.css rename : browser/themes/shared/places/places.inc.css => browser/themes/shared/places/sidebar.inc.css rename : browser/themes/shared/places/tree-icons.inc.css => browser/themes/shared/places/tree-icons.css rename : browser/themes/windows/places/places.css => browser/themes/windows/places/sidebar.css extra : rebase_source : 14b9345e1408cf1d873f29b3c5f4ae6313afbad6 --- browser/base/content/browser.xul | 2 +- browser/components/places/content/bookmarkProperties.xul | 2 +- browser/components/places/content/bookmarksSidebar.xul | 3 ++- browser/components/places/content/historySidebar.xul | 3 ++- browser/components/places/content/places.xul | 2 +- .../components/places/tests/chrome/test_0_bug510634.xul | 2 +- .../test_bug1163447_selectItems_through_shortcut.xul | 2 +- browser/components/places/tests/chrome/test_bug549192.xul | 2 +- browser/components/places/tests/chrome/test_bug549491.xul | 2 +- .../tests/chrome/test_selectItems_on_nested_tree.xul | 2 +- .../components/places/tests/chrome/test_treeview_date.xul | 2 +- browser/components/preferences/selectBookmark.xul | 2 +- browser/themes/linux/jar.mn | 2 +- browser/themes/linux/places/{places.css => sidebar.css} | 6 +----- browser/themes/osx/jar.mn | 2 +- browser/themes/osx/places/{places.css => sidebar.css} | 6 +----- browser/themes/shared/jar.inc.mn | 1 + .../shared/places/{places.inc.css => sidebar.inc.css} | 0 .../shared/places/{tree-icons.inc.css => tree-icons.css} | 5 ----- browser/themes/windows/jar.mn | 2 +- browser/themes/windows/places/{places.css => sidebar.css} | 8 +------- 21 files changed, 21 insertions(+), 37 deletions(-) rename browser/themes/linux/places/{places.css => sidebar.css} (90%) rename browser/themes/osx/places/{places.css => sidebar.css} (94%) rename browser/themes/shared/places/{places.inc.css => sidebar.inc.css} (100%) rename browser/themes/shared/places/{tree-icons.inc.css => tree-icons.css} (97%) rename browser/themes/windows/places/{places.css => sidebar.css} (90%) diff --git a/browser/base/content/browser.xul b/browser/base/content/browser.xul index 602d2f32deca..ff7f63fa293d 100644 --- a/browser/base/content/browser.xul +++ b/browser/base/content/browser.xul @@ -21,7 +21,7 @@ - + diff --git a/browser/components/places/content/bookmarkProperties.xul b/browser/components/places/content/bookmarkProperties.xul index 8330bfb759eb..cc79e777e5b5 100644 --- a/browser/components/places/content/bookmarkProperties.xul +++ b/browser/components/places/content/bookmarkProperties.xul @@ -6,7 +6,7 @@ - + - + + diff --git a/browser/components/places/content/historySidebar.xul b/browser/components/places/content/historySidebar.xul index 6c3232e22586..1afa866fd3f2 100644 --- a/browser/components/places/content/historySidebar.xul +++ b/browser/components/places/content/historySidebar.xul @@ -5,7 +5,8 @@ - + + diff --git a/browser/components/places/content/places.xul b/browser/components/places/content/places.xul index cbd25bc19c44..2fc2ce8d68c9 100644 --- a/browser/components/places/content/places.xul +++ b/browser/components/places/content/places.xul @@ -7,7 +7,7 @@ - + diff --git a/browser/components/places/tests/chrome/test_0_bug510634.xul b/browser/components/places/tests/chrome/test_0_bug510634.xul index d6953ccff588..a12c256d184a 100644 --- a/browser/components/places/tests/chrome/test_0_bug510634.xul +++ b/browser/components/places/tests/chrome/test_0_bug510634.xul @@ -9,7 +9,7 @@ type="text/css"?> - + - + - + - + - + - + - + Date: Tue, 21 Aug 2018 14:05:22 +0300 Subject: [PATCH 15/26] Backed out 2 changesets (bug 1443561) for causing bc perma failures in toolkit/components/extensions/test/browser/browser_ext_themes_sanitization.js Backed out changeset 1d748613485f (bug 1443561) Backed out changeset 37b4dc3bc73a (bug 1443561) --- browser/base/content/browser.css | 7 +- browser/themes/osx/browser.css | 16 --- browser/themes/shared/browser.inc.css | 9 -- .../components/extensions/parent/ext-theme.js | 9 +- .../extensions/test/browser/browser.ini | 1 - ...themes_additional_backgrounds_alignment.js | 105 ------------------ ...browser_ext_themes_multiple_backgrounds.js | 76 +++++-------- 7 files changed, 43 insertions(+), 180 deletions(-) delete mode 100644 toolkit/components/extensions/test/browser/browser_ext_themes_additional_backgrounds_alignment.js diff --git a/browser/base/content/browser.css b/browser/base/content/browser.css index 9966a9482d08..bb1361604962 100644 --- a/browser/base/content/browser.css +++ b/browser/base/content/browser.css @@ -8,6 +8,7 @@ :root { --panelui-subview-transition-duration: 150ms; --lwt-additional-images: none; + --lwt-background-alignment: right top; --lwt-background-tiling: no-repeat; } @@ -17,11 +18,13 @@ :root:-moz-lwtheme { background-color: var(--lwt-accent-color) !important; + background-image: var(--lwt-additional-images) !important; + background-position: var(--lwt-background-alignment) !important; + background-repeat: var(--lwt-background-tiling) !important; } :root:-moz-lwtheme[lwtheme-image] { - background-image: var(--lwt-header-image) !important; - background-position: right top !important; + background-image: var(--lwt-header-image), var(--lwt-additional-images) !important; } :root:-moz-lwtheme:-moz-window-inactive { diff --git a/browser/themes/osx/browser.css b/browser/themes/osx/browser.css index eaa2ecdf1d46..244399e87450 100644 --- a/browser/themes/osx/browser.css +++ b/browser/themes/osx/browser.css @@ -47,25 +47,9 @@ } #navigator-toolbox { - -moz-appearance: none; --tabs-border-color: rgba(0,0,0,.3); } -/* - This is a workaround for Bug 1482157 - -moz-appearance: toolbox; makes the macOS sheets attached to the element's - bottom border. We cannot put this property on the toolbox itself as it - cancels all backgrounds that are there, so we set it on the toolbox bottom - border. -*/ -#navigator-toolbox::after { - -moz-appearance: toolbox; - height: 1px; - /* use inset box-shadow instead of border because -moz-appearance hides the border */ - border: none; - box-shadow: inset 0 -1px var(--toolbox-border-bottom-color); -} - #tabbrowser-tabs { --tab-line-color: #0a84ff; } diff --git a/browser/themes/shared/browser.inc.css b/browser/themes/shared/browser.inc.css index 2fb5fc3ef010..848e315bb004 100644 --- a/browser/themes/shared/browser.inc.css +++ b/browser/themes/shared/browser.inc.css @@ -24,19 +24,10 @@ } /* Increase contrast of UI links on dark themes */ - :root[lwt-popup-brighttext] panel .text-link { color: @lwtPopupBrighttextLinkColor@; } -/* Set additional backgrounds alignment relative to toolbox*/ - -#navigator-toolbox:-moz-lwtheme { - background-image: var(--lwt-additional-images); - background-position: var(--lwt-background-alignment); - background-repeat: var(--lwt-background-tiling); -} - /* Toolbar / content area border */ #navigator-toolbox::after { diff --git a/toolkit/components/extensions/parent/ext-theme.js b/toolkit/components/extensions/parent/ext-theme.js index 25b2b0dee2e4..8a2dfc324a81 100644 --- a/toolkit/components/extensions/parent/ext-theme.js +++ b/toolkit/components/extensions/parent/ext-theme.js @@ -302,7 +302,11 @@ class Theme { break; } - this.lwtStyles.backgroundsAlignment = val.join(","); + let alignment = []; + if (this.lwtStyles.headerURL) { + alignment.push("right top"); + } + this.lwtStyles.backgroundsAlignment = alignment.concat(val).join(","); break; } case "additional_backgrounds_tiling": { @@ -311,6 +315,9 @@ class Theme { } let tiling = []; + if (this.lwtStyles.headerURL) { + tiling.push("no-repeat"); + } for (let i = 0, l = this.lwtStyles.additionalBackgrounds.length; i < l; ++i) { tiling.push(val[i] || "no-repeat"); } diff --git a/toolkit/components/extensions/test/browser/browser.ini b/toolkit/components/extensions/test/browser/browser.ini index adbc22daabfe..dd5db6b24716 100644 --- a/toolkit/components/extensions/test/browser/browser.ini +++ b/toolkit/components/extensions/test/browser/browser.ini @@ -4,7 +4,6 @@ support-files = [browser_ext_management_themes.js] skip-if = verify -[browser_ext_themes_additional_backgrounds_alignment.js] [browser_ext_themes_alpha_accentcolor.js] [browser_ext_themes_chromeparity.js] [browser_ext_themes_dynamic_getCurrent.js] diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_additional_backgrounds_alignment.js b/toolkit/components/extensions/test/browser/browser_ext_themes_additional_backgrounds_alignment.js deleted file mode 100644 index e1c67d2ff926..000000000000 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_additional_backgrounds_alignment.js +++ /dev/null @@ -1,105 +0,0 @@ -"use strict"; - -/* globals InspectorUtils */ - -// Case 1 - When there is a headerURL image and additional_backgrounds_alignment is not specified. -// So background-position should default to "left top" -add_task(async function test_default_additional_backgrounds_alignment() { - const LEFT_TOP = "0% 0%"; - const RIGHT_TOP = "100% 0%"; - - let extension = ExtensionTestUtils.loadExtension({ - manifest: { - "theme": { - "images": { - "headerURL": "image1.png", - "additional_backgrounds": ["image1.png", "image1.png"], - }, - "colors": { - "accentcolor": ACCENT_COLOR, - "textcolor": TEXT_COLOR, - }, - }, - }, - files: { - "image1.png": BACKGROUND, - }, - - }); - - await extension.startup(); - - let docEl = document.documentElement; - let rootCS = window.getComputedStyle(docEl); - - Assert.equal( - rootCS.getPropertyValue("background-position"), - RIGHT_TOP, - "root only contains headerURL alignment property" - ); - - - let toolbox = document.querySelector("#navigator-toolbox"); - let toolboxCS = window.getComputedStyle(toolbox); - - Assert.equal( - toolboxCS.getPropertyValue("background-position"), - LEFT_TOP, - toolbox.id + " only contains default additional backgrounds alignment property" - ); - - await extension.unload(); -}); - - -// Case 2 - When there is a headerURL image and additional_backgrounds_alignment is specified. -add_task(async function test_additional_backgrounds_alignment() { - const LEFT_BOTTOM = "0% 100%"; - const CENTER_CENTER = "50% 50%"; - const RIGHT_TOP = "100% 0%"; - - let extension = ExtensionTestUtils.loadExtension({ - manifest: { - "theme": { - "images": { - "headerURL": "image1.png", - "additional_backgrounds": ["image1.png", "image1.png", "image1.png"], - }, - "colors": { - "accentcolor": ACCENT_COLOR, - "textcolor": TEXT_COLOR, - }, - "properties": { - additional_backgrounds_alignment: ["left bottom", "center center", "right top"], - }, - }, - }, - files: { - "image1.png": BACKGROUND, - }, - - }); - - await extension.startup(); - - let docEl = document.documentElement; - let rootCS = window.getComputedStyle(docEl); - - Assert.equal( - rootCS.getPropertyValue("background-position"), - RIGHT_TOP, - "root only contains headerURL alignment property" - ); - - - let toolbox = document.querySelector("#navigator-toolbox"); - let toolboxCS = window.getComputedStyle(toolbox); - - Assert.equal( - toolboxCS.getPropertyValue("background-position"), - LEFT_BOTTOM + ", " + CENTER_CENTER + ", " + RIGHT_TOP, - toolbox.id + " contains additional backgrounds alignment properties" - ); - - await extension.unload(); -}); diff --git a/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js b/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js index f58545a094f9..4b2e82ad018f 100644 --- a/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js +++ b/toolkit/components/extensions/test/browser/browser_ext_themes_multiple_backgrounds.js @@ -5,8 +5,8 @@ add_task(async function test_support_backgrounds_position() { manifest: { "theme": { "images": { - "headerURL": "face1.png", - "additional_backgrounds": ["face2.png", "face2.png", "face2.png"], + "headerURL": "face.png", + "additional_backgrounds": ["face.png", "face.png", "face.png"], }, "colors": { "accentcolor": `rgb(${FRAME_COLOR.join(",")})`, @@ -18,45 +18,37 @@ add_task(async function test_support_backgrounds_position() { }, }, files: { - "face1.png": imageBufferFromDataURI(ENCODED_IMAGE_DATA), - "face2.png": imageBufferFromDataURI(ENCODED_IMAGE_DATA), + "face.png": imageBufferFromDataURI(ENCODED_IMAGE_DATA), }, }); await extension.startup(); let docEl = window.document.documentElement; - let toolbox = document.querySelector("#navigator-toolbox"); Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set"); Assert.equal(docEl.getAttribute("lwthemetextcolor"), "bright", "LWT text color attribute should be set"); - let toolboxCS = window.getComputedStyle(toolbox); - let rootCS = window.getComputedStyle(docEl); - let rootBgImage = rootCS.backgroundImage.split(",")[0].trim(); - let bgImage = toolboxCS.backgroundImage.split(",")[0].trim(); - Assert.ok(rootBgImage.includes("face1.png"), - `The backgroundImage should use face1.png. Actual value is: ${rootBgImage}`); - Assert.equal(toolboxCS.backgroundImage, Array(3).fill(bgImage).join(", "), - "The backgroundImage should use face2.png three times."); - Assert.equal(toolboxCS.backgroundPosition, "0% 0%, 50% 0%, 100% 100%", - "The backgroundPosition should use the three values provided."); - Assert.equal(toolboxCS.backgroundRepeat, "no-repeat", + let style = window.getComputedStyle(docEl); + let bgImage = style.backgroundImage.split(",")[0].trim(); + Assert.ok(bgImage.includes("face.png"), + `The backgroundImage should use face.png. Actual value is: ${bgImage}`); + Assert.equal(Array(4).fill(bgImage).join(", "), style.backgroundImage, + "The backgroundImage should use face.png four times."); + Assert.equal(style.backgroundPosition, "100% 0%, 0% 0%, 50% 0%, 100% 100%", + "The backgroundPosition should use the four values provided."); + Assert.equal(style.backgroundRepeat, "no-repeat", "The backgroundPosition should use the default value."); await extension.unload(); Assert.ok(!docEl.hasAttribute("lwtheme"), "LWT attribute should not be set"); - toolboxCS = window.getComputedStyle(toolbox); - + style = window.getComputedStyle(docEl); // Styles should've reverted to their initial values. - Assert.equal(rootCS.backgroundImage, "none"); - Assert.equal(rootCS.backgroundPosition, "0% 0%"); - Assert.equal(rootCS.backgroundRepeat, "repeat"); - Assert.equal(toolboxCS.backgroundImage, "none"); - Assert.equal(toolboxCS.backgroundPosition, "0% 0%"); - Assert.equal(toolboxCS.backgroundRepeat, "repeat"); + Assert.equal(style.backgroundImage, "none"); + Assert.equal(style.backgroundPosition, "0% 0%"); + Assert.equal(style.backgroundRepeat, "repeat"); }); add_task(async function test_support_backgrounds_repeat() { @@ -87,27 +79,21 @@ add_task(async function test_support_backgrounds_repeat() { await extension.startup(); let docEl = window.document.documentElement; - let toolbox = document.querySelector("#navigator-toolbox"); Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set"); Assert.equal(docEl.getAttribute("lwthemetextcolor"), "bright", "LWT text color attribute should be set"); - let rootCS = window.getComputedStyle(docEl); - let toolboxCS = window.getComputedStyle(toolbox); - let bgImage = rootCS.backgroundImage.split(",")[0].trim(); + let style = window.getComputedStyle(docEl); + let bgImage = style.backgroundImage.split(",")[0].trim(); Assert.ok(bgImage.includes("face0.png"), `The backgroundImage should use face.png. Actual value is: ${bgImage}`); - Assert.equal([1, 2, 3].map(num => bgImage.replace(/face[\d]*/, `face${num}`)).join(", "), - toolboxCS.backgroundImage, "The backgroundImage should use face.png three times."); - Assert.equal(rootCS.backgroundPosition, "100% 0%", - "The backgroundPosition should use the default value for root."); - Assert.equal(toolboxCS.backgroundPosition, "0% 0%", - "The backgroundPosition should use the default value for navigator-toolbox."); - Assert.equal(rootCS.backgroundRepeat, "no-repeat", - "The backgroundRepeat should use the default values for root."); - Assert.equal(toolboxCS.backgroundRepeat, "repeat-x, repeat-y, repeat", - "The backgroundRepeat should use the three values provided for navigator-toolbox."); + Assert.equal([0, 1, 2, 3].map(num => bgImage.replace(/face[\d]*/, `face${num}`)).join(", "), + style.backgroundImage, "The backgroundImage should use face.png four times."); + Assert.equal(style.backgroundPosition, "100% 0%", + "The backgroundPosition should use the default value."); + Assert.equal(style.backgroundRepeat, "no-repeat, repeat-x, repeat-y, repeat", + "The backgroundPosition should use the four values provided."); await extension.unload(); @@ -138,22 +124,20 @@ add_task(async function test_additional_images_check() { await extension.startup(); let docEl = window.document.documentElement; - let toolbox = document.querySelector("#navigator-toolbox"); Assert.ok(docEl.hasAttribute("lwtheme"), "LWT attribute should be set"); Assert.equal(docEl.getAttribute("lwthemetextcolor"), "bright", "LWT text color attribute should be set"); - let rootCS = window.getComputedStyle(docEl); - let toolboxCS = window.getComputedStyle(toolbox); - let bgImage = rootCS.backgroundImage.split(",")[0]; + let style = window.getComputedStyle(docEl); + let bgImage = style.backgroundImage.split(",")[0]; Assert.ok(bgImage.includes("face.png"), `The backgroundImage should use face.png. Actual value is: ${bgImage}`); - Assert.equal("none", toolboxCS.backgroundImage, - "The backgroundImage should not use face.png."); - Assert.equal(rootCS.backgroundPosition, "100% 0%", + Assert.equal(bgImage + ", none", style.backgroundImage, + "The backgroundImage should use face.png only once."); + Assert.equal(style.backgroundPosition, "100% 0%", "The backgroundPosition should use the default value."); - Assert.equal(rootCS.backgroundRepeat, "no-repeat", + Assert.equal(style.backgroundRepeat, "no-repeat", "The backgroundPosition should use only one (default) value."); await extension.unload(); From c5d3bd801c2220a18e14975057385b87a332d0c8 Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Tue, 21 Aug 2018 08:07:23 +0200 Subject: [PATCH 16/26] Bug 1484909 - [marionette] Fix handling of InvalidSessionException. The Marionette client raises a custom MarionetteException instead of passing through the InvalidSessionIdException as returned by the Marionette server. --HG-- extra : rebase_source : 99fd02062e858f31e25d2ca0fb80940586772fb2 --- .../marionette/client/marionette_driver/marionette.py | 8 +++++--- .../marionette_harness/tests/unit/test_crash.py | 10 +++++----- .../marionette_harness/tests/unit/test_quit_restart.py | 8 ++++---- .../marionette_harness/tests/unit/test_session.py | 2 +- testing/marionette/server.js | 5 +++-- 5 files changed, 18 insertions(+), 15 deletions(-) diff --git a/testing/marionette/client/marionette_driver/marionette.py b/testing/marionette/client/marionette_driver/marionette.py index 93dbb908a308..1d31102db3e5 100644 --- a/testing/marionette/client/marionette_driver/marionette.py +++ b/testing/marionette/client/marionette_driver/marionette.py @@ -740,9 +740,8 @@ class Marionette(object): :returns: Full response from the server, or if `key` is given, the value of said key in the response. """ - if not self.session_id and name != "WebDriver:NewSession": - raise errors.MarionetteException("Please start a session") + raise errors.InvalidSessionIdException("Please start a session") try: msg = self.client.request(name, params) @@ -1277,7 +1276,10 @@ class Marionette(object): """ try: if send_request: - self._send_message("WebDriver:DeleteSession") + try: + self._send_message("WebDriver:DeleteSession") + except errors.InvalidSessionIdException: + pass finally: self.process_id = None self.profile = None diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_crash.py b/testing/marionette/harness/marionette_harness/tests/unit/test_crash.py index 5842fb3a59ee..e1da5a32c19e 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_crash.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_crash.py @@ -10,7 +10,7 @@ import shutil from marionette_driver import Wait from marionette_driver.errors import ( - MarionetteException, + InvalidSessionIdException, NoSuchWindowException, TimeoutException ) @@ -106,8 +106,8 @@ class TestCrash(BaseCrashTestCase): self.assertEqual(self.marionette.crashed, 1) self.assertIsNone(self.marionette.session) - self.assertRaisesRegexp(MarionetteException, 'Please start a session', - self.marionette.get_url) + with self.assertRaisesRegexp(InvalidSessionIdException, 'Please start a session'): + self.marionette.get_url() self.marionette.start_session() self.assertNotEqual(self.marionette.process_id, self.pid) @@ -134,8 +134,8 @@ class TestCrash(BaseCrashTestCase): self.assertEqual(self.marionette.crashed, 1) self.assertIsNone(self.marionette.session) - self.assertRaisesRegexp(MarionetteException, 'Please start a session', - self.marionette.get_url) + with self.assertRaisesRegexp(InvalidSessionIdException, 'Please start a session'): + self.marionette.get_url() self.marionette.start_session() self.assertNotEqual(self.marionette.process_id, self.pid) diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py b/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py index 6248bcf7acd5..d94fd4123ddf 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_quit_restart.py @@ -140,7 +140,7 @@ class TestQuitRestart(MarionetteTestCase): self.marionette.quit(clean=True) self.assertEqual(self.marionette.session, None) - with self.assertRaisesRegexp(errors.MarionetteException, "Please start a session"): + with self.assertRaisesRegexp(errors.InvalidSessionIdException, "Please start a session"): self.marionette.get_url() self.marionette.start_session() @@ -153,7 +153,7 @@ class TestQuitRestart(MarionetteTestCase): self.marionette.quit() self.assertEqual(self.marionette.session, None) - with self.assertRaisesRegexp(errors.MarionetteException, "Please start a session"): + with self.assertRaisesRegexp(errors.InvalidSessionIdException, "Please start a session"): self.marionette.get_url() self.marionette.start_session() @@ -245,7 +245,7 @@ class TestQuitRestart(MarionetteTestCase): self.marionette.quit(in_app=True) self.assertEqual(self.marionette.session, None) - with self.assertRaisesRegexp(errors.MarionetteException, "Please start a session"): + with self.assertRaisesRegexp(errors.InvalidSessionIdException, "Please start a session"): self.marionette.get_url() self.marionette.start_session() @@ -257,7 +257,7 @@ class TestQuitRestart(MarionetteTestCase): def test_in_app_quit_with_callback(self): self.marionette.quit(in_app=True, callback=self.shutdown) self.assertEqual(self.marionette.session, None) - with self.assertRaisesRegexp(errors.MarionetteException, "Please start a session"): + with self.assertRaisesRegexp(errors.InvalidSessionIdException, "Please start a session"): self.marionette.get_url() self.marionette.start_session() diff --git a/testing/marionette/harness/marionette_harness/tests/unit/test_session.py b/testing/marionette/harness/marionette_harness/tests/unit/test_session.py index 78c1fdd4b2bd..aeab07523022 100644 --- a/testing/marionette/harness/marionette_harness/tests/unit/test_session.py +++ b/testing/marionette/harness/marionette_harness/tests/unit/test_session.py @@ -47,7 +47,7 @@ class TestSession(MarionetteTestCase): self.marionette._send_message("WebDriver:NewSession", {}) def test_no_session(self): - with self.assertRaisesRegexp(errors.MarionetteException, "Please start a session"): + with self.assertRaises(errors.InvalidSessionIdException): self.marionette.get_url() self.marionette.start_session() diff --git a/testing/marionette/server.js b/testing/marionette/server.js index fb19af591244..159e537aced5 100644 --- a/testing/marionette/server.js +++ b/testing/marionette/server.js @@ -290,8 +290,9 @@ class TCPConnection { throw new UnknownCommandError(cmd.name); } - if (!["newSession", "WebDriver:NewSession"].includes(cmd.name)) { - assert.session(this.driver); + if (cmd.name != "WebDriver:NewSession") { + assert.session(this.driver, + "Tried to run command without establishing a connection"); } let rv = await fn.bind(this.driver)(cmd); From 3af25764b7736eac82419acb1841cd43b00c8ee0 Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Tue, 21 Aug 2018 09:08:36 +0200 Subject: [PATCH 17/26] Bug 1484909 - [geckodriver] Return "invalid session id" error when there is no active session. If a command is used before creating a new session, an "invalid session id" error has to be returned by the driver. --HG-- extra : rebase_source : c45f33e9e39d876f5fd77561b57ab1fe664452d1 --- testing/geckodriver/src/marionette.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/geckodriver/src/marionette.rs b/testing/geckodriver/src/marionette.rs index 695ca34df5de..cbb40c2050ff 100644 --- a/testing/geckodriver/src/marionette.rs +++ b/testing/geckodriver/src/marionette.rs @@ -554,7 +554,7 @@ impl WebDriverHandler for MarionetteHandler { }, _ => { return Err(WebDriverError::new( - ErrorStatus::SessionNotCreated, + ErrorStatus::InvalidSessionId, "Tried to run command without establishing a connection")); } } From 8c777853e7b55cb811ff3a9a90fcd6bada4bc277 Mon Sep 17 00:00:00 2001 From: Henrik Skupin Date: Tue, 21 Aug 2018 09:15:21 +0200 Subject: [PATCH 18/26] Bug 1484909 - [wdclient] Fix handling of "invalid session id" error. Without a session being created first the "send_command" method inappropriately checks for the "session not created" error, which only gets raised by the "new session" command. For all the other commands the "invalid session id" error has to be handled. --HG-- extra : rebase_source : de7ad6ae0ef9af925f6f36c70cb4a85c97da4b0c --- .../tests/delete_session/delete.py.ini | 4 ---- .../tests/tools/webdriver/webdriver/client.py | 22 ++++++++++--------- .../webdriver/tests/delete_session/delete.py | 1 + 3 files changed, 13 insertions(+), 14 deletions(-) delete mode 100644 testing/web-platform/meta/webdriver/tests/delete_session/delete.py.ini diff --git a/testing/web-platform/meta/webdriver/tests/delete_session/delete.py.ini b/testing/web-platform/meta/webdriver/tests/delete_session/delete.py.ini deleted file mode 100644 index 34140802ec68..000000000000 --- a/testing/web-platform/meta/webdriver/tests/delete_session/delete.py.ini +++ /dev/null @@ -1,4 +0,0 @@ -[delete.py] - [test_dismissed_beforeunload_prompt] - expected: FAIL - diff --git a/testing/web-platform/tests/tools/webdriver/webdriver/client.py b/testing/web-platform/tests/tools/webdriver/webdriver/client.py index a8c9a061ccf5..34d1a90dde2d 100644 --- a/testing/web-platform/tests/tools/webdriver/webdriver/client.py +++ b/testing/web-platform/tests/tools/webdriver/webdriver/client.py @@ -404,6 +404,13 @@ class Session(object): self.end() def start(self): + """Start a new WebDriver session. + + :return: Dictionary with `capabilities` and `sessionId`. + + :raises error.WebDriverException: If the remote end returns + an error. + """ if self.session_id is not None: return @@ -422,13 +429,13 @@ class Session(object): return value def end(self): - """Tries to close the active session.""" + """Try to close the active session.""" if self.session_id is None: return try: self.send_command("DELETE", "session/%s" % self.session_id) - except error.SessionNotCreatedException: + except error.InvalidSessionIdException: pass finally: self.session_id = None @@ -446,10 +453,10 @@ class Session(object): the `value` field returned after parsing the response body as JSON. - :raises ValueError: If the response body does not contain a - `value` key. :raises error.WebDriverException: If the remote end returns an error. + :raises ValueError: If the response body does not contain a + `value` key. """ response = self.transport.send( method, url, body, @@ -459,7 +466,7 @@ class Session(object): if response.status != 200: err = error.from_response(response) - if isinstance(err, error.SessionNotCreatedException): + if isinstance(err, error.InvalidSessionIdException): # The driver could have already been deleted the session. self.session_id = None @@ -495,14 +502,9 @@ class Session(object): :return: `None` if the HTTP response body was empty, otherwise the result of parsing the body as JSON. - :raises error.SessionNotCreatedException: If there is no active - session. :raises error.WebDriverException: If the remote end returns an error. """ - if self.session_id is None: - raise error.SessionNotCreatedException() - url = urlparse.urljoin("session/%s/" % self.session_id, uri) return self.send_command(method, url, body) diff --git a/testing/web-platform/tests/webdriver/tests/delete_session/delete.py b/testing/web-platform/tests/webdriver/tests/delete_session/delete.py index 835f25257921..d0b4d796308e 100644 --- a/testing/web-platform/tests/webdriver/tests/delete_session/delete.py +++ b/testing/web-platform/tests/webdriver/tests/delete_session/delete.py @@ -13,6 +13,7 @@ def test_null_response_value(session): response = delete_session(session) value = assert_success(response) assert value is None + # Need an explicit call to session.end() to notify the test harness # that a new session needs to be created for subsequent tests. session.end() From a5f0fee21556cc0dbc1189f4d9713dcf6e488645 Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Wed, 1 Aug 2018 14:16:30 +0200 Subject: [PATCH 19/26] Bug 1480036 - Allow forcing a specific audio output device from a test. r=pehrsons Differential Revision: https://phabricator.services.mozilla.com/D3507 --HG-- extra : rebase_source : 2f20dcd24d148ea0cc9f7b73ff0dd81f25574467 extra : histedit_source : aa2205caf3ad5fe9e5fceaf78e6f7a7e3488155b --- dom/media/CubebUtils.cpp | 48 ++++++++++++++++++++++++--------------- dom/media/CubebUtils.h | 1 + dom/media/GraphDriver.cpp | 18 ++++++++++++++- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/dom/media/CubebUtils.cpp b/dom/media/CubebUtils.cpp index 5fec6e84bb3a..d4b553e5e730 100644 --- a/dom/media/CubebUtils.cpp +++ b/dom/media/CubebUtils.cpp @@ -35,6 +35,7 @@ #define PREF_VOLUME_SCALE "media.volume_scale" #define PREF_CUBEB_BACKEND "media.cubeb.backend" +#define PREF_CUBEB_OUTPUT_DEVICE "media.cubeb.output_device" #define PREF_CUBEB_LATENCY_PLAYBACK "media.cubeb_latency_playback_ms" #define PREF_CUBEB_LATENCY_MSG "media.cubeb_latency_msg_frames" // Allows to get something non-default for the preferred sample-rate, to allow @@ -142,6 +143,7 @@ size_t sAudioIPCStackSize; #endif StaticAutoPtr sBrandName; StaticAutoPtr sCubebBackendName; +StaticAutoPtr sCubebOutputDeviceName; const char kBrandBundleURL[] = "chrome://branding/locale/brand.properties"; @@ -188,6 +190,19 @@ static const uint32_t CUBEB_NORMAL_LATENCY_FRAMES = 1024; namespace CubebUtils { cubeb* GetCubebContextUnlocked(); +void GetPrefAndSetString(const char* aPref, StaticAutoPtr& aStorage) +{ + nsAutoCString value; + Preferences::GetCString(aPref, value); + if (value.IsEmpty()) { + aStorage = nullptr; + } else { + aStorage = new char[value.Length() + 1]; + PodCopy(aStorage.get(), value.get(), value.Length()); + aStorage[value.Length()] = 0; + } +} + void PrefChanged(const char* aPref, void* aClosure) { if (strcmp(aPref, PREF_VOLUME_SCALE) == 0) { @@ -235,15 +250,10 @@ void PrefChanged(const char* aPref, void* aClosure) } } else if (strcmp(aPref, PREF_CUBEB_BACKEND) == 0) { StaticMutexAutoLock lock(sMutex); - nsAutoCString value; - Preferences::GetCString(aPref, value); - if (value.IsEmpty()) { - sCubebBackendName = nullptr; - } else { - sCubebBackendName = new char[value.Length() + 1]; - PodCopy(sCubebBackendName.get(), value.get(), value.Length()); - sCubebBackendName[value.Length()] = 0; - } + GetPrefAndSetString(aPref, sCubebBackendName); + } else if (strcmp(aPref, PREF_CUBEB_OUTPUT_DEVICE) == 0) { + StaticMutexAutoLock lock(sMutex); + GetPrefAndSetString(aPref, sCubebOutputDeviceName); } else if (strcmp(aPref, PREF_CUBEB_FORCE_NULL_CONTEXT) == 0) { StaticMutexAutoLock lock(sMutex); sCubebForceNullContext = Preferences::GetBool(aPref, false); @@ -538,15 +548,11 @@ uint32_t GetCubebMSGLatencyInFrames(cubeb_stream_params * params) } static const char* gInitCallbackPrefs[] = { - PREF_VOLUME_SCALE, - PREF_CUBEB_LATENCY_PLAYBACK, - PREF_CUBEB_LATENCY_MSG, - PREF_CUBEB_BACKEND, - PREF_CUBEB_FORCE_NULL_CONTEXT, - PREF_CUBEB_SANDBOX, - PREF_AUDIOIPC_POOL_SIZE, - PREF_AUDIOIPC_STACK_SIZE, - nullptr, + PREF_VOLUME_SCALE, PREF_CUBEB_OUTPUT_DEVICE, + PREF_CUBEB_LATENCY_PLAYBACK, PREF_CUBEB_LATENCY_MSG, + PREF_CUBEB_BACKEND, PREF_CUBEB_FORCE_NULL_CONTEXT, + PREF_CUBEB_SANDBOX, PREF_AUDIOIPC_POOL_SIZE, + PREF_AUDIOIPC_STACK_SIZE, nullptr, }; static const char* gCallbackPrefs[] = { PREF_CUBEB_FORCE_SAMPLE_RATE, @@ -627,6 +633,12 @@ void GetCurrentBackend(nsAString& aBackend) aBackend.AssignLiteral("unknown"); } +char* GetForcedOutputDevice() +{ + StaticMutexAutoLock lock(sMutex); + return sCubebOutputDeviceName; +} + uint16_t ConvertCubebType(cubeb_device_type aType) { uint16_t map[] = { diff --git a/dom/media/CubebUtils.h b/dom/media/CubebUtils.h index 2c5bcb1dd849..eafafd00db22 100644 --- a/dom/media/CubebUtils.h +++ b/dom/media/CubebUtils.h @@ -49,6 +49,7 @@ void GetCurrentBackend(nsAString& aBackend); void GetDeviceCollection(nsTArray>& aDeviceInfos, Side aSide); cubeb_stream_prefs GetDefaultStreamPrefs(); +char* GetForcedOutputDevice(); #ifdef MOZ_WIDGET_ANDROID uint32_t AndroidGetAudioOutputSampleRate(); diff --git a/dom/media/GraphDriver.cpp b/dom/media/GraphDriver.cpp index 2a44261843b5..34eee89bdda2 100644 --- a/dom/media/GraphDriver.cpp +++ b/dom/media/GraphDriver.cpp @@ -659,6 +659,22 @@ AudioCallbackDriver::Init() return true; } + CubebUtils::AudioDeviceID forcedOutputDeviceId = nullptr; + + char* forcedOutputDeviceName = CubebUtils::GetForcedOutputDevice(); + if (forcedOutputDeviceName) { + nsTArray> deviceInfos; + GetDeviceCollection(deviceInfos, CubebUtils::Output); + for (const auto& device : deviceInfos) { + const nsString& name = device->Name(); + if (name.Equals(NS_ConvertUTF8toUTF16(forcedOutputDeviceName))) { + if (device->DeviceID()) { + forcedOutputDeviceId = device->DeviceID(); + } + } + } + } + mBuffer = AudioCallbackBufferWrapper(mOutputChannels); mScratchBuffer = SpillBuffer(mOutputChannels); @@ -690,7 +706,7 @@ AudioCallbackDriver::Init() "AudioCallbackDriver", input_id, inputWanted ? &input : nullptr, - output_id, + forcedOutputDeviceId ? forcedOutputDeviceId : output_id, &output, latency_frames, DataCallback_s, From 9d28010d74397c6528f3f66db354b01c72dbbdee Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Wed, 1 Aug 2018 15:18:32 +0200 Subject: [PATCH 20/26] Bug 1480036 - Stop relying on having the default device set globally. r=pehrsons Differential Revision: https://phabricator.services.mozilla.com/D3508 --HG-- extra : rebase_source : 273766c52eb01dd3a8f3a42fc72d7c8997e5b2a9 extra : histedit_source : eab93fe060536497b1e68fc4d964eb42c61ca029 --- testing/mochitest/runtests.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/testing/mochitest/runtests.py b/testing/mochitest/runtests.py index b6803ab6b8ff..5b5017923f86 100644 --- a/testing/mochitest/runtests.py +++ b/testing/mochitest/runtests.py @@ -759,7 +759,6 @@ def findTestMediaDevices(log): info['video'] = name pactl = spawn.find_executable("pactl") - pacmd = spawn.find_executable("pacmd") # Use pactl to see if the PulseAudio module-null-sink module is loaded. def null_sink_loaded(): @@ -776,9 +775,6 @@ def findTestMediaDevices(log): log.error('Couldn\'t load module-null-sink') return None - # Whether it was loaded or not make it the default output - subprocess.check_call([pacmd, 'set-default-sink', 'null']) - # Hardcode the name since it's always the same. info['audio'] = 'Monitor of Null Output' return info @@ -1930,6 +1926,7 @@ toolbar#nav-bar { if options.useTestMediaDevices: prefs['media.audio_loopback_dev'] = self.mediaDevices['audio'] prefs['media.video_loopback_dev'] = self.mediaDevices['video'] + prefs['media.cubeb.output_device'] = "Null Output" # Disable web replay rewinding by default if recordings are being saved. if options.recordingPath: From 3f50ca3b6facf9d7aaa6280d6714fbe997b07972 Mon Sep 17 00:00:00 2001 From: Paul Adenot Date: Wed, 1 Aug 2018 12:10:32 +0200 Subject: [PATCH 21/26] Bug 1480036 - Allow scaling all MSG volume with a pref. r=pehrsons Differential Revision: https://phabricator.services.mozilla.com/D2594 --HG-- extra : rebase_source : 12a931585d95fe190e6fc1e3b4fba9a2f0cd6cd9 extra : histedit_source : 355f316e6eba1feaa0ee3e7750feede92eeae4bc --- dom/media/MediaStreamGraph.cpp | 3 ++- dom/media/MediaStreamGraphImpl.h | 6 ++++++ dom/media/tests/mochitest/head.js | 6 ------ testing/mochitest/runtests.py | 1 + 4 files changed, 9 insertions(+), 7 deletions(-) diff --git a/dom/media/MediaStreamGraph.cpp b/dom/media/MediaStreamGraph.cpp index df60058d0365..964c237f9e5a 100644 --- a/dom/media/MediaStreamGraph.cpp +++ b/dom/media/MediaStreamGraph.cpp @@ -681,7 +681,7 @@ MediaStreamGraphImpl::PlayAudio(MediaStream* aStream) float volume = 0.0f; for (uint32_t i = 0; i < aStream->mAudioOutputs.Length(); ++i) { - volume += aStream->mAudioOutputs[i].mVolume; + volume += aStream->mAudioOutputs[i].mVolume * mGlobalVolume; } StreamTime ticksWritten = 0; @@ -3711,6 +3711,7 @@ MediaStreamGraphImpl::MediaStreamGraphImpl(GraphDriverType aDriverRequested, , mAbstractMainThread(aMainThread) , mSelfRef(this) , mOutputChannels(std::min(8, CubebUtils::MaxNumberOfChannels())) + , mGlobalVolume(CubebUtils::GetVolumeScale()) #ifdef DEBUG , mCanRunMessagesSynchronously(false) #endif diff --git a/dom/media/MediaStreamGraphImpl.h b/dom/media/MediaStreamGraphImpl.h index 3d753a11dda2..3aa49c50613f 100644 --- a/dom/media/MediaStreamGraphImpl.h +++ b/dom/media/MediaStreamGraphImpl.h @@ -922,6 +922,12 @@ private: */ const uint32_t mOutputChannels; + /** + * Global volume scale. Used when running tests so that the output is not too + * loud. + */ + const float mGlobalVolume; + #ifdef DEBUG /** * Used to assert when AppendMessage() runs ControlMessages synchronously. diff --git a/dom/media/tests/mochitest/head.js b/dom/media/tests/mochitest/head.js index 7cf4b4b2c6f1..5159024fcba7 100644 --- a/dom/media/tests/mochitest/head.js +++ b/dom/media/tests/mochitest/head.js @@ -426,12 +426,6 @@ function setupEnvironment() { ] }; - if (!WANT_FAKE_AUDIO) { - defaultMochitestPrefs.set.push( - ["media.volume_scale", "1"], - ); - } - const isAndroid = !!navigator.userAgent.includes("Android"); if (isAndroid) { diff --git a/testing/mochitest/runtests.py b/testing/mochitest/runtests.py index 5b5017923f86..d390b8ccd2b4 100644 --- a/testing/mochitest/runtests.py +++ b/testing/mochitest/runtests.py @@ -1927,6 +1927,7 @@ toolbar#nav-bar { prefs['media.audio_loopback_dev'] = self.mediaDevices['audio'] prefs['media.video_loopback_dev'] = self.mediaDevices['video'] prefs['media.cubeb.output_device'] = "Null Output" + prefs['media.volume_scale'] = "1.0" # Disable web replay rewinding by default if recordings are being saved. if options.recordingPath: From 483b00ee423c7bdd3f237fcd73caee2f534e3122 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Thu, 16 Aug 2018 11:59:19 +0200 Subject: [PATCH 22/26] Bug 1479430 - Make sure TypeNewScript::maybeAnalyze is called in the group's realm. r=luke --HG-- extra : rebase_source : e6f8f7ff651a4f3bd07060152e15738b0de9dd36 --- js/src/jit-test/tests/realms/bug1479430.js | 6 ++++++ js/src/jit/Ion.cpp | 1 + 2 files changed, 7 insertions(+) create mode 100644 js/src/jit-test/tests/realms/bug1479430.js diff --git a/js/src/jit-test/tests/realms/bug1479430.js b/js/src/jit-test/tests/realms/bug1479430.js new file mode 100644 index 000000000000..a7bd9cc075bb --- /dev/null +++ b/js/src/jit-test/tests/realms/bug1479430.js @@ -0,0 +1,6 @@ +function f(a) { + return a.toString(); +} +var g = newGlobal({sameCompartmentAs: this}); +g.evaluate("function Obj() {}"); +f(f(new g.Obj())); diff --git a/js/src/jit/Ion.cpp b/js/src/jit/Ion.cpp index eb91637dfa5d..0562887a2ab6 100644 --- a/js/src/jit/Ion.cpp +++ b/js/src/jit/Ion.cpp @@ -2087,6 +2087,7 @@ IonCompile(JSContext* cx, JSScript* script, const MIRGenerator::ObjectGroupVector& groups = builder->abortedPreliminaryGroups(); for (size_t i = 0; i < groups.length(); i++) { ObjectGroup* group = groups[i]; + AutoRealm ar(cx, group); AutoSweepObjectGroup sweep(group); if (auto* newScript = group->newScript(sweep)) { if (!newScript->maybeAnalyze(cx, group, nullptr, /* force = */ true)) From 965f93349eb1b424085fec945fab67df20c4be27 Mon Sep 17 00:00:00 2001 From: Jan de Mooij Date: Thu, 16 Aug 2018 12:35:20 +0200 Subject: [PATCH 23/26] Bug 1473957 - Require debugger and debuggee to be in different compartments. r=jimb We were checking for cross-compartment wrappers in the Debugger constructor, but this patch also fixes addDebuggee and addAllGlobalsAsDebuggees. Differential Revision: https://phabricator.services.mozilla.com/D3495 --HG-- extra : rebase_source : 3346baa677b4eae1ed8b7b13d93c1c8c89753d97 --- .../tests/debug/Debugger-debuggees-30.js | 32 +++++++++++++++++++ js/src/js.msg | 1 + js/src/vm/Debugger.cpp | 16 +++++++--- 3 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 js/src/jit-test/tests/debug/Debugger-debuggees-30.js diff --git a/js/src/jit-test/tests/debug/Debugger-debuggees-30.js b/js/src/jit-test/tests/debug/Debugger-debuggees-30.js new file mode 100644 index 000000000000..da80f5398356 --- /dev/null +++ b/js/src/jit-test/tests/debug/Debugger-debuggees-30.js @@ -0,0 +1,32 @@ +// Debugger and debuggees must be in different compartments. + +load(libdir + "asserts.js"); + +function testConstructor() { + var g = newGlobal({sameCompartmentAs: this}); + assertTypeErrorMessage(() => new Debugger(g), + "Debugger: argument must be an object from a different compartment"); +} +testConstructor(); + +function testAddDebuggee() { + var g = newGlobal({sameCompartmentAs: this}); + var dbg = new Debugger(); + assertTypeErrorMessage(() => dbg.addDebuggee(this), + "debugger and debuggee must be in different compartments"); +} +testAddDebuggee(); + +function testAddAllGlobalsAsDebuggees() { + var g1 = newGlobal({sameCompartmentAs: this}); + var g2 = newGlobal(); + var g3 = newGlobal({sameCompartmentAs: g2}); + var g4 = newGlobal({sameZoneAs: this}); + var dbg = new Debugger(); + dbg.addAllGlobalsAsDebuggees(); + assertEq(dbg.hasDebuggee(g1), false); + assertEq(dbg.hasDebuggee(g2), true); + assertEq(dbg.hasDebuggee(g3), true); + assertEq(dbg.hasDebuggee(g4), true); +} +testAddAllGlobalsAsDebuggees(); diff --git a/js/src/js.msg b/js/src/js.msg index 7aede21ac346..1248b773e585 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -461,6 +461,7 @@ MSG_DEF(JSMSG_DEBUG_BAD_REFERENT, 2, JSEXN_TYPEERR, "{0} does not refer to MSG_DEF(JSMSG_DEBUG_BAD_RESUMPTION, 0, JSEXN_TYPEERR, "debugger resumption value must be undefined, {throw: val}, {return: val}, or null") MSG_DEF(JSMSG_DEBUG_BAD_YIELD, 0, JSEXN_TYPEERR, "generator yielded invalid value") MSG_DEF(JSMSG_DEBUG_CANT_DEBUG_GLOBAL, 0, JSEXN_TYPEERR, "passing non-debuggable global to addDebuggee") +MSG_DEF(JSMSG_DEBUG_SAME_COMPARTMENT, 0, JSEXN_TYPEERR, "debugger and debuggee must be in different compartments") MSG_DEF(JSMSG_DEBUG_CCW_REQUIRED, 1, JSEXN_TYPEERR, "{0}: argument must be an object from a different compartment") MSG_DEF(JSMSG_DEBUG_COMPARTMENT_MISMATCH, 2, JSEXN_TYPEERR, "{0}: descriptor .{1} property is an object in a different compartment than the target object") MSG_DEF(JSMSG_DEBUG_LOOP, 0, JSEXN_TYPEERR, "cannot debug an object in same compartment as debugger or a compartment that is already debugging the debugger") diff --git a/js/src/vm/Debugger.cpp b/js/src/vm/Debugger.cpp index 3daee3288632..79c5752b802f 100644 --- a/js/src/vm/Debugger.cpp +++ b/js/src/vm/Debugger.cpp @@ -3723,9 +3723,11 @@ Debugger::addDebuggee(JSContext* cx, unsigned argc, Value* vp) Debugger::addAllGlobalsAsDebuggees(JSContext* cx, unsigned argc, Value* vp) { THIS_DEBUGGER(cx, argc, vp, "addAllGlobalsAsDebuggees", args, dbg); - for (ZonesIter zone(cx->runtime(), SkipAtoms); !zone.done(); zone.next()) { - for (RealmsInZoneIter r(zone); !r.done(); r.next()) { - if (r == dbg->object->realm() || r->creationOptions().invisibleToDebugger()) + for (CompartmentsIter comp(cx->runtime()); !comp.done(); comp.next()) { + if (comp == dbg->object->compartment()) + continue; + for (RealmsInCompartmentIter r(comp); !r.done(); r.next()) { + if (r->creationOptions().invisibleToDebugger()) continue; r->compartment()->gcState.scheduledForDestruction = false; GlobalObject* global = r->maybeGlobal(); @@ -3921,7 +3923,7 @@ Debugger::construct(JSContext* cx, unsigned argc, Value* vp) // Add the initial debuggees, if any. for (unsigned i = 0; i < args.length(); i++) { JSObject& wrappedObj = args[i].toObject().as().private_().toObject(); - Rooted debuggee(cx, &wrappedObj.deprecatedGlobal()); + Rooted debuggee(cx, &wrappedObj.nonCCWGlobal()); if (!debugger->addDebuggeeGlobal(cx, debuggee)) return false; } @@ -3946,6 +3948,12 @@ Debugger::addDebuggeeGlobal(JSContext* cx, Handle global) return false; } + // Debugger and debuggee must be in different compartments. + if (debuggeeRealm->compartment() == object->compartment()) { + JS_ReportErrorNumberASCII(cx, GetErrorMessage, nullptr, JSMSG_DEBUG_SAME_COMPARTMENT); + return false; + } + // Check for cycles. If global's realm is reachable from this Debugger // object's realm by following debuggee-to-debugger links, then adding // global would create a cycle. (Typically nobody is debugging the From 7b5c6eb8014b873e9f0c9bdf99f3e287c1c37bdd Mon Sep 17 00:00:00 2001 From: Andrew Osmond Date: Tue, 21 Aug 2018 08:29:09 -0400 Subject: [PATCH 24/26] Bug 1452513 - Avoid issuing transactions with WebRender when the namespace has changed. r=kats When the namespace changes (e.g. due to a tab move between windows), we may get stale transaction requests that we need to ignore. In WebRenderBridgeParent::RecvSetDisplayList, we would automatically send any unsent transaction data when exiting the method, but this did not take into account the staleness. This patch ensures we only flush the data if we actually want it. The transaction in question that was observed and causing crashes was UpdateImageBuffer. --- gfx/layers/wr/WebRenderBridgeParent.cpp | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/gfx/layers/wr/WebRenderBridgeParent.cpp b/gfx/layers/wr/WebRenderBridgeParent.cpp index d9b245f37298..8c498010d89a 100644 --- a/gfx/layers/wr/WebRenderBridgeParent.cpp +++ b/gfx/layers/wr/WebRenderBridgeParent.cpp @@ -773,8 +773,15 @@ WebRenderBridgeParent::RecvSetDisplayList(const gfx::IntSize& aSize, mAsyncImageManager->SetCompositionTime(TimeStamp::Now()); + // If id namespaces do not match, it means the command is obsolete, probably + // because the tab just moved to a new window. + // In that case do not send the commands to webrender. + bool validTransaction = aIdNamespace == mIdNamespace; wr::TransactionBuilder txn; - wr::AutoTransactionSender sender(mApi, &txn); + Maybe sender; + if (validTransaction) { + sender.emplace(mApi, &txn); + } ProcessWebRenderParentCommands(aCommands, txn); @@ -795,10 +802,7 @@ WebRenderBridgeParent::RecvSetDisplayList(const gfx::IntSize& aSize, wr::Vec dlData(std::move(dl)); - // If id namespaces do not match, it means the command is obsolete, probably - // because the tab just moved to a new window. - // In that case do not send the commands to webrender. - if (mIdNamespace == aIdNamespace) { + if (validTransaction) { if (IsRootWebRenderBridgeParent()) { LayoutDeviceIntSize widgetSize = mWidget->GetClientSize(); LayoutDeviceIntRect docRect(LayoutDeviceIntPoint(), widgetSize); @@ -817,7 +821,7 @@ WebRenderBridgeParent::RecvSetDisplayList(const gfx::IntSize& aSize, HoldPendingTransactionId(wrEpoch, aTransactionId, aRefreshStartTime, aTxnStartTime, aFwdTime); - if (mIdNamespace != aIdNamespace) { + if (!validTransaction) { // Pretend we composited since someone is wating for this event, // though DisplayList was not pushed to webrender. if (CompositorBridgeParent* cbp = GetRootCompositorBridgeParent()) { From d2cd03866dc333484a63dbddd0b7fd694e4cec5b Mon Sep 17 00:00:00 2001 From: Tom Tung Date: Mon, 20 Aug 2018 11:46:24 +0200 Subject: [PATCH 25/26] Bug 1484615 - Followup comment fix for bug 1409641; r=janv This patch add comments to IndexedDBHelper.jsm to clarify how txn.result is set. The comments should clear up any confusion caused by previous commit message (fix for bug 1409641). --HG-- extra : rebase_source : 57a6f3eb569224a9e432803ef67d80f57219f7ab --- dom/base/IndexedDBHelper.jsm | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/dom/base/IndexedDBHelper.jsm b/dom/base/IndexedDBHelper.jsm index ab7626795ab7..6eddc6997688 100644 --- a/dom/base/IndexedDBHelper.jsm +++ b/dom/base/IndexedDBHelper.jsm @@ -134,7 +134,7 @@ IndexedDBHelper.prototype = { * be invoked with the transaction and the `store' object store. * @param successCb * Success callback to call on a successful transaction commit. - * The result is stored in txn.result. + * The result is stored in txn.result (in the callback function). * @param failureCb * Error callback to call when an error is encountered. */ @@ -162,6 +162,14 @@ IndexedDBHelper.prototype = { txn.oncomplete = function () { if (DEBUG) debug("Transaction complete. Returning to callback."); + /* + * txn.result property is not part of the transaction object returned + * by this._db.transaction method called above. + * The property is expected to be set in the callback function. + * However, it can happen that the property is not set for some reason, + * so we have to check if the property exists before calling the + * success callback. + */ if (successCb) { if ("result" in txn) { successCb(txn.result); @@ -173,6 +181,12 @@ IndexedDBHelper.prototype = { txn.onabort = function () { if (DEBUG) debug("Caught error on transaction"); + /* + * txn.error property is part of the transaction object returned by + * this._db.transaction method called above. + * The attribute is defined in IDBTranscation WebIDL interface. + * It may be null. + */ if (failureCb) { failureCb(getErrorName(txn.error)); } From 2f53d4f28181095c7c7db5731ee7b287b82d7d2c Mon Sep 17 00:00:00 2001 From: Mats Palmgren Date: Tue, 21 Aug 2018 14:55:22 +0200 Subject: [PATCH 26/26] Bug 1484559 - Ensure that the scroll frame deregister its refresh driver observers (mAsyncScroll & mAsyncSmoothMSDScroll) before it's destroyed. r=dholbert --- layout/generic/nsGfxScrollFrame.cpp | 51 ++++++++++++++++------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp index b79f5db8b2f4..3472cd0d4960 100644 --- a/layout/generic/nsGfxScrollFrame.cpp +++ b/layout/generic/nsGfxScrollFrame.cpp @@ -1853,6 +1853,18 @@ public: return true; } + /** + * The mCallee holds a strong ref to us since the refresh driver doesn't. + * Our dtor and mCallee's Destroy() method both call RemoveObserver() - + * whichever comes first removes us from the refresh driver. + */ + void RemoveObserver() { + if (mCallee) { + RefreshDriver(mCallee)->RemoveRefreshObserver(this, FlushType::Style); + mCallee = nullptr; + } + } + private: // Private destructor, to discourage deletion outside of Release(): ~AsyncSmoothMSDScroll() { @@ -1865,17 +1877,6 @@ private: return aCallee->mOuter->PresContext()->RefreshDriver(); } - /* - * The refresh driver doesn't hold a reference to its observers, - * so releasing this object can (and is) used to remove the observer on DTOR. - * Currently, this object is released once the scrolling ends. - */ - void RemoveObserver() { - if (mCallee) { - RefreshDriver(mCallee)->RemoveRefreshObserver(this, FlushType::Style); - } - } - mozilla::layers::AxisPhysicsMSDModel mXAxisModel, mYAxisModel; nsRect mRange; mozilla::TimeStamp mLastRefreshTime; @@ -1974,17 +1975,10 @@ public: ScrollFrameHelper::AsyncScrollCallback(mCallee, aTime); } -private: - ScrollFrameHelper *mCallee; - - nsRefreshDriver* RefreshDriver(ScrollFrameHelper* aCallee) { - return aCallee->mOuter->PresContext()->RefreshDriver(); - } - - /* - * The refresh driver doesn't hold a reference to its observers, - * so releasing this object can (and is) used to remove the observer on DTOR. - * Currently, this object is released once the scrolling ends. + /** + * The mCallee holds a strong ref to us since the refresh driver doesn't. + * Our dtor and mCallee's Destroy() method both call RemoveObserver() - + * whichever comes first removes us from the refresh driver. */ void RemoveObserver() { if (mCallee) { @@ -1992,8 +1986,15 @@ private: if (nsIPresShell* shell = mCallee->mOuter->PresShell()) { shell->SuppressDisplayport(false); } + mCallee = nullptr; } } +private: + ScrollFrameHelper *mCallee; + + nsRefreshDriver* RefreshDriver(ScrollFrameHelper* aCallee) { + return aCallee->mOuter->PresContext()->RefreshDriver(); + } }; /* @@ -4891,6 +4892,12 @@ ScrollFrameHelper::Destroy(PostDestroyData& aPostDestroyData) mScrollActivityTimer->Cancel(); mScrollActivityTimer = nullptr; } + if (mAsyncScroll) { + mAsyncScroll->RemoveObserver(); + } + if (mAsyncSmoothMSDScroll) { + mAsyncSmoothMSDScroll->RemoveObserver(); + } } /**