Skip to content

Commit

Permalink
Bug 1257335. Replace some AutoSafeJSContext uses with AutoJSAPI or Au…
Browse files Browse the repository at this point in the history
…toJSContext uses. r=bholley

In general, using an AutoJSAPI inited with an object is NOT the same as using
AutoSafeJSContext (or AutoJSAPI inited without an object) and then entering the
compartment of the object: the former will report exceptions to the global of
the object as it comes off the stack, while the latter will not.  This only
really matters if we have an object from a window or worker global and hence
might fire error events, or report internal stuff to the web console.

The changes to initing with an object made in this bug are OK for the following
reasons:

1) dom/base/Console.cpp: Always clears its exception before coming off the stack.
2) dom/base/nsDOMClassInfo.cpp: Inits with a non-web global.
3) dom/base/nsFrameMessageManager.cpp: Inits with a non-web global.
4) dom/media/MediaPermissionGonk.cpp: We probably want the caller to notice if
   anything here throws.
5) dom/xbl/nsXBLPrototypeBinding.cpp: Inits with a non-web global.
6) dom/xul/nsXULElement.cpp: Inits with a non-web global.
7) extensions/pref/autoconfig/src/nsJSConfigTriggers.cpp: Inits with a non-web global.
8) ipc/testshell/XPCShellEnvironment.cpp: Inits with a non-web global.
  • Loading branch information
bzbarsky committed Mar 18, 2016
1 parent aeec6d5 commit 9bf80f0
Show file tree
Hide file tree
Showing 21 changed files with 138 additions and 85 deletions.
8 changes: 5 additions & 3 deletions dom/base/Console.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1333,12 +1333,14 @@ Console::ProcessCallData(ConsoleCallData* aData, JS::Handle<JSObject*> aGlobal,
frame = *aData->mTopStackFrame;
}

AutoSafeJSContext cx;
AutoJSAPI jsapi;
if (!jsapi.Init(aGlobal)) {
return;
}
JSContext* cx = jsapi.cx();
ClearException ce(cx);
RootedDictionary<ConsoleEvent> event(cx);

JSAutoCompartment ac(cx, aGlobal);

event.mID.Construct();
event.mInnerID.Construct();

