From 2dd3b9d1b30db1155d1bb93099f3301d91ab3f3b Mon Sep 17 00:00:00 2001 From: "Dragana Damjanovic dd.mozilla@gmail.com" Date: Fri, 19 May 2017 13:16:08 +0200 Subject: [PATCH 01/20] Bug 1352273 - Restart the TFO failure counter if a captive portal is detected.r=mcmanus --- netwerk/protocol/http/nsHttpHandler.cpp | 11 ++++++++++- netwerk/protocol/http/nsHttpHandler.h | 6 +++++- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/netwerk/protocol/http/nsHttpHandler.cpp b/netwerk/protocol/http/nsHttpHandler.cpp index 144008d0360c..b65abbc68396 100644 --- a/netwerk/protocol/http/nsHttpHandler.cpp +++ b/netwerk/protocol/http/nsHttpHandler.cpp @@ -481,6 +481,11 @@ nsHttpHandler::Init() "net:current-toplevel-outer-content-windowid", true); + if (mFastOpenSupported) { + obsService->AddObserver(this, "captive-portal-login", true); + obsService->AddObserver(this, "captive-portal-login-success", true); + } + // disabled as its a nop right now // obsService->AddObserver(this, "net:failed-to-process-uri-content", true); } @@ -648,7 +653,6 @@ nsHttpHandler::IncrementFastOpenConsecutiveFailureCounter() if (mFastOpenConsecutiveFailureCounter == mFastOpenConsecutiveFailureLimit) { LOG(("nsHttpHandler::IncrementFastOpenConsecutiveFailureCounter - " "Fast open failed too many times")); - SetFastOpenNotSupported(); } } } @@ -2315,6 +2319,11 @@ nsHttpHandler::Observe(nsISupports *subject, } } } + } else if (!strcmp(topic, "captive-portal-login") || + !strcmp(topic, "captive-portal-login-success")) { + // We have detected a captive portal and we will reset the Fast Open + // failure counter. + ResetFastOpenConsecutiveFailureCounter(); } return NS_OK; diff --git a/netwerk/protocol/http/nsHttpHandler.h b/netwerk/protocol/http/nsHttpHandler.h index ab6a6d7b4b14..abdd310adba7 100644 --- a/netwerk/protocol/http/nsHttpHandler.h +++ b/netwerk/protocol/http/nsHttpHandler.h @@ -164,7 +164,11 @@ public: return mTCPKeepaliveLongLivedIdleTimeS; } - bool UseFastOpen() { return mUseFastOpen && mFastOpenSupported; } + bool UseFastOpen() + { + return mUseFastOpen && mFastOpenSupported && + mFastOpenConsecutiveFailureCounter < mFastOpenConsecutiveFailureLimit; + } // If one of tcp connections return PR_NOT_TCP_SOCKET_ERROR while trying // fast open, it means that Fast Open is turned off so we will not try again // until a restart. This is only on Linux. From 0d165be73b0ecaab0170bd979080e1ccb7abc5a7 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Fri, 19 May 2017 14:00:31 +0200 Subject: [PATCH 02/20] Bug 1366011 - IPCBlob should not have race conditions between Send__delete__ and RecvStreamNeeded, r=smaug --- dom/file/ipc/IPCBlobInputStream.cpp | 4 ++ dom/file/ipc/IPCBlobInputStreamChild.cpp | 47 ++++++++++++++++++----- dom/file/ipc/IPCBlobInputStreamChild.h | 3 ++ dom/file/ipc/IPCBlobInputStreamParent.cpp | 9 +++++ dom/file/ipc/IPCBlobInputStreamParent.h | 3 ++ dom/file/ipc/PIPCBlobInputStream.ipdl | 4 +- 6 files changed, 58 insertions(+), 12 deletions(-) diff --git a/dom/file/ipc/IPCBlobInputStream.cpp b/dom/file/ipc/IPCBlobInputStream.cpp index 71ac79cf09c7..ba0dfdeb74dd 100644 --- a/dom/file/ipc/IPCBlobInputStream.cpp +++ b/dom/file/ipc/IPCBlobInputStream.cpp @@ -192,6 +192,10 @@ IPCBlobInputStream::Clone(nsIInputStream** aResult) MOZ_ASSERT(mActor); nsCOMPtr stream = mActor->CreateStream(); + if (!stream) { + return NS_ERROR_FAILURE; + } + stream.forget(aResult); return NS_OK; } diff --git a/dom/file/ipc/IPCBlobInputStreamChild.cpp b/dom/file/ipc/IPCBlobInputStreamChild.cpp index fda381365bb3..6ad1948bade3 100644 --- a/dom/file/ipc/IPCBlobInputStreamChild.cpp +++ b/dom/file/ipc/IPCBlobInputStreamChild.cpp @@ -13,19 +13,17 @@ namespace { // This runnable is used in case the last stream is forgotten on the 'wrong' // thread. -class DeleteRunnable final : public Runnable +class ShutdownRunnable final : public Runnable { public: - explicit DeleteRunnable(IPCBlobInputStreamChild* aActor) + explicit ShutdownRunnable(IPCBlobInputStreamChild* aActor) : mActor(aActor) {} NS_IMETHOD Run() override { - if (mActor->IsAlive()) { - mActor->Send__delete__(mActor); - } + mActor->Shutdown(); return NS_OK; } @@ -96,10 +94,29 @@ IPCBlobInputStreamChild::~IPCBlobInputStreamChild() {} void -IPCBlobInputStreamChild::ActorDestroy(IProtocol::ActorDestroyReason aReason) +IPCBlobInputStreamChild::Shutdown() { MutexAutoLock lock(mMutex); - mActorAlive = false; + + RefPtr kungFuDeathGrip = this; + + mPendingOperations.Clear(); + + if (mActorAlive) { + SendClose(); + mActorAlive = false; + } +} + +void +IPCBlobInputStreamChild::ActorDestroy(IProtocol::ActorDestroyReason aReason) +{ + { + MutexAutoLock lock(mMutex); + mActorAlive = false; + } + + Shutdown(); } bool @@ -114,6 +131,10 @@ IPCBlobInputStreamChild::CreateStream() { MutexAutoLock lock(mMutex); + if (!mActorAlive) { + return nullptr; + } + RefPtr stream = new IPCBlobInputStream(this); mStreams.AppendElement(stream); return stream.forget(); @@ -124,7 +145,7 @@ IPCBlobInputStreamChild::ForgetStream(IPCBlobInputStream* aStream) { MOZ_ASSERT(aStream); - RefPtr kungFoDeathGrip = this; + RefPtr kungFuDeathGrip = this; { MutexAutoLock lock(mMutex); @@ -136,11 +157,11 @@ IPCBlobInputStreamChild::ForgetStream(IPCBlobInputStream* aStream) } if (mOwningThread == NS_GetCurrentThread()) { - Send__delete__(this); + Shutdown(); return; } - RefPtr runnable = new DeleteRunnable(this); + RefPtr runnable = new ShutdownRunnable(this); mOwningThread->Dispatch(runnable, NS_DISPATCH_NORMAL); } @@ -149,6 +170,11 @@ IPCBlobInputStreamChild::StreamNeeded(IPCBlobInputStream* aStream, nsIEventTarget* aEventTarget) { MutexAutoLock lock(mMutex); + + if (!mActorAlive) { + return; + } + MOZ_ASSERT(mStreams.Contains(aStream)); PendingOperation* opt = mPendingOperations.AppendElement(); @@ -175,6 +201,7 @@ IPCBlobInputStreamChild::RecvStreamReady(const OptionalIPCStream& aStream) { MutexAutoLock lock(mMutex); MOZ_ASSERT(!mPendingOperations.IsEmpty()); + MOZ_ASSERT(mActorAlive); pendingStream = mPendingOperations[0].mStream; eventTarget = mPendingOperations[0].mEventTarget; diff --git a/dom/file/ipc/IPCBlobInputStreamChild.h b/dom/file/ipc/IPCBlobInputStreamChild.h index 56fc77a28eff..403654bfd6be 100644 --- a/dom/file/ipc/IPCBlobInputStreamChild.h +++ b/dom/file/ipc/IPCBlobInputStreamChild.h @@ -56,6 +56,9 @@ public: mozilla::ipc::IPCResult RecvStreamReady(const OptionalIPCStream& aStream) override; + void + Shutdown(); + private: ~IPCBlobInputStreamChild(); diff --git a/dom/file/ipc/IPCBlobInputStreamParent.cpp b/dom/file/ipc/IPCBlobInputStreamParent.cpp index 5415d0259b1e..35724be3456c 100644 --- a/dom/file/ipc/IPCBlobInputStreamParent.cpp +++ b/dom/file/ipc/IPCBlobInputStreamParent.cpp @@ -96,5 +96,14 @@ IPCBlobInputStreamParent::RecvStreamNeeded() return IPC_OK(); } +mozilla::ipc::IPCResult +IPCBlobInputStreamParent::RecvClose() +{ + MOZ_ASSERT(mContentManager || mPBackgroundManager); + + Unused << Send__delete__(this); + return IPC_OK(); +} + } // namespace dom } // namespace mozilla diff --git a/dom/file/ipc/IPCBlobInputStreamParent.h b/dom/file/ipc/IPCBlobInputStreamParent.h index f1cd9dbc75a6..3ab06aed7013 100644 --- a/dom/file/ipc/IPCBlobInputStreamParent.h +++ b/dom/file/ipc/IPCBlobInputStreamParent.h @@ -44,6 +44,9 @@ public: mozilla::ipc::IPCResult RecvStreamNeeded() override; + mozilla::ipc::IPCResult + RecvClose() override; + private: IPCBlobInputStreamParent(const nsID& aID, uint64_t aSize, nsIContentParent* aManager); diff --git a/dom/file/ipc/PIPCBlobInputStream.ipdl b/dom/file/ipc/PIPCBlobInputStream.ipdl index 7fa48f2f6e81..35ec2345771a 100644 --- a/dom/file/ipc/PIPCBlobInputStream.ipdl +++ b/dom/file/ipc/PIPCBlobInputStream.ipdl @@ -20,11 +20,11 @@ protocol PIPCBlobInputStream parent: async StreamNeeded(); - - async __delete__(); + async Close(); child: async StreamReady(OptionalIPCStream aStream); + async __delete__(); }; } // namespace dom From 6a4a1b53d07ef227b77986dfea7e7974f769e465 Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 19 May 2017 09:24:29 -0400 Subject: [PATCH 03/20] Bug 1361125 part 1. Add a way to ask a TemporaryTypeSet whether it might contain proxies. r=jandem --- js/src/vm/TypeInference.cpp | 21 +++++++++++++++++++++ js/src/vm/TypeInference.h | 3 +++ 2 files changed, 24 insertions(+) diff --git a/js/src/vm/TypeInference.cpp b/js/src/vm/TypeInference.cpp index 1c2c5a1bd0f2..e3531e655c95 100644 --- a/js/src/vm/TypeInference.cpp +++ b/js/src/vm/TypeInference.cpp @@ -2409,6 +2409,27 @@ TemporaryTypeSet::maybeCallable(CompilerConstraintList* constraints) return false; } +bool +TemporaryTypeSet::maybeProxy(CompilerConstraintList* constraints) +{ + if (!maybeObject()) + return false; + + if (unknownObject()) + return true; + + unsigned count = getObjectCount(); + for (unsigned i = 0; i < count; i++) { + const Class* clasp = getObjectClass(i); + if (!clasp) + continue; + if (clasp->isProxy() || !getObject(i)->hasStableClassAndProto(constraints)) + return true; + } + + return false; +} + bool TemporaryTypeSet::maybeEmulatesUndefined(CompilerConstraintList* constraints) { diff --git a/js/src/vm/TypeInference.h b/js/src/vm/TypeInference.h index 822fe9129379..628d02e20968 100644 --- a/js/src/vm/TypeInference.h +++ b/js/src/vm/TypeInference.h @@ -829,6 +829,9 @@ class TemporaryTypeSet : public TypeSet /* Whether clasp->isCallable() is true for one or more objects in this set. */ bool maybeCallable(CompilerConstraintList* constraints); + /* Whether clasp->isProxy() might be true for one or more objects in this set. */ + bool maybeProxy(CompilerConstraintList* constraints); + /* Whether clasp->emulatesUndefined() is true for one or more objects in this set. */ bool maybeEmulatesUndefined(CompilerConstraintList* constraints); From a4fdbd6fade1c7f9942c86491aa5dce183d6d57c Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 19 May 2017 09:24:30 -0400 Subject: [PATCH 04/20] Bug 1361125 part 2. Disable Ion fast paths for DOM getters on proxies when the jitinfo indicates the value can live in a slot. r=jandem We do this for now because the Ion fast paths assume things about whether slots are fixed or not, and how reserved slot indices map to fixed slot indices, that are not true for proxies, because they have an extra reserved slot. --- js/src/jit/CodeGenerator.cpp | 15 ++++++++ js/src/jit/IonBuilder.cpp | 67 ++++++++++++++++++++---------------- 2 files changed, 53 insertions(+), 29 deletions(-) diff --git a/js/src/jit/CodeGenerator.cpp b/js/src/jit/CodeGenerator.cpp index 18145e737aea..d1241b8de82b 100644 --- a/js/src/jit/CodeGenerator.cpp +++ b/js/src/jit/CodeGenerator.cpp @@ -11437,6 +11437,11 @@ CodeGenerator::visitGetDOMProperty(LGetDOMProperty* ins) // It's a bit annoying to redo these slot calculations, which duplcate // LSlots and a few other things like that, but I'm not sure there's a // way to reuse those here. + // + // If this ever gets fixed to work with proxies (by not assuming that + // reserved slot indices, which is what domMemberSlotIndex() returns, + // match fixed slot indices), we can reenable MGetDOMProperty for + // proxies in IonBuilder. if (slot < NativeObject::MAX_FIXED_SLOTS) { masm.loadValue(Address(ObjectReg, NativeObject::getFixedSlotOffset(slot)), JSReturnOperand); @@ -11509,6 +11514,11 @@ CodeGenerator::visitGetDOMMemberV(LGetDOMMemberV* ins) // require us to have MGetDOMMember inherit from MLoadFixedSlot, and then // we'd have to duplicate a bunch of stuff we now get for free from // MGetDOMProperty. + // + // If this ever gets fixed to work with proxies (by not assuming that + // reserved slot indices, which is what domMemberSlotIndex() returns, + // match fixed slot indices), we can reenable MGetDOMMember for + // proxies in IonBuilder. Register object = ToRegister(ins->object()); size_t slot = ins->mir()->domMemberSlotIndex(); ValueOperand result = GetValueOutput(ins); @@ -11524,6 +11534,11 @@ CodeGenerator::visitGetDOMMemberT(LGetDOMMemberT* ins) // require us to have MGetDOMMember inherit from MLoadFixedSlot, and then // we'd have to duplicate a bunch of stuff we now get for free from // MGetDOMProperty. + // + // If this ever gets fixed to work with proxies (by not assuming that + // reserved slot indices, which is what domMemberSlotIndex() returns, + // match fixed slot indices), we can reenable MGetDOMMember for + // proxies in IonBuilder. Register object = ToRegister(ins->object()); size_t slot = ins->mir()->domMemberSlotIndex(); AnyRegister result = ToAnyRegister(ins->getDef(0)); diff --git a/js/src/jit/IonBuilder.cpp b/js/src/jit/IonBuilder.cpp index 01381be56d34..e5ee27cee38a 100644 --- a/js/src/jit/IonBuilder.cpp +++ b/js/src/jit/IonBuilder.cpp @@ -10881,38 +10881,47 @@ IonBuilder::getPropTryCommonGetter(bool* emitted, MDefinition* obj, PropertyName if (isDOM) { const JSJitInfo* jitinfo = commonGetter->jitInfo(); - MInstruction* get; - if (jitinfo->isAlwaysInSlot) { - // If our object is a singleton and we know the property is - // constant (which is true if and only if the get doesn't alias - // anything), we can just read the slot here and use that constant. - JSObject* singleton = objTypes->maybeSingleton(); - if (singleton && jitinfo->aliasSet() == JSJitInfo::AliasNone) { - size_t slot = jitinfo->slotIndex; - *emitted = true; - pushConstant(GetReservedSlot(singleton, slot)); - return Ok(); + // We don't support MGetDOMProperty/MGetDOMMember on things that might + // be proxies when the value might be in a slot, because the + // CodeGenerator code for LGetDOMProperty/LGetDOMMember doesn't handle + // that case correctly. + if ((!jitinfo->isAlwaysInSlot && !jitinfo->isLazilyCachedInSlot) || + !objTypes->maybeProxy(constraints())) { + MInstruction* get; + if (jitinfo->isAlwaysInSlot) { + // If our object is a singleton and we know the property is + // constant (which is true if and only if the get doesn't alias + // anything), we can just read the slot here and use that + // constant. + JSObject* singleton = objTypes->maybeSingleton(); + if (singleton && jitinfo->aliasSet() == JSJitInfo::AliasNone) { + size_t slot = jitinfo->slotIndex; + *emitted = true; + pushConstant(GetReservedSlot(singleton, slot)); + return Ok(); + } + + // We can't use MLoadFixedSlot here because it might not have + // the right aliasing behavior; we want to alias DOM setters as + // needed. + get = MGetDOMMember::New(alloc(), jitinfo, obj, guard, globalGuard); + } else { + get = MGetDOMProperty::New(alloc(), jitinfo, obj, guard, globalGuard); } + if (!get) + return abort(AbortReason::Alloc); + current->add(get); + current->push(get); - // We can't use MLoadFixedSlot here because it might not have the - // right aliasing behavior; we want to alias DOM setters as needed. - get = MGetDOMMember::New(alloc(), jitinfo, obj, guard, globalGuard); - } else { - get = MGetDOMProperty::New(alloc(), jitinfo, obj, guard, globalGuard); + if (get->isEffectful()) + MOZ_TRY(resumeAfter(get)); + + MOZ_TRY(pushDOMTypeBarrier(get, types, commonGetter)); + + trackOptimizationOutcome(TrackedOutcome::DOM); + *emitted = true; + return Ok(); } - if (!get) - return abort(AbortReason::Alloc); - current->add(get); - current->push(get); - - if (get->isEffectful()) - MOZ_TRY(resumeAfter(get)); - - MOZ_TRY(pushDOMTypeBarrier(get, types, commonGetter)); - - trackOptimizationOutcome(TrackedOutcome::DOM); - *emitted = true; - return Ok(); } // Don't call the getter with a primitive value. From 682fe478ec1b3a234309968b2a04287d9f98859f Mon Sep 17 00:00:00 2001 From: Boris Zbarsky Date: Fri, 19 May 2017 09:24:30 -0400 Subject: [PATCH 05/20] Bug 1237503. Add support for [Cached] and [StoreInSlot] things on DOM proxies. r=qdot The only difference between proxies and non-proxies is that proxies only support up to MAX_FIXED_SLOTS slots all told (reserved plus private slot). SpiderMonkey already has static asserts to make sure we don't ask for too many reserved slots on a proxy. --- dom/bindings/Codegen.py | 18 ++++++++++-------- dom/bindings/test/TestBindingHeader.h | 2 ++ dom/bindings/test/TestCodeGen.webidl | 2 ++ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/dom/bindings/Codegen.py b/dom/bindings/Codegen.py index 6409eb9405e9..36f3156e6ead 100644 --- a/dom/bindings/Codegen.py +++ b/dom/bindings/Codegen.py @@ -424,6 +424,10 @@ def DOMClass(descriptor): hooks=NativePropertyHooks(descriptor)) +def InstanceReservedSlots(descriptor): + return INSTANCE_RESERVED_SLOTS + descriptor.interface.totalMembersInSlots + + class CGDOMJSClass(CGThing): """ Generate a DOMJSClass for a given descriptor @@ -438,7 +442,7 @@ class CGDOMJSClass(CGThing): def define(self): callHook = LEGACYCALLER_HOOK_NAME if self.descriptor.operations["LegacyCaller"] else 'nullptr' objectMovedHook = OBJECT_MOVED_HOOK_NAME if self.descriptor.wrapperCache else 'nullptr' - slotCount = INSTANCE_RESERVED_SLOTS + self.descriptor.interface.totalMembersInSlots + slotCount = InstanceReservedSlots(self.descriptor) classFlags = "JSCLASS_IS_DOMJSCLASS | JSCLASS_FOREGROUND_FINALIZE | " if self.descriptor.isGlobal(): classFlags += "JSCLASS_DOM_GLOBAL | JSCLASS_GLOBAL_FLAGS_WITH_SLOTS(DOM_GLOBAL_SLOTS)" @@ -528,9 +532,10 @@ class CGDOMProxyJSClass(CGThing): return "" def define(self): + slotCount = InstanceReservedSlots(self.descriptor) # We need one reserved slot (DOM_OBJECT_SLOT). flags = ["JSCLASS_IS_DOMJSCLASS", - "JSCLASS_HAS_RESERVED_SLOTS(1)"] + "JSCLASS_HAS_RESERVED_SLOTS(%d)" % slotCount] # We don't use an IDL annotation for JSCLASS_EMULATES_UNDEFINED because # we don't want people ever adding that to any interface other than # HTMLAllCollection. So just hardcode it here. @@ -12717,10 +12722,6 @@ class CGDescriptor(CGThing): if descriptor.concrete: if descriptor.proxy: - if descriptor.interface.totalMembersInSlots != 0: - raise TypeError("We can't have extra reserved slots for " - "proxy interface %s" % - descriptor.interface.identifier.name) cgThings.append(CGGeneric(fill( """ static_assert(IsBaseOf::value, @@ -12741,8 +12742,9 @@ class CGDescriptor(CGThing): else: cgThings.append(CGDOMJSClass(descriptor)) cgThings.append(CGGetJSClassMethod(descriptor)) - if descriptor.interface.hasMembersInSlots(): - cgThings.append(CGUpdateMemberSlotsMethod(descriptor)) + + if descriptor.interface.hasMembersInSlots(): + cgThings.append(CGUpdateMemberSlotsMethod(descriptor)) if descriptor.isGlobal(): assert descriptor.wrapperCache diff --git a/dom/bindings/test/TestBindingHeader.h b/dom/bindings/test/TestBindingHeader.h index 9b23ecb928fe..af83c98a1fb0 100644 --- a/dom/bindings/test/TestBindingHeader.h +++ b/dom/bindings/test/TestBindingHeader.h @@ -1217,6 +1217,8 @@ public: uint32_t Item(uint32_t, bool&) = delete; uint32_t Length(); void LegacyCall(JS::Handle); + int32_t CachedAttr(); + int32_t StoreInSlotAttr(); }; class TestNamedGetterInterface : public nsISupports, diff --git a/dom/bindings/test/TestCodeGen.webidl b/dom/bindings/test/TestCodeGen.webidl index 4259ca75641e..e971e5226b32 100644 --- a/dom/bindings/test/TestCodeGen.webidl +++ b/dom/bindings/test/TestCodeGen.webidl @@ -1188,6 +1188,8 @@ interface TestIndexedGetterInterface { getter long item(unsigned long idx); readonly attribute unsigned long length; legacycaller void(); + [Cached, Pure] readonly attribute long cachedAttr; + [StoreInSlot, Pure] readonly attribute long storeInSlotAttr; }; interface TestNamedGetterInterface { From f5567643a35606a6c3ade1aa28fc803c4f8db36c Mon Sep 17 00:00:00 2001 From: Ehsan Akhgari Date: Fri, 19 May 2017 00:58:18 -0400 Subject: [PATCH 06/20] Bug 1366156 - Temporarily disable the collection of content js delay event telemetry probes to investigate whether they are the cause of child process INPUT_EVENT_RESPONSE_MS regressions; r=farre --- ipc/glue/BackgroundChildImpl.cpp | 3 +++ xpcom/threads/SchedulerGroup.cpp | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/ipc/glue/BackgroundChildImpl.cpp b/ipc/glue/BackgroundChildImpl.cpp index 42517dd33126..35fbdca051cb 100644 --- a/ipc/glue/BackgroundChildImpl.cpp +++ b/ipc/glue/BackgroundChildImpl.cpp @@ -569,11 +569,14 @@ BackgroundChildImpl::DeallocPGamepadTestChannelChild(PGamepadTestChannelChild* a void BackgroundChildImpl::OnChannelReceivedMessage(const Message& aMsg) { +// Telemetry collection temporarily disabled in bug 1366156. +#if 0 if (aMsg.type() == layout::PVsync::MessageType::Msg_Notify__ID) { // Not really necessary to look at the message payload, it will be // <0.5ms away from TimeStamp::Now() SchedulerGroup::MarkVsyncReceived(); } +#endif } #endif diff --git a/xpcom/threads/SchedulerGroup.cpp b/xpcom/threads/SchedulerGroup.cpp index efade2cb1a98..74342cadbbda 100644 --- a/xpcom/threads/SchedulerGroup.cpp +++ b/xpcom/threads/SchedulerGroup.cpp @@ -61,17 +61,23 @@ public: : mIsBackground(aRunnable->IsBackground()) { MOZ_GUARD_OBJECT_NOTIFIER_INIT; +// Telemetry collection temporarily disabled in bug 1366156. +#if 0 #ifdef EARLY_BETA_OR_EARLIER aRunnable->GetName(mKey); mStart = TimeStamp::Now(); +#endif #endif } ~AutoCollectVsyncTelemetry() { +// Telemetry collection temporarily disabled in bug 1366156. +#if 0 #ifdef EARLY_BETA_OR_EARLIER if (Telemetry::CanRecordBase()) { CollectTelemetry(); } +#endif #endif } From 1494e1d3e810bfe06117a91c6168fb791befa789 Mon Sep 17 00:00:00 2001 From: Andreas Farre Date: Mon, 15 May 2017 13:06:28 +0200 Subject: [PATCH 07/20] Bug 1364858 - Make sure throttle timer isn't recreated. r=smaug MozReview-Commit-ID: DU8ukd9JqI6 --- dom/base/TimeoutManager.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dom/base/TimeoutManager.cpp b/dom/base/TimeoutManager.cpp index ae50d344d3c5..0586a876e7a8 100644 --- a/dom/base/TimeoutManager.cpp +++ b/dom/base/TimeoutManager.cpp @@ -256,6 +256,7 @@ TimeoutManager::TimeoutManager(nsGlobalWindow& aWindow) TimeoutManager::~TimeoutManager() { + MOZ_DIAGNOSTIC_ASSERT(mWindow.AsInner()->InnerObjectsFreed()); MOZ_DIAGNOSTIC_ASSERT(!mThrottleTrackingTimeoutsTimer); MOZ_LOG(gLog, LogLevel::Debug, @@ -1472,7 +1473,8 @@ TimeoutManager::OnDocumentLoaded() void TimeoutManager::MaybeStartThrottleTrackingTimout() { - if (gTrackingTimeoutThrottlingDelay <= 0) { + if (gTrackingTimeoutThrottlingDelay <= 0 || + mWindow.AsInner()->InnerObjectsFreed()) { return; } From efae3978c1033ffd012474c0c706790600f9bea2 Mon Sep 17 00:00:00 2001 From: Jessica Jong Date: Thu, 18 May 2017 02:44:00 -0400 Subject: [PATCH 08/20] Bug 1365395 - Resetting select should reset option dirtiness too. r=smaug Also added a extra flag NO_RESELECT, to avoid selecting the same option (with default value false) again after setting it to its default value, and later no way of deselecting it because it's dirtiness is set to false already. MozReview-Commit-ID: L35sBMWwi3m --- dom/html/HTMLOptionElement.h | 5 +++++ dom/html/HTMLSelectElement.cpp | 5 +++-- dom/html/HTMLSelectElement.h | 6 +++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/dom/html/HTMLOptionElement.h b/dom/html/HTMLOptionElement.h index 6e893762888d..f59e98ed3893 100644 --- a/dom/html/HTMLOptionElement.h +++ b/dom/html/HTMLOptionElement.h @@ -44,6 +44,11 @@ public: bool Selected() const; bool DefaultSelected() const; + void SetSelectedChanged(bool aValue) + { + mSelectedChanged = aValue; + } + virtual nsChangeHint GetAttributeChangeHint(const nsIAtom* aAttribute, int32_t aModType) const override; diff --git a/dom/html/HTMLSelectElement.cpp b/dom/html/HTMLSelectElement.cpp index 9cafae6cc4c2..d8695da874ec 100644 --- a/dom/html/HTMLSelectElement.cpp +++ b/dom/html/HTMLSelectElement.cpp @@ -1072,7 +1072,7 @@ HTMLSelectElement::SetOptionsSelectedByIndex(int32_t aStartIndex, } // Make sure something is selected unless we were set to -1 (none) - if (optionsDeselected && aStartIndex != -1) { + if (optionsDeselected && aStartIndex != -1 && !(aOptionsMask & NO_RESELECT)) { optionsSelected = CheckSelectSomething(aOptionsMask & NOTIFY) || optionsSelected; } @@ -1646,13 +1646,14 @@ HTMLSelectElement::Reset() // Reset the option to its default value // - uint32_t mask = SET_DISABLED | NOTIFY; + uint32_t mask = SET_DISABLED | NOTIFY | NO_RESELECT; if (option->DefaultSelected()) { mask |= IS_SELECTED; numSelected++; } SetOptionsSelectedByIndex(i, i, mask); + option->SetSelectedChanged(false); } } diff --git a/dom/html/HTMLSelectElement.h b/dom/html/HTMLSelectElement.h index 635f2b0f9301..fb8a3a791565 100644 --- a/dom/html/HTMLSelectElement.h +++ b/dom/html/HTMLSelectElement.h @@ -134,12 +134,16 @@ public: * (for JavaScript) * * NOTIFY whether to notify frames and such + * + * NO_RESELECT no need to select something after an option is deselected + * (for reset) */ enum OptionType { IS_SELECTED = 1 << 0, CLEAR_ALL = 1 << 1, SET_DISABLED = 1 << 2, - NOTIFY = 1 << 3 + NOTIFY = 1 << 3, + NO_RESELECT = 1 << 4 }; using nsIConstraintValidation::GetValidationMessage; From 7cf1d1c9b0748e03a8d2ec0b088a7eb0917b7bf2 Mon Sep 17 00:00:00 2001 From: Andrea Marchesini Date: Fri, 19 May 2017 17:26:42 +0200 Subject: [PATCH 09/20] Bug 1366101 - Remove extra warnings in SessionStorageManage, r=smaug --- dom/storage/SessionStorageManager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dom/storage/SessionStorageManager.cpp b/dom/storage/SessionStorageManager.cpp index da4e30cef504..e45b157ff258 100644 --- a/dom/storage/SessionStorageManager.cpp +++ b/dom/storage/SessionStorageManager.cpp @@ -89,7 +89,7 @@ SessionStorageManager::GetStorage(mozIDOMWindow* aWindow, nsAutoCString originKey; nsAutoCString originAttributes; nsresult rv = GenerateOriginKey(aPrincipal, originAttributes, originKey); - if (NS_WARN_IF(NS_FAILED(rv))) { + if (NS_FAILED(rv)) { return rv; } @@ -128,7 +128,7 @@ SessionStorageManager::CloneStorage(nsIDOMStorage* aStorage) nsAutoCString originAttributes; nsresult rv = GenerateOriginKey(storage->Principal(), originAttributes, originKey); - if (NS_WARN_IF(NS_FAILED(rv))) { + if (NS_FAILED(rv)) { return rv; } From 1ae80523784ee678ca3c5486bc5391ba83d89e9f Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Thu, 18 May 2017 14:16:41 -0700 Subject: [PATCH 10/20] Bug 1366085 - Make JSON output use hierarchical names, r=jonco --HG-- extra : rebase_source : 33a999e29bf9fc9710cb715ef7da12bfb2d81162 --- js/src/gc/GenerateStatsPhases.py | 68 +++++++++++++++++--------------- js/src/gc/Statistics.cpp | 33 +++------------- 2 files changed, 43 insertions(+), 58 deletions(-) diff --git a/js/src/gc/GenerateStatsPhases.py b/js/src/gc/GenerateStatsPhases.py index e31a3024648e..300b43bd469f 100644 --- a/js/src/gc/GenerateStatsPhases.py +++ b/js/src/gc/GenerateStatsPhases.py @@ -49,6 +49,7 @@ # | E | | E'| # +---+ +---+ +import re import sys import collections @@ -174,52 +175,56 @@ AllPhaseKinds = findAllPhaseKinds() # one parent. class Phase: - def __init__(self, phaseKind, parent, depth): + def __init__(self, phaseKind, parent): self.phaseKind = phaseKind self.parent = parent - self.depth = depth + self.depth = parent.depth + 1 if parent else 0 self.children = [] self.nextSibling = None self.nextInPhaseKind = None + self.path = re.sub(r'\W+', '_', phaseKind.name.lower()) + if parent is not None: + self.path = parent.path + '.' + self.path + def expandPhases(): phases = [] - phasesForPhase = collections.defaultdict(list) + phasesForKind = collections.defaultdict(list) - def traverse(phaseKind, parent, depth): - ep = Phase(phaseKind, parent, depth) + def traverse(phaseKind, parent): + ep = Phase(phaseKind, parent) phases.append(ep) # Update list of expanded phases for this phase kind. - if phasesForPhase[phaseKind]: - phasesForPhase[phaseKind][-1].nextInPhaseKind = ep - phasesForPhase[phaseKind].append(ep) + if phasesForKind[phaseKind]: + phasesForKind[phaseKind][-1].nextInPhaseKind = ep + phasesForKind[phaseKind].append(ep) # Recurse over children. for child in phaseKind.children: - child_ep = traverse(child, ep, depth + 1) + child_ep = traverse(child, ep) if ep.children: ep.children[-1].nextSibling = child_ep ep.children.append(child_ep) return ep for phaseKind in PhaseKindGraphRoots: - traverse(phaseKind, None, 0) + traverse(phaseKind, None) - return phases, phasesForPhase + return phases, phasesForKind AllPhases, PhasesForPhaseKind = expandPhases() -# Name expanded phases based on phase kind name and index if there are -# multiple expanded phases corresponding to a single phase kind. +# Name phases based on phase kind name and index if there are multiple phases +# corresponding to a single phase kind. for phaseKind in AllPhaseKinds: phases = PhasesForPhaseKind[phaseKind] if len(phases) == 1: phases[0].name = "%s" % phaseKind.name else: - for index, xphase in enumerate(phases): - xphase.name = "%s_%d" % (phaseKind.name, index + 1) + for index, phase in enumerate(phases): + phase.name = "%s_%d" % (phaseKind.name, index + 1) # Generate code. @@ -250,13 +255,13 @@ def generateHeader(out): # # Generate Phase enum. # - expandedPhaseNames = map(lambda xphase: xphase.name, AllPhases) + phaseNames = map(lambda phase: phase.name, AllPhases) extraPhases = [ "NONE = LIMIT", "EXPLICIT_SUSPENSION = LIMIT", "IMPLICIT_SUSPENSION" ] - writeEnumClass(out, "Phase", "uint8_t", expandedPhaseNames, extraPhases) + writeEnumClass(out, "Phase", "uint8_t", phaseNames, extraPhases) def generateCpp(out): # @@ -264,29 +269,30 @@ def generateCpp(out): # out.write("static const PhaseKindTable phaseKinds = {\n") for phaseKind in AllPhaseKinds: - xPhase = PhasesForPhaseKind[phaseKind][0] + phase = PhasesForPhaseKind[phaseKind][0] out.write(" /* PhaseKind::%s */ PhaseKindInfo { Phase::%s, %d },\n" % - (phaseKind.name, xPhase.name, phaseKind.bucket)) + (phaseKind.name, phase.name, phaseKind.bucket)) out.write("};\n") out.write("\n") # # Generate the PhaseInfo tree. # - def name(xphase): - return "Phase::" + xphase.name if xphase else "Phase::NONE" + def name(phase): + return "Phase::" + phase.name if phase else "Phase::NONE" out.write("static const PhaseTable phases = {\n") - for xphase in AllPhases: - firstChild = xphase.children[0] if xphase.children else None - phaseKind = xphase.phaseKind - out.write(" /* %s */ PhaseInfo { %s, %s, %s, %s, PhaseKind::%s, %d, \"%s\" },\n" % - (name(xphase), - name(xphase.parent), + for phase in AllPhases: + firstChild = phase.children[0] if phase.children else None + phaseKind = phase.phaseKind + out.write(" /* %s */ PhaseInfo { %s, %s, %s, %s, PhaseKind::%s, %d, \"%s\", \"%s\" },\n" % + (name(phase), + name(phase.parent), name(firstChild), - name(xphase.nextSibling), - name(xphase.nextInPhaseKind), + name(phase.nextSibling), + name(phase.nextInPhaseKind), phaseKind.name, - xphase.depth, - phaseKind.descr)) + phase.depth, + phaseKind.descr, + phase.path)) out.write("};\n") diff --git a/js/src/gc/Statistics.cpp b/js/src/gc/Statistics.cpp index 37c0d32aa837..da72e49b1d44 100644 --- a/js/src/gc/Statistics.cpp +++ b/js/src/gc/Statistics.cpp @@ -107,12 +107,13 @@ struct PhaseInfo PhaseKind phaseKind; uint8_t depth; const char* name; + const char* path; }; -// A table of ExpandePhaseInfo indexed by Phase. +// A table of PhaseInfo indexed by Phase. using PhaseTable = EnumeratedArray; -// A table of PhaseKindInfo indexed by Phase. +// A table of PhaseKindInfo indexed by PhaseKind. using PhaseKindTable = EnumeratedArray; #include "gc/StatsPhasesGenerated.cpp" @@ -132,8 +133,8 @@ Statistics::currentPhase() const PhaseKind Statistics::currentPhaseKind() const { - // Public API to get the current phase. Return the current phase, - // suppressing the synthetic PhaseKind::MUTATOR phase. + // Public API to get the current phase kind, suppressing the synthetic + // PhaseKind::MUTATOR phase. Phase phase = currentPhase(); MOZ_ASSERT_IF(phase == Phase::MUTATOR, phaseNestingDepth == 1); @@ -621,35 +622,13 @@ Statistics::formatJsonSliceDescription(unsigned i, const SliceData& slice, JSONP json.property("end_timestamp", slice.end - originTime, JSONPrinter::SECONDS); } -static UniqueChars -SanitizeJsonKey(const char* const buffer) -{ - char* mut = strdup(buffer); - if (!mut) - return UniqueChars(nullptr); - - char* c = mut; - while (*c) { - if (!isalpha(*c)) - *c = '_'; - else if (isupper(*c)) - *c = tolower(*c); - ++c; - } - - return UniqueChars(mut); -} - void Statistics::formatJsonPhaseTimes(const PhaseTimeTable& phaseTimes, JSONPrinter& json) const { for (auto phase : AllPhases()) { - UniqueChars name = SanitizeJsonKey(phases[phase].name); - if (!name) - json.outOfMemory(); TimeDuration ownTime = phaseTimes[phase]; if (!ownTime.IsZero()) - json.property(name.get(), ownTime, JSONPrinter::MILLISECONDS); + json.property(phases[phase].path, ownTime, JSONPrinter::MILLISECONDS); } } From 1514d9b219f52924e82a528889be250450616368 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Fri, 12 May 2017 17:20:48 -0700 Subject: [PATCH 11/20] Bug 1364287 - Move g{Out,Err}FilePtr into ShellContext for thread safety, r=jonco MozReview-Commit-ID: KNAAL7pRAMG --HG-- extra : rebase_source : b1febb564d18601769a19c6c73b16cacdeff4fc3 --- js/src/shell/OSObject.cpp | 14 +-- js/src/shell/js.cpp | 258 +++++++++++++------------------------- js/src/shell/jsshell.h | 126 +++++++++++++++++++ 3 files changed, 218 insertions(+), 180 deletions(-) diff --git a/js/src/shell/OSObject.cpp b/js/src/shell/OSObject.cpp index 9ce998f88981..dad3989532b7 100644 --- a/js/src/shell/OSObject.cpp +++ b/js/src/shell/OSObject.cpp @@ -45,9 +45,6 @@ using js::shell::RCFile; -static RCFile** gErrFilePtr = nullptr; -static RCFile** gOutFilePtr = nullptr; - namespace js { namespace shell { @@ -555,13 +552,15 @@ Redirect(JSContext* cx, const CallArgs& args, RCFile** outFile) static bool osfile_redirectOutput(JSContext* cx, unsigned argc, Value* vp) { CallArgs args = CallArgsFromVp(argc, vp); - return Redirect(cx, args, gOutFilePtr); + ShellContext* scx = GetShellContext(cx); + return Redirect(cx, args, scx->outFilePtr); } static bool osfile_redirectError(JSContext* cx, unsigned argc, Value* vp) { CallArgs args = CallArgsFromVp(argc, vp); - return Redirect(cx, args, gErrFilePtr); + ShellContext* scx = GetShellContext(cx); + return Redirect(cx, args, scx->errFilePtr); } static bool @@ -1010,8 +1009,9 @@ DefineOS(JSContext* cx, HandleObject global, if (!GenerateInterfaceHelp(cx, obj, "os")) return false; - gOutFilePtr = shellOut; - gErrFilePtr = shellErr; + ShellContext* scx = GetShellContext(cx); + scx->outFilePtr = shellOut; + scx->errFilePtr = shellErr; // For backwards compatibility, expose various os.file.* functions as // direct methods on the global. diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp index 590eec75a440..1c15eff8027f 100644 --- a/js/src/shell/js.cpp +++ b/js/src/shell/js.cpp @@ -113,6 +113,8 @@ using namespace js; using namespace js::cli; using namespace js::shell; +using js::shell::RCFile; + using mozilla::ArrayLength; using mozilla::Atomic; using mozilla::MakeScopeExit; @@ -152,189 +154,97 @@ static const TimeDuration MAX_TIMEOUT_INTERVAL = TimeDuration::FromSeconds(1800. // SharedArrayBuffer and Atomics are enabled by default (tracking Firefox). #define SHARED_MEMORY_DEFAULT 1 -// Some platform hooks must be implemented for single-step profiling. -#if defined(JS_SIMULATOR_ARM) || defined(JS_SIMULATOR_MIPS64) -# define SINGLESTEP_PROFILING -#endif - -enum class ScriptKind +bool +OffThreadState::startIfIdle(JSContext* cx, ScriptKind kind, ScopedJSFreePtr& newSource) { - Script, - DecodeScript, - Module -}; + AutoLockMonitor alm(monitor); + if (state != IDLE) + return false; -class OffThreadState { - enum State { - IDLE, /* ready to work; no token, no source */ - COMPILING, /* working; no token, have source */ - DONE /* compilation done: have token and source */ - }; + MOZ_ASSERT(!token); - public: - OffThreadState() - : monitor(mutexid::ShellOffThreadState), - state(IDLE), - token(), - source(nullptr) - { } + source = newSource.forget(); - bool startIfIdle(JSContext* cx, ScriptKind kind, - ScopedJSFreePtr& newSource) - { - AutoLockMonitor alm(monitor); - if (state != IDLE) - return false; + scriptKind = kind; + state = COMPILING; + return true; +} - MOZ_ASSERT(!token); - - source = newSource.forget(); - - scriptKind = kind; - state = COMPILING; - return true; - } - - bool startIfIdle(JSContext* cx, ScriptKind kind, - JS::TranscodeBuffer&& newXdr) - { - AutoLockMonitor alm(monitor); - if (state != IDLE) - return false; - - MOZ_ASSERT(!token); - - xdr = mozilla::Move(newXdr); - - scriptKind = kind; - state = COMPILING; - return true; - } - - void abandon(JSContext* cx) { - AutoLockMonitor alm(monitor); - MOZ_ASSERT(state == COMPILING); - MOZ_ASSERT(!token); - MOZ_ASSERT(source || !xdr.empty()); - - if (source) - js_free(source); - source = nullptr; - xdr.clearAndFree(); - - state = IDLE; - } - - void markDone(void* newToken) { - AutoLockMonitor alm(monitor); - MOZ_ASSERT(state == COMPILING); - MOZ_ASSERT(!token); - MOZ_ASSERT(source || !xdr.empty()); - MOZ_ASSERT(newToken); - - token = newToken; - state = DONE; - alm.notify(); - } - - void* waitUntilDone(JSContext* cx, ScriptKind kind) { - AutoLockMonitor alm(monitor); - if (state == IDLE || scriptKind != kind) - return nullptr; - - if (state == COMPILING) { - while (state != DONE) - alm.wait(); - } - - MOZ_ASSERT(source || !xdr.empty()); - if (source) - js_free(source); - source = nullptr; - xdr.clearAndFree(); - - MOZ_ASSERT(token); - void* holdToken = token; - token = nullptr; - state = IDLE; - return holdToken; - } - - JS::TranscodeBuffer& xdrBuffer() { return xdr; } - - private: - Monitor monitor; - ScriptKind scriptKind; - State state; - void* token; - char16_t* source; - JS::TranscodeBuffer xdr; -}; - -#ifdef SINGLESTEP_PROFILING -typedef Vector StackChars; -#endif - -class NonshrinkingGCObjectVector : public GCVector +bool +OffThreadState::startIfIdle(JSContext* cx, ScriptKind kind, JS::TranscodeBuffer&& newXdr) { - public: - void sweep() { - for (uint32_t i = 0; i < this->length(); i++) { - if (JS::GCPolicy::needsSweep(&(*this)[i])) - (*this)[i] = nullptr; - } - } -}; + AutoLockMonitor alm(monitor); + if (state != IDLE) + return false; -using MarkBitObservers = JS::WeakCache; + MOZ_ASSERT(!token); + + xdr = mozilla::Move(newXdr); + + scriptKind = kind; + state = COMPILING; + return true; +} + +void +OffThreadState::abandon(JSContext* cx) +{ + AutoLockMonitor alm(monitor); + MOZ_ASSERT(state == COMPILING); + MOZ_ASSERT(!token); + MOZ_ASSERT(source || !xdr.empty()); + + if (source) + js_free(source); + source = nullptr; + xdr.clearAndFree(); + + state = IDLE; +} + +void +OffThreadState::markDone(void* newToken) +{ + AutoLockMonitor alm(monitor); + MOZ_ASSERT(state == COMPILING); + MOZ_ASSERT(!token); + MOZ_ASSERT(source || !xdr.empty()); + MOZ_ASSERT(newToken); + + token = newToken; + state = DONE; + alm.notify(); +} + +void* +OffThreadState::waitUntilDone(JSContext* cx, ScriptKind kind) +{ + AutoLockMonitor alm(monitor); + if (state == IDLE || scriptKind != kind) + return nullptr; + + if (state == COMPILING) { + while (state != DONE) + alm.wait(); + } + + MOZ_ASSERT(source || !xdr.empty()); + if (source) + js_free(source); + source = nullptr; + xdr.clearAndFree(); + + MOZ_ASSERT(token); + void* holdToken = token; + token = nullptr; + state = IDLE; + return holdToken; +} struct ShellCompartmentPrivate { JS::Heap grayRoot; }; -// Per-context shell state. -struct ShellContext -{ - explicit ShellContext(JSContext* cx); - bool isWorker; - double timeoutInterval; - double startTime; - Atomic serviceInterrupt; - Atomic haveInterruptFunc; - JS::PersistentRootedValue interruptFunc; - bool lastWarningEnabled; - JS::PersistentRootedValue lastWarning; - JS::PersistentRootedValue promiseRejectionTrackerCallback; -#ifdef SINGLESTEP_PROFILING - Vector stacks; -#endif - - /* - * Watchdog thread state. - */ - Mutex watchdogLock; - ConditionVariable watchdogWakeup; - Maybe watchdogThread; - Maybe watchdogTimeout; - - ConditionVariable sleepWakeup; - - int exitCode; - bool quitting; - - UniqueChars readLineBuf; - size_t readLineBufPos; - - static const uint32_t GeckoProfilingMaxStackSize = 1000; - ProfileEntry geckoProfilingStack[GeckoProfilingMaxStackSize]; - mozilla::Atomic geckoProfilingStackSize; - - OffThreadState offThreadState; - - UniqueChars moduleLoadPath; - UniquePtr markObservers; -}; - struct MOZ_STACK_CLASS EnvironmentPreparer : public js::ScriptEnvironmentPreparer { JSContext* cx; explicit EnvironmentPreparer(JSContext* cx) @@ -476,11 +386,13 @@ ShellContext::ShellContext(JSContext* cx) exitCode(0), quitting(false), readLineBufPos(0), + errFilePtr(nullptr), + outFilePtr(nullptr), geckoProfilingStackSize(0) {} -static ShellContext* -GetShellContext(JSContext* cx) +ShellContext* +js::shell::GetShellContext(JSContext* cx) { ShellContext* sc = static_cast(JS_GetContextPrivate(cx)); MOZ_ASSERT(sc); diff --git a/js/src/shell/jsshell.h b/js/src/shell/jsshell.h index 29a69bbaf5be..53ac44a35ab9 100644 --- a/js/src/shell/jsshell.h +++ b/js/src/shell/jsshell.h @@ -7,8 +7,25 @@ #ifndef jsshell_js_h #define jsshell_js_h +#include "mozilla/Atomics.h" +#include "mozilla/Maybe.h" +#include "mozilla/TimeStamp.h" + #include "jsapi.h" +#include "js/GCVector.h" +#include "threading/ConditionVariable.h" +#include "threading/LockGuard.h" +#include "threading/Mutex.h" +#include "threading/Thread.h" +#include "vm/GeckoProfiler.h" +#include "vm/Monitor.h" + +// Some platform hooks must be implemented for single-step profiling. +#if defined(JS_SIMULATOR_ARM) || defined(JS_SIMULATOR_MIPS64) +# define SINGLESTEP_PROFILING +#endif + namespace js { namespace shell { @@ -90,6 +107,115 @@ struct RCFile { bool CreateAlias(JSContext* cx, const char* dstName, JS::HandleObject namespaceObj, const char* srcName); +enum class ScriptKind +{ + Script, + DecodeScript, + Module +}; + +class OffThreadState { + enum State { + IDLE, /* ready to work; no token, no source */ + COMPILING, /* working; no token, have source */ + DONE /* compilation done: have token and source */ + }; + + public: + OffThreadState() + : monitor(mutexid::ShellOffThreadState), + state(IDLE), + token(), + source(nullptr) + { } + + bool startIfIdle(JSContext* cx, ScriptKind kind, ScopedJSFreePtr& newSource); + + bool startIfIdle(JSContext* cx, ScriptKind kind, JS::TranscodeBuffer&& newXdr); + + void abandon(JSContext* cx); + + void markDone(void* newToken); + + void* waitUntilDone(JSContext* cx, ScriptKind kind); + + JS::TranscodeBuffer& xdrBuffer() { return xdr; } + + private: + js::Monitor monitor; + ScriptKind scriptKind; + State state; + void* token; + char16_t* source; + JS::TranscodeBuffer xdr; +}; + +class NonshrinkingGCObjectVector : public GCVector +{ + public: + void sweep() { + for (uint32_t i = 0; i < this->length(); i++) { + if (JS::GCPolicy::needsSweep(&(*this)[i])) + (*this)[i] = nullptr; + } + } +}; + +using MarkBitObservers = JS::WeakCache; + +#ifdef SINGLESTEP_PROFILING +using StackChars = Vector; +#endif + +// Per-context shell state. +struct ShellContext +{ + explicit ShellContext(JSContext* cx); + bool isWorker; + double timeoutInterval; + double startTime; + mozilla::Atomic serviceInterrupt; + mozilla::Atomic haveInterruptFunc; + JS::PersistentRootedValue interruptFunc; + bool lastWarningEnabled; + JS::PersistentRootedValue lastWarning; + JS::PersistentRootedValue promiseRejectionTrackerCallback; +#ifdef SINGLESTEP_PROFILING + Vector stacks; +#endif + + /* + * Watchdog thread state. + */ + js::Mutex watchdogLock; + js::ConditionVariable watchdogWakeup; + mozilla::Maybe watchdogThread; + mozilla::Maybe watchdogTimeout; + + js::ConditionVariable sleepWakeup; + + int exitCode; + bool quitting; + + JS::UniqueChars readLineBuf; + size_t readLineBufPos; + + js::shell::RCFile** errFilePtr; + js::shell::RCFile** outFilePtr; + + static const uint32_t GeckoProfilingMaxStackSize = 1000; + js::ProfileEntry geckoProfilingStack[GeckoProfilingMaxStackSize]; + mozilla::Atomic geckoProfilingStackSize; + + OffThreadState offThreadState; + + JS::UniqueChars moduleLoadPath; + UniquePtr markObservers; +}; + +extern ShellContext* +GetShellContext(JSContext* cx); + } /* namespace shell */ } /* namespace js */ From 63bc9a359598effa49bb66a38f4fd06d6e3be807 Mon Sep 17 00:00:00 2001 From: Steve Fink Date: Thu, 11 May 2017 20:00:13 -0700 Subject: [PATCH 12/20] Bug 1364288 - Fix thread races on global JS shell vars, r=jonco MozReview-Commit-ID: HcHf96vJAPP --HG-- extra : rebase_source : eff483c918d32ad9742722a7b13384c84e1b3c35 --- js/src/builtin/TestingFunctions.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/js/src/builtin/TestingFunctions.cpp b/js/src/builtin/TestingFunctions.cpp index 75c695359509..d6cad0167706 100644 --- a/js/src/builtin/TestingFunctions.cpp +++ b/js/src/builtin/TestingFunctions.cpp @@ -6,6 +6,7 @@ #include "builtin/TestingFunctions.h" +#include "mozilla/Atomics.h" #include "mozilla/FloatingPoint.h" #include "mozilla/Move.h" #include "mozilla/Sprintf.h" @@ -67,11 +68,11 @@ using mozilla::Move; // If fuzzingSafe is set, remove functionality that could cause problems with // fuzzers. Set this via the environment variable MOZ_FUZZING_SAFE. -static bool fuzzingSafe = false; +static mozilla::Atomic fuzzingSafe(false); // If disableOOMFunctions is set, disable functionality that causes artificial // OOM conditions. -static bool disableOOMFunctions = false; +static mozilla::Atomic disableOOMFunctions(false); static bool EnvVarIsDefined(const char* name) From 4e5e7e27da8053ec57e84d2b3d0b598c039cec74 Mon Sep 17 00:00:00 2001 From: Mason Chang Date: Fri, 19 May 2017 08:38:25 -0700 Subject: [PATCH 13/20] Bug 1365357 Fix DISPLAY_ITEM_USAGE_COUNT telemetry alert email address. r=chutten --- toolkit/components/telemetry/Histograms.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/components/telemetry/Histograms.json b/toolkit/components/telemetry/Histograms.json index cba53ff53bad..ac7be9eaa17a 100644 --- a/toolkit/components/telemetry/Histograms.json +++ b/toolkit/components/telemetry/Histograms.json @@ -12914,7 +12914,7 @@ }, "DISPLAY_ITEM_USAGE_COUNT": { "record_in_processes": ["main", "content"], - "alert_emails": ["mchang@mozilla.com", "gfx-telemetry@mozilla.com"], + "alert_emails": ["mchang@mozilla.com", "gfx-telemetry-alerts@mozilla.com"], "bug_numbers": [1353521], "expires_in_version": "56", "kind": "enumerated", From 5cf1e66fd87c7d6a3ca0dc23a2320775bb627b39 Mon Sep 17 00:00:00 2001 From: Ryan VanderMeulen Date: Fri, 19 May 2017 12:23:10 -0400 Subject: [PATCH 14/20] Backed out changeset 99ed53cfebd6 (bug 1298803) for causing Windows Marionette hangs. --- .../keyboard_shortcuts/test_browser_window.py | 25 ------------------- .../firefox/firefox_puppeteer/ui/windows.py | 3 ++- 2 files changed, 2 insertions(+), 26 deletions(-) diff --git a/testing/firefox-ui/tests/functional/keyboard_shortcuts/test_browser_window.py b/testing/firefox-ui/tests/functional/keyboard_shortcuts/test_browser_window.py index 2b5eef630a6f..5b656d0e5fd4 100644 --- a/testing/firefox-ui/tests/functional/keyboard_shortcuts/test_browser_window.py +++ b/testing/firefox-ui/tests/functional/keyboard_shortcuts/test_browser_window.py @@ -2,9 +2,6 @@ # 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/. -import sys -import unittest - from firefox_puppeteer import PuppeteerMixin from marionette_driver import Wait from marionette_harness import MarionetteTestCase @@ -57,25 +54,3 @@ class TestBrowserWindowShortcuts(PuppeteerMixin, MarionetteTestCase): return selection_name == "input" Wait(self.marionette).until(has_input_selected) - - -@unittest.skipIf(sys.platform == 'darwin', - 'Quit Shotcut not supported due to native menu of Mac OS') -class TestBrowserQuitShortcut(PuppeteerMixin, MarionetteTestCase): - - def test_quit_firefox_shortcut(self): - def quit_via_shortcut_callback(): - if self.puppeteer.platform == 'win': - key = 'quitApplicationCmdWin2.accesskey' - else: - key = 'quitApplicationCmdUnix.key' - - self.browser.send_shortcut(self.browser.localize_entity(key), - accel=True) - - self.marionette.quit(in_app=True, callback=quit_via_shortcut_callback) - self.assertIsNone(self.marionette.session) - - def tearDown(self): - self.marionette.start_session() - super(TestBrowserQuitShortcut, self).tearDown() diff --git a/testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/windows.py b/testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/windows.py index 7cf0e6946c4e..f81ba4b56869 100644 --- a/testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/windows.py +++ b/testing/marionette/puppeteer/firefox/firefox_puppeteer/ui/windows.py @@ -409,7 +409,8 @@ class BaseWindow(BaseLib): if kwargs[modifier] is True: keys.append(keymap[modifier]) - keys.append(command_key) + # Bug 1125209 - Only lower-case command keys should be sent + keys.append(command_key.lower()) self.switch_to() From 341bdc643d3e50c1c67f744aea136aebd3030068 Mon Sep 17 00:00:00 2001 From: Kris Maglione Date: Wed, 17 May 2017 22:55:14 -0700 Subject: [PATCH 15/20] Bug 1364934: Ignore cached scripts from content processes which were removed in a cache flush. r=erahm MozReview-Commit-ID: AnmsM3WiZMX --HG-- extra : rebase_source : 9c240099272cc1546dfe35764f130d5b6356f4e2 --- js/xpconnect/loader/ScriptPreloader.cpp | 31 ++++++++++++++++++++----- js/xpconnect/loader/ScriptPreloader.h | 3 ++- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/js/xpconnect/loader/ScriptPreloader.cpp b/js/xpconnect/loader/ScriptPreloader.cpp index 35a73b855ea7..cc3efc148398 100644 --- a/js/xpconnect/loader/ScriptPreloader.cpp +++ b/js/xpconnect/loader/ScriptPreloader.cpp @@ -33,7 +33,7 @@ #define DOC_ELEM_INSERTED_TOPIC "document-element-inserted" #define CLEANUP_TOPIC "xpcom-shutdown" #define SHUTDOWN_TOPIC "quit-application-granted" -#define CACHE_FLUSH_TOPIC "startupcache-invalidate" +#define CACHE_INVALIDATE_TOPIC "startupcache-invalidate" namespace mozilla { namespace { @@ -163,7 +163,9 @@ ScriptPreloader::InitContentChild(ContentParent& parent) cache.mInitializedProcesses += processType; auto fd = cache.mCacheData.cloneFileDescriptor(); - if (fd.IsValid()) { + // Don't send original cache data to new processes if the cache has been + // invalidated. + if (fd.IsValid() && !cache.mCacheInvalidated) { Unused << parent.SendPScriptCacheConstructor(fd, wantScriptData); } else { Unused << parent.SendPScriptCacheConstructor(NS_ERROR_FILE_NOT_FOUND, wantScriptData); @@ -227,7 +229,7 @@ ScriptPreloader::ScriptPreloader() } obs->AddObserver(this, SHUTDOWN_TOPIC, false); obs->AddObserver(this, CLEANUP_TOPIC, false); - obs->AddObserver(this, CACHE_FLUSH_TOPIC, false); + obs->AddObserver(this, CACHE_INVALIDATE_TOPIC, false); AutoSafeJSAPI jsapi; JS_AddExtraGCRootsTracer(jsapi.cx(), TraceOp, this); @@ -265,10 +267,12 @@ ScriptPreloader::Cleanup() } void -ScriptPreloader::FlushCache() +ScriptPreloader::InvalidateCache() { MonitorAutoLock mal(mMonitor); + mCacheInvalidated = true; + for (auto& script : IterHash(mScripts)) { // We can only purge finished scripts here. Async scripts that are // still being parsed off-thread have a non-refcounted reference to @@ -321,8 +325,8 @@ ScriptPreloader::Observe(nsISupports* subject, const char* topic, const char16_t ForceWriteCacheFile(); } else if (!strcmp(topic, CLEANUP_TOPIC)) { Cleanup(); - } else if (!strcmp(topic, CACHE_FLUSH_TOPIC)) { - FlushCache(); + } else if (!strcmp(topic, CACHE_INVALIDATE_TOPIC)) { + InvalidateCache(); } return NS_OK; @@ -724,6 +728,21 @@ ScriptPreloader::NoteScript(const nsCString& url, const nsCString& cachePath, } } + if (!script->mSize && !script->mScript) { + // If the content process is sending us a script entry for a script + // which was in the cache at startup, it expects us to already have this + // script data, so it doesn't send it. + // + // However, the cache may have been invalidated at this point (usually + // due to the add-on manager installing or uninstalling a legacy + // extension during very early startup), which means we may no longer + // have an entry for this script. Since that means we have no data to + // write to the new cache, and no JSScript to generate it from, we need + // to discard this entry. + mScripts.Remove(cachePath); + return; + } + script->UpdateLoadTime(loadTime); script->mProcessTypes += processType; } diff --git a/js/xpconnect/loader/ScriptPreloader.h b/js/xpconnect/loader/ScriptPreloader.h index 80fc5bec729c..5ff46cd24fc7 100644 --- a/js/xpconnect/loader/ScriptPreloader.h +++ b/js/xpconnect/loader/ScriptPreloader.h @@ -350,7 +350,7 @@ private: void ForceWriteCacheFile(); void Cleanup(); - void FlushCache(); + void InvalidateCache(); // Opens the cache file for reading. Result OpenCache(); @@ -404,6 +404,7 @@ private: bool mCacheInitialized = false; bool mSaveComplete = false; bool mDataPrepared = false; + bool mCacheInvalidated = false; // The process type of the current process. static ProcessType sProcessType; From 0c9253b86ddaddf38470faddae0f08c7a7f4a349 Mon Sep 17 00:00:00 2001 From: Randall Barker Date: Wed, 17 May 2017 15:18:04 -0700 Subject: [PATCH 16/20] Bug 1365161 - Ensure dynamic toolbar static snapshot visibility stays in sync with the real toolbar chrome r=botond,jchen There were two issues that prevented the static snapshot toolbar and real chrome toolbar from staying in sync. 1) When a page would resize such as when going fullscreen, if the root content document was not scrollable, the animator would not receive root composition page size updates. The page resize is used by the animator to hide the static snapshot, so it would remain visible while the real chrome toolbar would be hidden. 2) Certain places in UI java code would toggle the chrome state directly instead of going through the animator to change the state. MozReview-Commit-ID: DCQgRFS0UAO --- .../apz/src/AndroidDynamicToolbarAnimator.cpp | 76 +++++++++++-------- .../apz/src/AndroidDynamicToolbarAnimator.h | 7 +- gfx/layers/apz/src/AsyncPanZoomController.cpp | 7 +- .../composite/AsyncCompositionManager.cpp | 23 +++--- .../java/org/mozilla/gecko/BrowserApp.java | 25 +++--- 5 files changed, 74 insertions(+), 64 deletions(-) diff --git a/gfx/layers/apz/src/AndroidDynamicToolbarAnimator.cpp b/gfx/layers/apz/src/AndroidDynamicToolbarAnimator.cpp index c236bd7be894..36efcf6ed0a8 100644 --- a/gfx/layers/apz/src/AndroidDynamicToolbarAnimator.cpp +++ b/gfx/layers/apz/src/AndroidDynamicToolbarAnimator.cpp @@ -67,6 +67,7 @@ AndroidDynamicToolbarAnimator::AndroidDynamicToolbarAnimator() , mCompositorReceivedFirstPaint(false) , mCompositorWaitForPageResize(false) , mCompositorToolbarShowRequested(false) + , mCompositorSendResponseForSnapshotUpdate(false) , mCompositorAnimationStyle(eAnimate) , mCompositorMaxToolbarHeight(0) , mCompositorToolbarHeight(0) @@ -270,30 +271,6 @@ AndroidDynamicToolbarAnimator::GetCompositionHeight() const return mCompositorCompositionSize.height; } -bool -AndroidDynamicToolbarAnimator::SetCompositionSize(ScreenIntSize aSize) -{ - MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread()); - if (mCompositorCompositionSize == aSize) { - return false; - } - - ScreenIntSize prevSize = mCompositorCompositionSize; - mCompositorCompositionSize = aSize; - - // The width has changed so the static snapshot needs to be updated - if ((prevSize.width != aSize.width) && (mToolbarState != eToolbarVisible)) { - PostMessage(STATIC_TOOLBAR_NEEDS_UPDATE); - } - - if (prevSize.height != aSize.height) { - UpdateControllerCompositionHeight(aSize.height); - UpdateFixedLayerMargins(); - } - - return true; -} - void AndroidDynamicToolbarAnimator::SetScrollingRootContent() { @@ -318,8 +295,8 @@ AndroidDynamicToolbarAnimator::ToolbarAnimatorMessageFromUI(int32_t aMessage) StartCompositorAnimation(mCompositorAnimationDirection, mCompositorAnimationStyle, mCompositorToolbarHeight, mCompositorWaitForPageResize); } } else { - // The compositor is already animating the toolbar so no need to defer. - mCompositorAnimationDeferred = false; + // Animation already running so just make sure it is going the right direction. + StartCompositorAnimation(MOVE_TOOLBAR_UP, mCompositorAnimationStyle, mCompositorToolbarHeight, mCompositorWaitForPageResize); } break; case TOOLBAR_VISIBLE: @@ -470,6 +447,35 @@ AndroidDynamicToolbarAnimator::UpdateRootFrameMetrics(const FrameMetrics& aMetri UpdateFrameMetrics(scrollOffset, scale, cssPageRect); } +void +AndroidDynamicToolbarAnimator::MaybeUpdateCompositionSizeAndRootFrameMetrics(const FrameMetrics& aMetrics) +{ + MOZ_ASSERT(CompositorThreadHolder::IsInCompositorThread()); + CSSToScreenScale scale = ViewTargetAs(aMetrics.GetZoom().ToScaleFactor(), + PixelCastJustification::ScreenIsParentLayerForRoot); + ScreenIntSize size = ScreenIntSize::Round(aMetrics.GetRootCompositionSize() * scale); + + if (mCompositorCompositionSize == size) { + return; + } + + ScreenIntSize prevSize = mCompositorCompositionSize; + mCompositorCompositionSize = size; + + // The width has changed so the static snapshot needs to be updated + if ((prevSize.width != size.width) && (mToolbarState == eToolbarUnlocked)) { + // No need to set mCompositorSendResponseForSnapshotUpdate. If it is already true we don't want to change it. + PostMessage(STATIC_TOOLBAR_NEEDS_UPDATE); + } + + if (prevSize.height != size.height) { + UpdateControllerCompositionHeight(size.height); + UpdateFixedLayerMargins(); + } + + UpdateRootFrameMetrics(aMetrics); +} + // Layers updates are need by Robocop test which enables them void AndroidDynamicToolbarAnimator::EnableLayersUpdateNotifications(bool aEnable) @@ -525,7 +531,8 @@ AndroidDynamicToolbarAnimator::GetToolbarEffect(CompositorOGL* gl) uiController->DeallocShmem(mCompositorToolbarPixels.ref()); mCompositorToolbarPixels.reset(); // Send notification that texture is ready after the current composition has completed. - if (mCompositorToolbarTexture) { + if (mCompositorToolbarTexture && mCompositorSendResponseForSnapshotUpdate) { + mCompositorSendResponseForSnapshotUpdate = false; CompositorThreadHolder::Loop()->PostTask(NewRunnableMethod(this, &AndroidDynamicToolbarAnimator::PostToolbarReady)); } } @@ -574,6 +581,7 @@ AndroidDynamicToolbarAnimator::ProcessTouchDelta(StaticToolbarState aCurrentTool if (aCurrentToolbarState == eToolbarVisible) { if (tryingToHideToolbar && (mControllerState != eUnlockPending)) { + mCompositorSendResponseForSnapshotUpdate = true; PostMessage(STATIC_TOOLBAR_NEEDS_UPDATE); mControllerState = eUnlockPending; } @@ -796,7 +804,7 @@ AndroidDynamicToolbarAnimator::UpdateFixedLayerMargins() } AsyncCompositionManager* manager = parent->GetCompositionManager(nullptr); if (manager) { - if (mToolbarState == eToolbarAnimating) { + if ((mToolbarState == eToolbarAnimating) && mCompositorAnimationStarted) { manager->SetFixedLayerMargins(mCompositorToolbarHeight, 0); } else { manager->SetFixedLayerMargins(0, GetFixedLayerMarginsBottom()); @@ -851,6 +859,7 @@ AndroidDynamicToolbarAnimator::StartCompositorAnimation(int32_t aDirection, Anim // and defer animation until it has been unlocked if ((initialToolbarState != eToolbarUnlocked) && (initialToolbarState != eToolbarAnimating)) { mCompositorAnimationDeferred = true; + mCompositorSendResponseForSnapshotUpdate = true; PostMessage(STATIC_TOOLBAR_NEEDS_UPDATE); } else { // Toolbar is either unlocked or already animating so animation may begin immediately @@ -859,13 +868,14 @@ AndroidDynamicToolbarAnimator::StartCompositorAnimation(int32_t aDirection, Anim if (initialToolbarState != eToolbarAnimating) { mCompositorAnimationStarted = false; } - // Let the controller know we starting an animation so it may clear the AnimationStartPending flag. + // Let the controller know we are starting an animation so it may clear the AnimationStartPending flag. NotifyControllerAnimationStarted(); - CompositorBridgeParent* parent = CompositorBridgeParent::GetCompositorBridgeParentFromLayersId(mRootLayerTreeId); - if (parent) { - mCompositorAnimationStartTimeStamp = parent->GetAPZCTreeManager()->GetFrameTime(); - } + // Only reset the time stamp and start compositor animation if not already animating. if (initialToolbarState != eToolbarAnimating) { + CompositorBridgeParent* parent = CompositorBridgeParent::GetCompositorBridgeParentFromLayersId(mRootLayerTreeId); + if (parent) { + mCompositorAnimationStartTimeStamp = parent->GetAPZCTreeManager()->GetFrameTime(); + } // Kick the compositor to start the animation if we aren't already animating. RequestComposite(); } diff --git a/gfx/layers/apz/src/AndroidDynamicToolbarAnimator.h b/gfx/layers/apz/src/AndroidDynamicToolbarAnimator.h index ac514333161c..7cf56daacd3b 100644 --- a/gfx/layers/apz/src/AndroidDynamicToolbarAnimator.h +++ b/gfx/layers/apz/src/AndroidDynamicToolbarAnimator.h @@ -81,8 +81,6 @@ public: // shown, the content may extend beyond the bottom of the surface until the toolbar is completely // visible or hidden. ScreenIntCoord GetCompositionHeight() const; - // Returns true if the composition size has changed from the last time it was set. - bool SetCompositionSize(ScreenIntSize aSize); // Called to signal that root content is being scrolled. This prevents sub scroll frames from // affecting the toolbar when being scrolled up. The idea is a scrolling down will always // show the toolbar while scrolling up will only hide the toolbar if it is the root content @@ -95,6 +93,8 @@ public: void FirstPaint(); // Called whenever the root document's FrameMetrics have reached a steady state. void UpdateRootFrameMetrics(const FrameMetrics& aMetrics); + // Only update the frame metrics if the root composition size has changed + void MaybeUpdateCompositionSizeAndRootFrameMetrics(const FrameMetrics& aMetrics); // When aEnable is set to true, it informs the animator that the UI thread expects to // be notified when the layer tree has been updated. Enabled currently by robocop tests. void EnableLayersUpdateNotifications(bool aEnable); @@ -225,7 +225,8 @@ protected: bool mCompositorReceivedFirstPaint; // Set to true when a first paint occurs. Used by toolbar animator to detect a new page load. bool mCompositorWaitForPageResize; // Set to true if the bottom of the page has been reached and the toolbar animator should wait for the page to resize before ending animation. bool mCompositorToolbarShowRequested; // Set to true if the animator has already requested the real toolbar chrome be shown - AnimationStyle mCompositorAnimationStyle; // Set to true when the snap should be immediately hidden or shown in the animation update + bool mCompositorSendResponseForSnapshotUpdate; // Set to true when a message should be sent after a static toolbar snapshot update + AnimationStyle mCompositorAnimationStyle; // Set to true when the snapshot should be immediately hidden or shown in the animation update ScreenIntCoord mCompositorMaxToolbarHeight; // Should contain the same value as mControllerMaxToolbarHeight ScreenIntCoord mCompositorToolbarHeight; // This value is only updated by the compositor thread when the mToolbarState == ToolbarAnimating ScreenIntCoord mCompositorSurfaceHeight; // Current height of the render surface diff --git a/gfx/layers/apz/src/AsyncPanZoomController.cpp b/gfx/layers/apz/src/AsyncPanZoomController.cpp index 59d312af5ce6..3efa3b50a068 100644 --- a/gfx/layers/apz/src/AsyncPanZoomController.cpp +++ b/gfx/layers/apz/src/AsyncPanZoomController.cpp @@ -3477,12 +3477,7 @@ void AsyncPanZoomController::NotifyLayersUpdated(const ScrollMetadata& aScrollMe if (APZCTreeManager* manager = GetApzcTreeManager()) { AndroidDynamicToolbarAnimator* animator = manager->GetAndroidDynamicToolbarAnimator(); MOZ_ASSERT(animator); - CSSToScreenScale scale = ViewTargetAs(aLayerMetrics.GetZoom().ToScaleFactor(), - PixelCastJustification::ScreenIsParentLayerForRoot); - ScreenIntSize size = ScreenIntSize::Round(aLayerMetrics.GetRootCompositionSize() * scale); - if (animator->SetCompositionSize(size)) { - animator->UpdateRootFrameMetrics(aLayerMetrics); - } + animator->MaybeUpdateCompositionSizeAndRootFrameMetrics(aLayerMetrics); } } #endif diff --git a/gfx/layers/composite/AsyncCompositionManager.cpp b/gfx/layers/composite/AsyncCompositionManager.cpp index 1fe78140d219..9374634983e1 100644 --- a/gfx/layers/composite/AsyncCompositionManager.cpp +++ b/gfx/layers/composite/AsyncCompositionManager.cpp @@ -945,23 +945,24 @@ AsyncCompositionManager::ApplyAsyncContentTransformToTree(Layer *aLayer, if (*aOutFoundRoot) { mRootScrollableId = metrics.GetScrollId(); Compositor* compositor = mLayerManager->GetCompositor(); - if (mIsFirstPaint) { - if (CompositorBridgeParent* bridge = compositor->GetCompositorBridgeParent()) { - AndroidDynamicToolbarAnimator* animator = bridge->GetAPZCTreeManager()->GetAndroidDynamicToolbarAnimator(); - MOZ_ASSERT(animator); + if (CompositorBridgeParent* bridge = compositor->GetCompositorBridgeParent()) { + AndroidDynamicToolbarAnimator* animator = bridge->GetAPZCTreeManager()->GetAndroidDynamicToolbarAnimator(); + MOZ_ASSERT(animator); + if (mIsFirstPaint) { animator->UpdateRootFrameMetrics(metrics); animator->FirstPaint(); + mIsFirstPaint = false; } - } - if (mLayersUpdated) { - if (CompositorBridgeParent* bridge = compositor->GetCompositorBridgeParent()) { - AndroidDynamicToolbarAnimator* animator = bridge->GetAPZCTreeManager()->GetAndroidDynamicToolbarAnimator(); - MOZ_ASSERT(animator); + if (mLayersUpdated) { animator->NotifyLayersUpdated(); + mLayersUpdated = false; + } + // If this is not actually the root content then the animator is not getting updated in AsyncPanZoomController::NotifyLayersUpdated + // because the root content document is not scrollable. So update it here so it knows if the root composition size has changed. + if (!metrics.IsRootContent()) { + animator->MaybeUpdateCompositionSizeAndRootFrameMetrics(metrics); } - mLayersUpdated = false; } - mIsFirstPaint = false; fixedLayerMargins = mFixedLayerMargins; } } diff --git a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java index 63b403bb7d21..ea080347c293 100644 --- a/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java +++ b/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java @@ -1705,7 +1705,15 @@ public class BrowserApp extends GeckoApp @Override public void toggleToolbarChrome(final boolean aShow) { - toggleChrome(aShow); + if (aShow) { + mBrowserChrome.setVisibility(View.VISIBLE); + } else { + // The chrome needs to be INVISIBLE instead of GONE so that + // it will continue update when the layout changes. This + // ensures the bitmap generated for the static toolbar + // snapshot is the correct size. + mBrowserChrome.setVisibility(View.INVISIBLE); + } } public void refreshToolbarHeight() { @@ -1727,22 +1735,17 @@ public class BrowserApp extends GeckoApp @Override void toggleChrome(final boolean aShow) { - if (aShow) { - mBrowserChrome.setVisibility(View.VISIBLE); - } else { - // The chrome needs to be INVISIBLE instead of GONE so that - // it will continue update when the layout changes. This - // ensures the bitmap generated for the static toolbar - // snapshot is the correct size. - mBrowserChrome.setVisibility(View.INVISIBLE); + if (mDynamicToolbar != null) { + mDynamicToolbar.setVisible(aShow, VisibilityTransition.IMMEDIATE); } - super.toggleChrome(aShow); } @Override void focusChrome() { - mBrowserChrome.setVisibility(View.VISIBLE); + if (mDynamicToolbar != null) { + mDynamicToolbar.setVisible(true, VisibilityTransition.IMMEDIATE); + } mActionBarFlipper.requestFocusFromTouch(); super.focusChrome(); From 14a83fcff4b226701bde8e51b4ffa63df86305bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A3o=20Gottwald?= Date: Fri, 19 May 2017 16:50:10 +0200 Subject: [PATCH 17/20] Bug 1366278 - Get rid of --urlbar-background-color since it's always -moz-field and textbox.css already sets that by default. r=dale MozReview-Commit-ID: C0nBOYl6CYd --- browser/themes/linux/browser.css | 10 +++++----- browser/themes/osx/browser.css | 1 - browser/themes/shared/urlbar-searchbar.inc.css | 1 - browser/themes/windows/browser.css | 1 - 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/browser/themes/linux/browser.css b/browser/themes/linux/browser.css index 87d6bf9e9563..b14327088c62 100644 --- a/browser/themes/linux/browser.css +++ b/browser/themes/linux/browser.css @@ -268,13 +268,17 @@ menuitem.bookmark-item { #main-window { --urlbar-border-color: ThreeDShadow; --urlbar-border-color-hover: var(--urlbar-border-color); - --urlbar-background-color: -moz-field; } #navigator-toolbox:-moz-lwtheme { --urlbar-border-color: rgba(0,0,0,.3); } +#urlbar { + /* override textbox[enablehistory="true"] styling: */ + background-color: -moz-field; +} + %include ../shared/urlbar-searchbar.inc.css %ifdef MOZ_PHOTON_THEME @@ -301,10 +305,6 @@ menuitem.bookmark-item { border-color: Highlight; } -#urlbar { - background-color: -moz-field; -} - #urlbar:-moz-lwtheme, .searchbar-textbox:-moz-lwtheme { background-color: rgba(255,255,255,.8); diff --git a/browser/themes/osx/browser.css b/browser/themes/osx/browser.css index 25f529fb6428..c29a51741ea2 100644 --- a/browser/themes/osx/browser.css +++ b/browser/themes/osx/browser.css @@ -527,7 +527,6 @@ toolbarpaletteitem[place="palette"] > #personal-bookmarks > #bookmarks-toolbar-p #main-window { --urlbar-border-color: hsla(240, 5%, 5%, .25); --urlbar-border-color-hover: hsla(240, 5%, 5%, .35); - --urlbar-background-color: -moz-field; } #urlbar[focused="true"], diff --git a/browser/themes/shared/urlbar-searchbar.inc.css b/browser/themes/shared/urlbar-searchbar.inc.css index 4a3053f10272..f4e51bb6a0f0 100644 --- a/browser/themes/shared/urlbar-searchbar.inc.css +++ b/browser/themes/shared/urlbar-searchbar.inc.css @@ -8,7 +8,6 @@ .searchbar-textbox { -moz-appearance: none; background-clip: content-box; - background-color: var(--urlbar-background-color); border: 1px solid var(--urlbar-border-color); border-radius: var(--toolbarbutton-border-radius); box-shadow: 0 1px 4px hsla(0, 0%, 0%, .05); diff --git a/browser/themes/windows/browser.css b/browser/themes/windows/browser.css index 6a9bbc4a2c92..1048b1d3bfbd 100644 --- a/browser/themes/windows/browser.css +++ b/browser/themes/windows/browser.css @@ -670,7 +670,6 @@ toolbar[brighttext] #close-button { #main-window:not(:-moz-lwtheme) { --urlbar-border-color: hsla(240, 5%, 5%, .25); --urlbar-border-color-hover: hsla(240, 5%, 5%, .35); - --urlbar-background-color: -moz-field; } } From 935d7cfd4bf331cb7c55938434fe7a99449b7dd3 Mon Sep 17 00:00:00 2001 From: Geoff Brown Date: Fri, 19 May 2017 12:06:12 -0600 Subject: [PATCH 18/20] Bug 1260312 - Remove a race in the mochitest-chrome harness; r=jmaher Start listening for "chromeEvent" before loading the script that sends it. --- testing/mochitest/browser-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/mochitest/browser-test.js b/testing/mochitest/browser-test.js index 8e3eac8b1eae..0ee759b44ecf 100644 --- a/testing/mochitest/browser-test.js +++ b/testing/mochitest/browser-test.js @@ -97,8 +97,8 @@ function testInit() { }; var listener = 'data:,function doLoad(e) { var data=e.detail&&e.detail.data;removeEventListener("contentEvent", function (e) { doLoad(e); }, false, true);sendAsyncMessage("chromeEvent", {"data":data}); };addEventListener("contentEvent", function (e) { doLoad(e); }, false, true);'; - messageManager.loadFrameScript(listener, true); messageManager.addMessageListener("chromeEvent", messageHandler); + messageManager.loadFrameScript(listener, true); } if (gConfig.e10s) { e10s_init(); From 9552c26f47d4192a7be7d1dc91f1682648b8eb0a Mon Sep 17 00:00:00 2001 From: Geoff Brown Date: Fri, 19 May 2017 12:06:14 -0600 Subject: [PATCH 19/20] Bug 1260312 - Retry mochitest-chrome redirection to improve reliability; r=jmaher The mochitest-chrome harness sends a custom "contentEvent" event from redirect.html to a listener in browser-test.js. It is possible for redirect.html to be loaded and send contentEvent before the listener is set up in browser-test.js. In the absence of a better synchronization strategy, redirect.html now retries the send after a few seconds. If the first contentEvent was received, the second will be ignored; if the first contentEvent was not received, the second should avoid an intermittent harness hang and timeout. --- testing/mochitest/redirect.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/testing/mochitest/redirect.html b/testing/mochitest/redirect.html index 72190b8fab9a..f85d19b5d386 100644 --- a/testing/mochitest/redirect.html +++ b/testing/mochitest/redirect.html @@ -27,6 +27,8 @@ // added until then. window.addEventListener("MozAfterPaint", function() { setTimeout(redirectToHarness, 0); + // In case the listener was not ready, try again after a few seconds. + setTimeout(redirectToHarness, 5000); }, {once: true}); } From c0fe8b0e0e60645cebcae81ab37e8936b05aa865 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Fri, 19 May 2017 11:30:34 -0700 Subject: [PATCH 20/20] Make "prepare" times more inclusive in the compositor diagnostic overlay. (bug 1366037, r=mattwoodrow) --- gfx/layers/composite/Diagnostics.h | 4 ++-- gfx/layers/composite/LayerManagerComposite.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gfx/layers/composite/Diagnostics.h b/gfx/layers/composite/Diagnostics.h index 2ffaf3689d74..834856e02299 100644 --- a/gfx/layers/composite/Diagnostics.h +++ b/gfx/layers/composite/Diagnostics.h @@ -81,9 +81,9 @@ public: class Record { public: - Record() { + explicit Record(TimeStamp aStart = TimeStamp()) { if (gfxPrefs::LayersDrawFPS()) { - mStart = TimeStamp::Now(); + mStart = aStart.IsNull() ? TimeStamp::Now() : aStart; } } bool Recording() const { diff --git a/gfx/layers/composite/LayerManagerComposite.cpp b/gfx/layers/composite/LayerManagerComposite.cpp index bffe321ca70e..ca9c5f650e54 100644 --- a/gfx/layers/composite/LayerManagerComposite.cpp +++ b/gfx/layers/composite/LayerManagerComposite.cpp @@ -919,7 +919,7 @@ LayerManagerComposite::Render(const nsIntRegion& aInvalidRegion, const nsIntRegi // Render our layers. { - Diagnostics::Record record; + Diagnostics::Record record(mRenderStartTime); RootLayer()->Prepare(ViewAs(clipRect, PixelCastJustification::RenderTargetIsParentLayerForRoot)); if (record.Recording()) { mDiagnostics->RecordPrepareTime(record.Duration());