From ac5f22042311051696cdd751d690104bdb6fc895 Mon Sep 17 00:00:00 2001 From: Ben Kelly Date: Fri, 1 May 2015 08:15:52 -0700 Subject: [PATCH] Bug 1160227 Improve Cache API warnings. r=ehsan --- dom/cache/Cache.cpp | 53 ++++++++++++++++---------------- dom/cache/CacheOpChild.cpp | 2 +- dom/cache/CacheOpParent.cpp | 2 +- dom/cache/CacheStorage.cpp | 24 +++++++-------- dom/cache/CacheStorageParent.cpp | 2 +- dom/cache/Manager.cpp | 6 ++-- 6 files changed, 45 insertions(+), 44 deletions(-) diff --git a/dom/cache/Cache.cpp b/dom/cache/Cache.cpp index 113cca2bb16..64c3d2c8b13 100644 --- a/dom/cache/Cache.cpp +++ b/dom/cache/Cache.cpp @@ -229,7 +229,7 @@ already_AddRefed Cache::Match(const RequestOrUSVString& aRequest, const CacheQueryOptions& aOptions, ErrorResult& aRv) { - if (!mActor) { + if (NS_WARN_IF(!mActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } @@ -256,7 +256,7 @@ already_AddRefed Cache::MatchAll(const Optional& aRequest, const CacheQueryOptions& aOptions, ErrorResult& aRv) { - if (!mActor) { + if (NS_WARN_IF(!mActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } @@ -286,7 +286,7 @@ already_AddRefed Cache::Add(JSContext* aContext, const RequestOrUSVString& aRequest, ErrorResult& aRv) { - if (!mActor) { + if (NS_WARN_IF(!mActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } @@ -301,13 +301,13 @@ Cache::Add(JSContext* aContext, const RequestOrUSVString& aRequest, nsTArray> requestList(1); nsRefPtr request = Request::Constructor(global, aRequest, RequestInit(), aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } nsAutoString url; request->GetUrl(url); - if (!IsValidPutRequestURL(url, aRv)) { + if (NS_WARN_IF(!IsValidPutRequestURL(url, aRv))) { return nullptr; } @@ -320,7 +320,7 @@ Cache::AddAll(JSContext* aContext, const Sequence& aRequestList, ErrorResult& aRv) { - if (!mActor) { + if (NS_WARN_IF(!mActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } @@ -334,7 +334,8 @@ Cache::AddAll(JSContext* aContext, if (aRequestList[i].IsRequest()) { requestOrString.SetAsRequest() = aRequestList[i].GetAsRequest(); - if (!IsValidPutRequestMethod(requestOrString.GetAsRequest(), aRv)) { + if (NS_WARN_IF(!IsValidPutRequestMethod(requestOrString.GetAsRequest(), + aRv))) { return nullptr; } } else { @@ -345,13 +346,13 @@ Cache::AddAll(JSContext* aContext, nsRefPtr request = Request::Constructor(global, requestOrString, RequestInit(), aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } nsAutoString url; request->GetUrl(url); - if (!IsValidPutRequestURL(url, aRv)) { + if (NS_WARN_IF(!IsValidPutRequestURL(url, aRv))) { return nullptr; } @@ -365,17 +366,17 @@ already_AddRefed Cache::Put(const RequestOrUSVString& aRequest, Response& aResponse, ErrorResult& aRv) { - if (!mActor) { + if (NS_WARN_IF(!mActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } - if (!IsValidPutRequestMethod(aRequest, aRv)) { + if (NS_WARN_IF(!IsValidPutRequestMethod(aRequest, aRv))) { return nullptr; } nsRefPtr ir = ToInternalRequest(aRequest, ReadBody, aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } @@ -383,7 +384,7 @@ Cache::Put(const RequestOrUSVString& aRequest, Response& aResponse, args.Add(ir, ReadBody, TypeErrorOnInvalidScheme, aResponse, aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } @@ -394,13 +395,13 @@ already_AddRefed Cache::Delete(const RequestOrUSVString& aRequest, const CacheQueryOptions& aOptions, ErrorResult& aRv) { - if (!mActor) { + if (NS_WARN_IF(!mActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } nsRefPtr ir = ToInternalRequest(aRequest, IgnoreBody, aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } @@ -410,7 +411,7 @@ Cache::Delete(const RequestOrUSVString& aRequest, AutoChildOpArgs args(this, CacheDeleteArgs(CacheRequest(), params)); args.Add(ir, IgnoreBody, IgnoreInvalidScheme, aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } @@ -421,7 +422,7 @@ already_AddRefed Cache::Keys(const Optional& aRequest, const CacheQueryOptions& aOptions, ErrorResult& aRv) { - if (!mActor) { + if (NS_WARN_IF(!mActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } @@ -434,12 +435,12 @@ Cache::Keys(const Optional& aRequest, if (aRequest.WasPassed()) { nsRefPtr ir = ToInternalRequest(aRequest.Value(), IgnoreBody, aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } args.Add(ir, IgnoreBody, IgnoreInvalidScheme, aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } } @@ -529,7 +530,7 @@ already_AddRefed Cache::ExecuteOp(AutoChildOpArgs& aOpArgs, ErrorResult& aRv) { nsRefPtr promise = Promise::Create(mGlobal, aRv); - if (!promise) { + if (NS_WARN_IF(!promise)) { return nullptr; } @@ -546,7 +547,7 @@ Cache::AddAll(const GlobalObject& aGlobal, // If there is no work to do, then resolve immediately if (aRequestList.IsEmpty()) { nsRefPtr promise = Promise::Create(mGlobal, aRv); - if (!promise) { + if (NS_WARN_IF(!promise)) { return nullptr; } @@ -566,7 +567,7 @@ Cache::AddAll(const GlobalObject& aGlobal, requestOrString.SetAsRequest() = aRequestList[i]; nsRefPtr fetch = FetchRequest(mGlobal, requestOrString, RequestInit(), aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } @@ -574,7 +575,7 @@ Cache::AddAll(const GlobalObject& aGlobal, } nsRefPtr promise = Promise::Create(mGlobal, aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } @@ -582,7 +583,7 @@ Cache::AddAll(const GlobalObject& aGlobal, Move(aRequestList), promise); nsRefPtr fetchPromise = Promise::All(aGlobal, fetchList, aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } fetchPromise->AppendNativeHandler(handler); @@ -597,7 +598,7 @@ Cache::PutAll(const nsTArray>& aRequestList, { MOZ_ASSERT(aRequestList.Length() == aResponseList.Length()); - if (!mActor) { + if (NS_WARN_IF(!mActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } @@ -607,7 +608,7 @@ Cache::PutAll(const nsTArray>& aRequestList, for (uint32_t i = 0; i < aRequestList.Length(); ++i) { nsRefPtr ir = aRequestList[i]->GetInternalRequest(); args.Add(ir, ReadBody, TypeErrorOnInvalidScheme, *aResponseList[i], aRv); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { return nullptr; } } diff --git a/dom/cache/CacheOpChild.cpp b/dom/cache/CacheOpChild.cpp index f1a107c45fb..e14a29ad58e 100644 --- a/dom/cache/CacheOpChild.cpp +++ b/dom/cache/CacheOpChild.cpp @@ -97,7 +97,7 @@ CacheOpChild::Recv__delete__(const ErrorResult& aRv, { NS_ASSERT_OWNINGTHREAD(CacheOpChild); - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { MOZ_ASSERT(aResult.type() == CacheOpResult::Tvoid_t); // TODO: Remove this const_cast (bug 1152078). // It is safe for now since this ErrorResult is handed off to us by IPDL diff --git a/dom/cache/CacheOpParent.cpp b/dom/cache/CacheOpParent.cpp index c7e2edc9100..085079daa51 100644 --- a/dom/cache/CacheOpParent.cpp +++ b/dom/cache/CacheOpParent.cpp @@ -164,7 +164,7 @@ CacheOpParent::OnOpComplete(ErrorResult&& aRv, const CacheOpResult& aResult, // Never send an op-specific result if we have an error. Instead, send // void_t() to ensure that we don't leak actors on the child side. - if (aRv.Failed()) { + if (NS_WARN_IF(aRv.Failed())) { unused << Send__delete__(this, aRv, void_t()); aRv.SuppressException(); // We serialiazed it, as best we could. return; diff --git a/dom/cache/CacheStorage.cpp b/dom/cache/CacheStorage.cpp index 06881f19822..a999e8e9a3f 100644 --- a/dom/cache/CacheStorage.cpp +++ b/dom/cache/CacheStorage.cpp @@ -166,7 +166,7 @@ CacheStorage::CacheStorage(Namespace aNamespace, nsIGlobalObject* aGlobal, // wait for the async ActorCreated() callback. MOZ_ASSERT(NS_IsMainThread()); bool ok = BackgroundChild::GetOrCreateForCurrentThread(this); - if (!ok) { + if (NS_WARN_IF(!ok)) { ActorFailed(); } } @@ -177,7 +177,7 @@ CacheStorage::Match(const RequestOrUSVString& aRequest, { NS_ASSERT_OWNINGTHREAD(CacheStorage); - if (mFailedActor) { + if (NS_WARN_IF(mFailedActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } @@ -189,7 +189,7 @@ CacheStorage::Match(const RequestOrUSVString& aRequest, } nsRefPtr promise = Promise::Create(mGlobal, aRv); - if (!promise) { + if (NS_WARN_IF(!promise)) { return nullptr; } @@ -212,13 +212,13 @@ CacheStorage::Has(const nsAString& aKey, ErrorResult& aRv) { NS_ASSERT_OWNINGTHREAD(CacheStorage); - if (mFailedActor) { + if (NS_WARN_IF(mFailedActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } nsRefPtr promise = Promise::Create(mGlobal, aRv); - if (!promise) { + if (NS_WARN_IF(!promise)) { return nullptr; } @@ -237,13 +237,13 @@ CacheStorage::Open(const nsAString& aKey, ErrorResult& aRv) { NS_ASSERT_OWNINGTHREAD(CacheStorage); - if (mFailedActor) { + if (NS_WARN_IF(mFailedActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } nsRefPtr promise = Promise::Create(mGlobal, aRv); - if (!promise) { + if (NS_WARN_IF(!promise)) { return nullptr; } @@ -262,13 +262,13 @@ CacheStorage::Delete(const nsAString& aKey, ErrorResult& aRv) { NS_ASSERT_OWNINGTHREAD(CacheStorage); - if (mFailedActor) { + if (NS_WARN_IF(mFailedActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } nsRefPtr promise = Promise::Create(mGlobal, aRv); - if (!promise) { + if (NS_WARN_IF(!promise)) { return nullptr; } @@ -287,13 +287,13 @@ CacheStorage::Keys(ErrorResult& aRv) { NS_ASSERT_OWNINGTHREAD(CacheStorage); - if (mFailedActor) { + if (NS_WARN_IF(mFailedActor)) { aRv.Throw(NS_ERROR_UNEXPECTED); return nullptr; } nsRefPtr promise = Promise::Create(mGlobal, aRv); - if (!promise) { + if (NS_WARN_IF(!promise)) { return nullptr; } @@ -434,7 +434,7 @@ CacheStorage::MaybeRunPendingRequests() if (entry->mRequest) { args.Add(entry->mRequest, IgnoreBody, IgnoreInvalidScheme, rv); } - if (rv.Failed()) { + if (NS_WARN_IF(rv.Failed())) { entry->mPromise->MaybeReject(rv); continue; } diff --git a/dom/cache/CacheStorageParent.cpp b/dom/cache/CacheStorageParent.cpp index 9c85e9c8047..b4b83d1e366 100644 --- a/dom/cache/CacheStorageParent.cpp +++ b/dom/cache/CacheStorageParent.cpp @@ -100,7 +100,7 @@ CacheStorageParent::RecvPCacheOpConstructor(PCacheOpParent* aActor, return true; } - if (NS_FAILED(mVerifiedStatus)) { + if (NS_WARN_IF(NS_FAILED(mVerifiedStatus))) { unused << CacheOpParent::Send__delete__(actor, ErrorResult(mVerifiedStatus), void_t()); return true; diff --git a/dom/cache/Manager.cpp b/dom/cache/Manager.cpp index 6cddb043aba..f9be99e93d0 100644 --- a/dom/cache/Manager.cpp +++ b/dom/cache/Manager.cpp @@ -1623,7 +1623,7 @@ Manager::ExecuteCacheOp(Listener* aListener, CacheId aCacheId, MOZ_ASSERT(aListener); MOZ_ASSERT(aOpArgs.type() != CacheOpArgs::TCachePutAllArgs); - if (mState == Closing) { + if (NS_WARN_IF(mState == Closing)) { aListener->OnOpComplete(ErrorResult(NS_ERROR_FAILURE), void_t()); return; } @@ -1667,7 +1667,7 @@ Manager::ExecuteStorageOp(Listener* aListener, Namespace aNamespace, NS_ASSERT_OWNINGTHREAD(Manager); MOZ_ASSERT(aListener); - if (mState == Closing) { + if (NS_WARN_IF(mState == Closing)) { aListener->OnOpComplete(ErrorResult(NS_ERROR_FAILURE), void_t()); return; } @@ -1716,7 +1716,7 @@ Manager::ExecutePutAll(Listener* aListener, CacheId aCacheId, NS_ASSERT_OWNINGTHREAD(Manager); MOZ_ASSERT(aListener); - if (mState == Closing) { + if (NS_WARN_IF(mState == Closing)) { aListener->OnOpComplete(ErrorResult(NS_ERROR_FAILURE), CachePutAllResult()); return; }