Expand Down
12 changes: 12 additions & 0 deletions dom/base/ScriptSettings.h
Original file line number Diff line number Diff line change
Expand Up @@ -221,12 +221,20 @@ class MOZ_STACK_CLASS AutoJSAPI {
// This uses the SafeJSContext (or worker equivalent), and enters a null
// compartment, so that the consumer is forced to select a compartment to
// enter before manipulating objects.
//
// This variant will ensure that any errors reported by this AutoJSAPI as it
// comes off the stack will not fire error events or be associated with any
// particular web-visible global.
void Init();

// This uses the SafeJSContext (or worker equivalent), and enters the
// compartment of aGlobalObject.
// If aGlobalObject or its associated JS global are null then it returns
// false and use of cx() will cause an assertion.
//
// If aGlobalObject represents a web-visible global, errors reported by this
// AutoJSAPI as it comes off the stack will fire the relevant error events and
// show up in the corresponding web console.
bool Init(nsIGlobalObject* aGlobalObject);

// This is a helper that grabs the native global associated with aObject and
Expand All @@ -237,6 +245,10 @@ class MOZ_STACK_CLASS AutoJSAPI {
// If aGlobalObject or its associated JS global are null then it returns
// false and use of cx() will cause an assertion.
// If aCx is null it will cause an assertion.
//
// If aGlobalObject represents a web-visible global, errors reported by this
// AutoJSAPI as it comes off the stack will fire the relevant error events and
// show up in the corresponding web console.
bool Init(nsIGlobalObject* aGlobalObject, JSContext* aCx);

// Convenience functions to take an nsPIDOMWindow* or nsGlobalWindow*,
Expand Down
12 changes: 7 additions & 5 deletions dom/base/nsDOMClassInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -325,9 +325,13 @@ nsDOMClassInfo::GetNative(nsIXPConnectWrappedNative *wrapper, JSObject *obj)
}

nsresult
nsDOMClassInfo::DefineStaticJSVals(JSContext *cx)
nsDOMClassInfo::DefineStaticJSVals()
{
#define SET_JSID_TO_STRING(_id, _cx, _str) \
AutoJSAPI jsapi;
jsapi.Init(xpc::UnprivilegedJunkScope());
JSContext* cx = jsapi.cx();

#define SET_JSID_TO_STRING(_id, _cx, _str) \
if (JSString *str = ::JS_AtomizeAndPinString(_cx, _str)) \
_id = INTERNED_STRING_TO_JSID(_cx, str); \
else \
Expand Down Expand Up @@ -500,8 +504,6 @@ nsDOMClassInfo::Init()
nsCOMPtr<nsIXPCFunctionThisTranslator> elt = new nsEventListenerThisTranslator();
sXPConnect->SetFunctionThisTranslator(NS_GET_IID(nsIDOMEventListener), elt);

AutoSafeJSContext cx;

DOM_CLASSINFO_MAP_BEGIN_NO_CLASS_IF(DOMPrototype, nsIDOMDOMConstructor)
DOM_CLASSINFO_MAP_ENTRY(nsIDOMDOMConstructor)
DOM_CLASSINFO_MAP_END
Expand Down Expand Up @@ -665,7 +667,7 @@ nsDOMClassInfo::Init()
#endif

// Initialize static JSString's
DefineStaticJSVals(cx);
DefineStaticJSVals();

int32_t i;

Expand Down
2 changes: 1 addition & 1 deletion dom/base/nsDOMClassInfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class nsDOMClassInfo : public nsXPCClassInfo
static nsIXPConnect *sXPConnect;

// nsIXPCScriptable code
static nsresult DefineStaticJSVals(JSContext *cx);
static nsresult DefineStaticJSVals();

static bool sIsInitialized;

Expand Down
13 changes: 6 additions & 7 deletions dom/base/nsFrameMessageManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1646,7 +1646,6 @@ void
nsMessageManagerScriptExecutor::Shutdown()
{
if (sCachedScripts) {
AutoSafeJSContext cx;
NS_ASSERTION(sCachedScripts != nullptr, "Need cached scripts");
for (auto iter = sCachedScripts->Iter(); !iter.Done(); iter.Next()) {
delete iter.Data();
Expand Down Expand Up @@ -1761,12 +1760,13 @@ nsMessageManagerScriptExecutor::TryCacheLoadAndCompileScript(
JS::SourceBufferHolder::GiveOwnership);

if (dataStringBuf && dataStringLength > 0) {
AutoSafeJSContext cx;
// Compile the script in the compilation scope instead of the current global
// to avoid keeping the current compartment alive.
JS::Rooted<JSObject*> global(cx, xpc::CompilationScope());

JSAutoCompartment ac(cx, global);
AutoJSAPI jsapi;
if (!jsapi.Init(xpc::CompilationScope())) {
return;
}
JSContext* cx = jsapi.cx();
JS::CompileOptions options(cx, JSVERSION_LATEST);
options.setFileAndLine(url.get(), 1);
options.setNoScriptRval(true);
Expand Down Expand Up @@ -1801,8 +1801,7 @@ nsMessageManagerScriptExecutor::TryCacheLoadAndCompileScript(
const nsAString& aURL,
bool aRunInGlobalScope)
{
AutoSafeJSContext cx;
JS::Rooted<JSScript*> script(cx);
JS::Rooted<JSScript*> script(nsContentUtils::RootingCx());
TryCacheLoadAndCompileScript(aURL, aRunInGlobalScope, true, &script);
}

Expand Down
14 changes: 10 additions & 4 deletions dom/base/nsGlobalWindow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4012,7 +4012,9 @@ nsGlobalWindow::GetMozSelfSupport(ErrorResult& aError)
return mMozSelfSupport;
}

AutoSafeJSContext cx;
// We're called from JS and want to use out existing JSContext (and,
// importantly, its compartment!) here.
AutoJSContext cx;
GlobalObject global(cx, FastGetGlobalJSObject());
mMozSelfSupport = MozSelfSupport::Constructor(global, cx, aError);
return mMozSelfSupport;
Expand Down Expand Up @@ -5700,8 +5702,13 @@ nsGlobalWindow::DispatchResizeEvent(const CSSIntSize& aSize)
return false;
}

AutoSafeJSContext cx;
// We don't init the AutoJSAPI with ourselves because we don't want it
// reporting errors to our onerror handlers.
AutoJSAPI jsapi;
jsapi.Init();
JSContext* cx = jsapi.cx();
JSAutoCompartment ac(cx, GetWrapperPreserveColor());

DOMWindowResizeEventDetail detail;
detail.mWidth = aSize.width;
detail.mHeight = aSize.height;
Expand Down Expand Up @@ -8672,8 +8679,7 @@ nsGlobalWindow::NotifyDOMWindowThawed(nsGlobalWindow* aWindow) {
JSObject*
nsGlobalWindow::GetCachedXBLPrototypeHandler(nsXBLPrototypeHandler* aKey)
{
AutoSafeJSContext cx;
JS::Rooted<JSObject*> handler(cx);
JS::Rooted<JSObject*> handler(nsContentUtils::RootingCx());
if (mCachedXBLPrototypeHandlers) {
mCachedXBLPrototypeHandlers->Get(aKey, handler.address());
}
Expand Down
12 changes: 8 additions & 4 deletions dom/base/nsObjectLoadingContent.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3720,11 +3720,15 @@ nsObjectLoadingContent::TeardownProtoChain()
nsCOMPtr<nsIContent> thisContent =
do_QueryInterface(static_cast<nsIImageLoadingContent*>(this));

// Use the safe JSContext here as we're not always able to find the
// JSContext associated with the NPP any more.
AutoSafeJSContext cx;
NS_ENSURE_TRUE_VOID(thisContent->GetWrapper());

// We don't init the AutoJSAPI with our wrapper because we don't want it
// reporting errors to our window's onerror listeners.
AutoJSAPI jsapi;
jsapi.Init();
JSContext* cx = jsapi.cx();
JS::Rooted<JSObject*> obj(cx, thisContent->GetWrapper());
NS_ENSURE_TRUE(obj, /* void */);
MOZ_ASSERT(obj);

JS::Rooted<JSObject*> proto(cx);
JSAutoCompartment ac(cx, obj);
Expand Down
8 changes: 4 additions & 4 deletions dom/base/nsXMLHttpRequest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2412,13 +2412,13 @@ GetRequestBody(nsIVariant* aBody, nsIInputStream** aResult, uint64_t* aContentLe
}

// ArrayBuffer?
AutoSafeJSContext cx;
JS::Rooted<JS::Value> realVal(cx);
JSContext* rootingCx = nsContentUtils::RootingCx();
JS::Rooted<JS::Value> realVal(rootingCx);

nsresult rv = aBody->GetAsJSVal(&realVal);
if (NS_SUCCEEDED(rv) && !realVal.isPrimitive()) {
JS::Rooted<JSObject*> obj(cx, realVal.toObjectOrNull());
ArrayBuffer buf;
JS::Rooted<JSObject*> obj(rootingCx, realVal.toObjectOrNull());
RootedTypedArray<ArrayBuffer> buf(rootingCx);
if (buf.Init(obj)) {
buf.ComputeLengthAndData();
return GetRequestBody(buf.Data(), buf.Length(), aResult,
Expand Down
6 changes: 5 additions & 1 deletion dom/bindings/CallbackObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,11 @@ CallbackObjectHolderBase::ToXPCOMCallback(CallbackObject* aCallback,
return nullptr;
}

AutoSafeJSContext cx;
// We don't init the AutoJSAPI with our callback because we don't want it
// reporting errors to its global's onerror handlers.
AutoJSAPI jsapi;
jsapi.Init();
JSContext* cx = jsapi.cx();

JS::Rooted<JSObject*> callback(cx, aCallback->Callback());

Expand Down
4 changes: 2 additions & 2 deletions dom/bluetooth/common/BluetoothReplyRunnable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include "BluetoothReplyRunnable.h"
#include "BluetoothUtils.h"
#include "DOMRequest.h"
#include "nsContentUtils.h"
#include "mozilla/dom/ScriptSettings.h"
#include "mozilla/dom/bluetooth/BluetoothTypes.h"
#include "mozilla/dom/Promise.h"
Expand Down Expand Up @@ -92,8 +93,7 @@ BluetoothReplyRunnable::Run()
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(mReply);

AutoSafeJSContext cx;
JS::Rooted<JS::Value> v(cx, JS::UndefinedValue());
JS::Rooted<JS::Value> v(nsContentUtils::RootingCx(), JS::UndefinedValue());

nsresult rv;
if (mReply->type() != BluetoothReply::TBluetoothReplySuccess) {
Expand Down
12 changes: 5 additions & 7 deletions dom/datastore/DataStoreDB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,9 @@ DataStoreDB::CreateFactoryIfNeeded()
nsIXPConnect* xpc = nsContentUtils::XPConnect();
MOZ_ASSERT(xpc);

AutoSafeJSContext cx;
AutoJSAPI jsapi;
jsapi.Init();
JSContext* cx = jsapi.cx();
JS::Rooted<JSObject*> global(cx);
rv = xpc->CreateSandbox(cx, principal, global.address());
if (NS_WARN_IF(NS_FAILED(rv))) {
Expand Down Expand Up @@ -222,10 +224,8 @@ DataStoreDB::UpgradeSchema(nsIDOMEvent* aEvent)
MOZ_ASSERT(version.Value() == DATASTOREDB_VERSION);
#endif

AutoSafeJSContext cx;

ErrorResult error;
JS::Rooted<JS::Value> result(cx);
JS::Rooted<JS::Value> result(nsContentUtils::RootingCx());
mRequest->GetResult(&result, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
Expand Down Expand Up @@ -284,10 +284,8 @@ DataStoreDB::DatabaseOpened()
{
MOZ_ASSERT(NS_IsMainThread());

AutoSafeJSContext cx;

ErrorResult error;
JS::Rooted<JS::Value> result(cx);
JS::Rooted<JS::Value> result(nsContentUtils::RootingCx());
mRequest->GetResult(&result, error);
if (NS_WARN_IF(error.Failed())) {
return error.StealNSResult();
Expand Down
7 changes: 5 additions & 2 deletions dom/media/MediaPermissionGonk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,12 @@ MediaPermissionRequest::Allow(JS::HandleValue aChoices)
return NS_ERROR_INVALID_ARG;
}
// iterate through audio-capture and video-capture
AutoSafeJSContext cx;
AutoJSAPI jsapi;
if (!jsapi.init(&aChoices.toObject())) {
return NS_ERROR_UNEXPECTED;
}
JSContext* cx = jsapi.cx();
JS::Rooted<JSObject*> obj(cx, &aChoices.toObject());
JSAutoCompartment ac(cx, obj);
JS::Rooted<JS::Value> v(cx);

// get selected audio device name
Expand Down
3 changes: 1 addition & 2 deletions dom/system/gonk/AudioManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1134,8 +1134,7 @@ AudioManager::MaybeUpdateVolumeSettingToDatabase(bool aForce)
}

// Send events to update the Gaia volumes
mozilla::AutoSafeJSContext cx;
JS::Rooted<JS::Value> value(cx);
JS::Rooted<JS::Value> value(nsContentUtils::RootingCx());
uint32_t volume = 0;
for (uint32_t idx = 0; idx < MOZ_ARRAY_LENGTH(gVolumeData); ++idx) {
int32_t streamType = gVolumeData[idx].mStreamType;
Expand Down
4 changes: 2 additions & 2 deletions dom/system/gonk/AutoMounterSetting.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ class SetStatusRunnable : public nsRunnable
settingsService->CreateLock(nullptr, getter_AddRefs(lock));
// lock may be null if this gets called during shutdown.
if (lock) {
mozilla::AutoSafeJSContext cx;
JS::Rooted<JS::Value> value(cx, JS::Int32Value(mStatus));
JS::Rooted<JS::Value> value(nsContentUtils::RootingCx(),
JS::Int32Value(mStatus));
lock->Set(UMS_STATUS, value, nullptr, nullptr);
}
return NS_OK;
Expand Down
10 changes: 4 additions & 6 deletions dom/system/gonk/SystemWorkerManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,15 +78,13 @@ SystemWorkerManager::Init()
NS_ASSERTION(NS_IsMainThread(), "We can only initialize on the main thread");
NS_ASSERTION(!mShutdown, "Already shutdown!");

mozilla::AutoSafeJSContext cx;

nsresult rv = InitWifi(cx);
nsresult rv = InitWifi();
if (NS_FAILED(rv)) {
NS_WARNING("Failed to initialize WiFi Networking!");
return rv;
}

InitKeyStore(cx);
InitKeyStore();

InitAutoMounter();
InitializeTimeZoneSettingObserver();
Expand Down Expand Up @@ -205,7 +203,7 @@ SystemWorkerManager::RegisterRilWorker(unsigned int aClientId,
}

nsresult
SystemWorkerManager::InitWifi(JSContext *cx)
SystemWorkerManager::InitWifi()
{
nsCOMPtr<nsIWorkerHolder> worker = do_CreateInstance(kWifiWorkerCID);
NS_ENSURE_TRUE(worker, NS_ERROR_FAILURE);
Expand All @@ -215,7 +213,7 @@ SystemWorkerManager::InitWifi(JSContext *cx)
}

nsresult
SystemWorkerManager::InitKeyStore(JSContext *cx)
SystemWorkerManager::InitKeyStore()
{
mKeyStore = new KeyStore();
return NS_OK;
Expand Down
4 changes: 2 additions & 2 deletions dom/system/gonk/SystemWorkerManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@ class SystemWorkerManager final : public nsIObserver,
SystemWorkerManager();
~SystemWorkerManager();

nsresult InitWifi(JSContext *cx);
nsresult InitKeyStore(JSContext *cx);
nsresult InitWifi();
nsresult InitKeyStore();

nsCOMPtr<nsIWorkerHolder> mWifiWorker;

Expand Down
1 change: 0 additions & 1 deletion dom/time/DateCacheCleaner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ class DateCacheCleaner : public SystemTimezoneChangeObserver
}
void Notify(const SystemTimezoneChangeInformation& aSystemTimezoneChangeInfo)
{
mozilla::AutoSafeJSContext cx;
JS::ResetTimeZone();
}

Expand Down
18 changes: 12 additions & 6 deletions dom/xbl/nsXBLPrototypeBinding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -881,9 +881,12 @@ nsXBLPrototypeBinding::Read(nsIObjectInputStream* aStream,
mInterfaceTable.Put(iid, mBinding);
}

AutoSafeJSContext cx;
JS::Rooted<JSObject*> compilationGlobal(cx, xpc::CompilationScope());
JSAutoCompartment ac(cx, compilationGlobal);
// We're not directly using this AutoJSAPI here, but callees use it via
// AutoJSContext.
AutoJSAPI jsapi;
if (!jsapi.Init(xpc::CompilationScope())) {
return NS_ERROR_UNEXPECTED;
}

bool isFirstBinding = aFlags & XBLBinding_Serialize_IsFirstBinding;
rv = Init(id, aDocInfo, nullptr, isFirstBinding);
Expand Down Expand Up @@ -1024,9 +1027,12 @@ nsXBLPrototypeBinding::Write(nsIObjectOutputStream* aStream)
// mKeyHandlersRegistered and mKeyHandlers are not serialized as they are
// computed on demand.

AutoSafeJSContext cx;
JS::Rooted<JSObject*> compilationGlobal(cx, xpc::CompilationScope());
JSAutoCompartment ac(cx, compilationGlobal);
// We're not directly using this AutoJSAPI here, but callees use it via
// AutoJSContext.
AutoJSAPI jsapi;
if (!jsapi.Init(xpc::CompilationScope())) {
return NS_ERROR_UNEXPECTED;
}

uint8_t flags = mInheritStyle ? XBLBinding_Serialize_InheritStyle : 0;

Expand Down
Loading

0 comments on commit 9bf80f0

Please sign in to comment.