diff --git a/dom/cache/Action.h b/dom/cache/Action.h index b9270ea6f8..da6b3fd499 100644 --- a/dom/cache/Action.h +++ b/dom/cache/Action.h @@ -11,6 +11,8 @@ #include "mozilla/dom/cache/Types.h" #include "nsISupportsImpl.h" +class mozIStorageConnection; + namespace mozilla { namespace dom { namespace cache { @@ -33,12 +35,27 @@ public: Release(void) = 0; }; + // Class containing data that can be opportunistically shared between + // multiple Actions running on the same thread/Context. In theory + // this could be abstracted to a generic key/value map, but for now + // just explicitly provide accessors for the data we need. + class Data + { + public: + virtual mozIStorageConnection* + GetConnection() const = 0; + + virtual void + SetConnection(mozIStorageConnection* aConn) = 0; + }; + // Execute operations on the target thread. Once complete call // Resolver::Resolve(). This can be done sync or async. // Note: Action should hold Resolver ref until its ready to call Resolve(). // Note: The "target" thread is determined when the Action is scheduled on // Context. The Action should not assume any particular thread is used. - virtual void RunOnTarget(Resolver* aResolver, const QuotaInfo& aQuotaInfo) = 0; + virtual void RunOnTarget(Resolver* aResolver, const QuotaInfo& aQuotaInfo, + Data* aOptionalData) = 0; // Called on initiating thread when the Action is canceled. The Action is // responsible for calling Resolver::Resolve() as normal; either with a diff --git a/dom/cache/Context.cpp b/dom/cache/Context.cpp index fdcc7f69ae..1d6697a19d 100644 --- a/dom/cache/Context.cpp +++ b/dom/cache/Context.cpp @@ -9,11 +9,13 @@ #include "mozilla/AutoRestore.h" #include "mozilla/DebugOnly.h" #include "mozilla/dom/cache/Action.h" +#include "mozilla/dom/cache/FileUtils.h" #include "mozilla/dom/cache/Manager.h" #include "mozilla/dom/cache/ManagerId.h" #include "mozilla/dom/cache/OfflineStorage.h" #include "mozilla/dom/quota/OriginOrPatternString.h" #include "mozilla/dom/quota/QuotaManager.h" +#include "mozIStorageConnection.h" #include "nsIFile.h" #include "nsIPrincipal.h" #include "nsIRunnable.h" @@ -22,6 +24,7 @@ namespace { using mozilla::dom::Nullable; +using mozilla::dom::cache::Action; using mozilla::dom::cache::QuotaInfo; using mozilla::dom::quota::Client; using mozilla::dom::quota::OriginOrPatternString; @@ -54,6 +57,22 @@ private: const QuotaInfo mQuotaInfo; }; +class NullAction final : public Action +{ +public: + NullAction() + { + } + + virtual void + RunOnTarget(Resolver* aResolver, const QuotaInfo&, Data*) override + { + // Resolve success immediately. This Action does no actual work. + MOZ_ASSERT(aResolver); + aResolver->Resolve(NS_OK); + } +}; + } // namespace namespace mozilla { @@ -66,6 +85,49 @@ using mozilla::dom::quota::QuotaManager; using mozilla::dom::quota::PERSISTENCE_TYPE_DEFAULT; using mozilla::dom::quota::PersistenceType; +class Context::Data final : public Action::Data +{ +public: + explicit Data(nsIThread* aTarget) + : mTarget(aTarget) + { + MOZ_ASSERT(mTarget); + } + + virtual mozIStorageConnection* + GetConnection() const override + { + MOZ_ASSERT(mTarget == NS_GetCurrentThread()); + return mConnection; + } + + virtual void + SetConnection(mozIStorageConnection* aConn) override + { + MOZ_ASSERT(mTarget == NS_GetCurrentThread()); + MOZ_ASSERT(!mConnection); + mConnection = aConn; + MOZ_ASSERT(mConnection); + } + +private: + ~Data() + { + // We could proxy release our data here, but instead just assert. The + // Context code should guarantee that we are destroyed on the target + // thread. If we're not, then QuotaManager might race and try to clear the + // origin out from under us. + MOZ_ASSERT(mTarget == NS_GetCurrentThread()); + } + + nsCOMPtr mTarget; + nsCOMPtr mConnection; + + // Threadsafe counting because we're created on the PBackground thread + // and destroyed on the target IO thread. + NS_INLINE_DECL_THREADSAFE_REFCOUNTING(Context::Data) +}; + // Executed to perform the complicated dance of steps necessary to initialize // the QuotaManager. This must be performed for each origin before any disk // IO occurrs. @@ -74,11 +136,15 @@ class Context::QuotaInitRunnable final : public nsIRunnable public: QuotaInitRunnable(Context* aContext, Manager* aManager, - Action* aQuotaIOThreadAction) + Data* aData, + nsIThread* aTarget, + Action* aInitAction) : mContext(aContext) , mThreadsafeHandle(aContext->CreateThreadsafeHandle()) , mManager(aManager) - , mQuotaIOThreadAction(aQuotaIOThreadAction) + , mData(aData) + , mTarget(aTarget) + , mInitAction(aInitAction) , mInitiatingThread(NS_GetCurrentThread()) , mResult(NS_OK) , mState(STATE_INIT) @@ -87,7 +153,10 @@ public: { MOZ_ASSERT(mContext); MOZ_ASSERT(mManager); + MOZ_ASSERT(mData); + MOZ_ASSERT(mTarget); MOZ_ASSERT(mInitiatingThread); + MOZ_ASSERT(mInitAction); } nsresult Dispatch() @@ -109,7 +178,7 @@ public: NS_ASSERT_OWNINGTHREAD(QuotaInitRunnable); MOZ_ASSERT(!mCanceled); mCanceled = true; - mQuotaIOThreadAction->CancelOnInitiatingThread(); + mInitAction->CancelOnInitiatingThread(); } private: @@ -145,7 +214,7 @@ private: { MOZ_ASSERT(mState == STATE_COMPLETE); MOZ_ASSERT(!mContext); - MOZ_ASSERT(!mQuotaIOThreadAction); + MOZ_ASSERT(!mInitAction); } enum State @@ -154,6 +223,7 @@ private: STATE_CALL_WAIT_FOR_OPEN_ALLOWED, STATE_WAIT_FOR_OPEN_ALLOWED, STATE_ENSURE_ORIGIN_INITIALIZED, + STATE_RUN_ON_TARGET, STATE_RUNNING, STATE_COMPLETING, STATE_COMPLETE @@ -165,13 +235,15 @@ private: MOZ_ASSERT(mContext); mContext = nullptr; mManager = nullptr; - mQuotaIOThreadAction = nullptr; + mInitAction = nullptr; } nsRefPtr mContext; nsRefPtr mThreadsafeHandle; nsRefPtr mManager; - nsRefPtr mQuotaIOThreadAction; + nsRefPtr mData; + nsCOMPtr mTarget; + nsRefPtr mInitAction; nsCOMPtr mInitiatingThread; nsresult mResult; QuotaInfo mQuotaInfo; @@ -209,9 +281,14 @@ NS_IMPL_ISUPPORTS(mozilla::dom::cache::Context::QuotaInitRunnable, nsIRunnable); // | (Quota IO Thread) +----------------+ // +----------+------------+ | // | | +// +----------v------------+ | +// | RunOnTarget | Resolve(error) | +// | (Target Thread) +----------------+ +// +----------+------------+ | +// | | // +---------v---------+ +------v------+ // | Running | | Completing | -// | (Quota IO Thread) +------------>(Orig Thread)| +// | (Target Thread) +------------>(Orig Thread)| // +-------------------+ +------+------+ // | // +-----v----+ @@ -234,7 +311,7 @@ Context::QuotaInitRunnable::Run() { MOZ_ASSERT(NS_IsMainThread()); - if (mCanceled) { + if (mCanceled || QuotaManager::IsShuttingDown()) { resolver->Resolve(NS_ERROR_ABORT); break; } @@ -327,27 +404,40 @@ Context::QuotaInitRunnable::Run() break; } - mState = STATE_RUNNING; + mState = STATE_RUN_ON_TARGET; - if (!mQuotaIOThreadAction) { - resolver->Resolve(NS_OK); - break; - } + MOZ_ALWAYS_TRUE(NS_SUCCEEDED( + mTarget->Dispatch(this, nsIThread::DISPATCH_NORMAL))); + break; + } + // ------------------- + case STATE_RUN_ON_TARGET: + { + MOZ_ASSERT(NS_GetCurrentThread() == mTarget); + + mState = STATE_RUNNING; // Execute the provided initialization Action. The Action must Resolve() // before returning. - mQuotaIOThreadAction->RunOnTarget(resolver, mQuotaInfo); + mInitAction->RunOnTarget(resolver, mQuotaInfo, mData); MOZ_ASSERT(resolver->Resolved()); + mData = nullptr; + + // If the database was opened, then we should always succeed when creating + // the marker file. If it wasn't opened successfully, then no need to + // create a marker file anyway. + if (NS_SUCCEEDED(resolver->Result())) { + MOZ_ALWAYS_TRUE(NS_SUCCEEDED(CreateMarkerFile(mQuotaInfo))); + } + break; } // ------------------- case STATE_COMPLETING: { NS_ASSERT_OWNINGTHREAD(QuotaInitRunnable); - if (mQuotaIOThreadAction) { - mQuotaIOThreadAction->CompleteOnInitiatingThread(mResult); - } + mInitAction->CompleteOnInitiatingThread(mResult); mContext->OnQuotaInit(mResult, mQuotaInfo, mOfflineStorage); mState = STATE_COMPLETE; @@ -391,9 +481,10 @@ class Context::ActionRunnable final : public nsIRunnable , public Context::Activity { public: - ActionRunnable(Context* aContext, nsIEventTarget* aTarget, Action* aAction, - const QuotaInfo& aQuotaInfo) + ActionRunnable(Context* aContext, Data* aData, nsIEventTarget* aTarget, + Action* aAction, const QuotaInfo& aQuotaInfo) : mContext(aContext) + , mData(aData) , mTarget(aTarget) , mAction(aAction) , mQuotaInfo(aQuotaInfo) @@ -403,9 +494,10 @@ public: , mExecutingRunOnTarget(false) { MOZ_ASSERT(mContext); + // mData may be nullptr MOZ_ASSERT(mTarget); MOZ_ASSERT(mAction); - MOZ_ASSERT(mQuotaInfo.mDir); + // mQuotaInfo.mDir may be nullptr if QuotaInitRunnable failed MOZ_ASSERT(mInitiatingThread); } @@ -492,6 +584,7 @@ private: }; nsRefPtr mContext; + nsRefPtr mData; nsCOMPtr mTarget; nsRefPtr mAction; const QuotaInfo mQuotaInfo; @@ -558,7 +651,9 @@ Context::ActionRunnable::Run() mExecutingRunOnTarget = true; mState = STATE_RUNNING; - mAction->RunOnTarget(this, mQuotaInfo); + mAction->RunOnTarget(this, mQuotaInfo, mData); + + mData = nullptr; // Resolve was called synchronously from RunOnTarget(). We can // immediately move to completing now since we are sure RunOnTarget() @@ -667,10 +762,22 @@ void Context::ThreadsafeHandle::AllowToCloseOnOwningThread() { MOZ_ASSERT(mOwningThread == NS_GetCurrentThread()); + // A Context "closes" when its ref count drops to zero. Dropping this // strong ref is necessary, but not sufficient for the close to occur. // Any outstanding IO will continue and keep the Context alive. Once // the Context is idle, it will be destroyed. + + // First, tell the context to flush any target thread shared data. This + // data must be released on the target thread prior to running the Context + // destructor. This will schedule an Action which ensures that the + // ~Context() is not immediately executed when we drop the strong ref. + if (mStrongRef) { + mStrongRef->DoomTargetData(); + } + + // Now drop our strong ref and let Context finish running any outstanding + // Actions. mStrongRef = nullptr; } @@ -701,36 +808,28 @@ Context::ThreadsafeHandle::ContextDestroyed(Context* aContext) // static already_AddRefed -Context::Create(Manager* aManager, Action* aQuotaIOThreadAction, - Context* aOldContext) +Context::Create(Manager* aManager, nsIThread* aTarget, + Action* aInitAction, Context* aOldContext) { - nsRefPtr context = new Context(aManager); - - // Do this here to avoid doing an AddRef() in the constructor - context->mInitRunnable = new QuotaInitRunnable(context, aManager, - aQuotaIOThreadAction); - - if (aOldContext) { - aOldContext->SetNextContext(context); - } else { - context->Start(); - } - + nsRefPtr context = new Context(aManager, aTarget); + context->Init(aInitAction, aOldContext); return context.forget(); } -Context::Context(Manager* aManager) +Context::Context(Manager* aManager, nsIThread* aTarget) : mManager(aManager) + , mTarget(aTarget) + , mData(new Data(aTarget)) , mState(STATE_CONTEXT_PREINIT) + , mOrphanedData(false) { MOZ_ASSERT(mManager); } void -Context::Dispatch(nsIEventTarget* aTarget, Action* aAction) +Context::Dispatch(Action* aAction) { NS_ASSERT_OWNINGTHREAD(Context); - MOZ_ASSERT(aTarget); MOZ_ASSERT(aAction); MOZ_ASSERT(mState != STATE_CONTEXT_CANCELED); @@ -739,13 +838,12 @@ Context::Dispatch(nsIEventTarget* aTarget, Action* aAction) } else if (mState == STATE_CONTEXT_INIT || mState == STATE_CONTEXT_PREINIT) { PendingAction* pending = mPendingActions.AppendElement(); - pending->mTarget = aTarget; pending->mAction = aAction; return; } MOZ_ASSERT(STATE_CONTEXT_READY); - DispatchAction(aTarget, aAction); + DispatchAction(aAction); } void @@ -826,18 +924,42 @@ Context::~Context() { NS_ASSERT_OWNINGTHREAD(Context); MOZ_ASSERT(mManager); + MOZ_ASSERT(!mData); if (mThreadsafeHandle) { mThreadsafeHandle->ContextDestroyed(this); } + // Note, this may set the mOrphanedData flag. mManager->RemoveContext(this); + if (mQuotaInfo.mDir && !mOrphanedData) { + MOZ_ALWAYS_TRUE(NS_SUCCEEDED(DeleteMarkerFile(mQuotaInfo))); + } + if (mNextContext) { mNextContext->Start(); } } +void +Context::Init(Action* aInitAction, Context* aOldContext) +{ + NS_ASSERT_OWNINGTHREAD(Context); + MOZ_ASSERT(!mInitRunnable); + + // Do this here to avoid doing an AddRef() in the constructor + mInitRunnable = new QuotaInitRunnable(this, mManager, mData, mTarget, + aInitAction); + + if (aOldContext) { + aOldContext->SetNextContext(this); + return; + } + + Start(); +} + void Context::Start() { @@ -863,12 +985,17 @@ Context::Start() } void -Context::DispatchAction(nsIEventTarget* aTarget, Action* aAction) +Context::DispatchAction(Action* aAction, bool aDoomData) { NS_ASSERT_OWNINGTHREAD(Context); nsRefPtr runnable = - new ActionRunnable(this, aTarget, aAction, mQuotaInfo); + new ActionRunnable(this, mData, mTarget, aAction, mQuotaInfo); + + if (aDoomData) { + mData = nullptr; + } + nsresult rv = runnable->Dispatch(); if (NS_FAILED(rv)) { // Shutdown must be delayed until all Contexts are destroyed. Crash @@ -908,7 +1035,7 @@ Context::OnQuotaInit(nsresult aRv, const QuotaInfo& aQuotaInfo, mState = STATE_CONTEXT_READY; for (uint32_t i = 0; i < mPendingActions.Length(); ++i) { - DispatchAction(mPendingActions[i].mTarget, mPendingActions[i].mAction); + DispatchAction(mPendingActions[i].mAction); } mPendingActions.Clear(); } @@ -931,6 +1058,14 @@ Context::RemoveActivity(Activity* aActivity) MOZ_ASSERT(!mActivityList.Contains(aActivity)); } +void +Context::NoteOrphanedData() +{ + NS_ASSERT_OWNINGTHREAD(Context); + // This may be called more than once + mOrphanedData = true; +} + already_AddRefed Context::CreateThreadsafeHandle() { @@ -951,6 +1086,26 @@ Context::SetNextContext(Context* aNextContext) mNextContext = aNextContext; } +void +Context::DoomTargetData() +{ + NS_ASSERT_OWNINGTHREAD(Context); + MOZ_ASSERT(mData); + + // We are about to drop our reference to the Data. We need to ensure that + // the ~Context() destructor does not run until contents of Data have been + // released on the Target thread. + + // Dispatch a no-op Action. This will hold the Context alive through a + // roundtrip to the target thread and back to the owning thread. The + // ref to the Data object is cleared on the owning thread after creating + // the ActionRunnable, but before dispatching it. + nsRefPtr action = new NullAction(); + DispatchAction(action, true /* doomed data */); + + MOZ_ASSERT(!mData); +} + } // namespace cache } // namespace dom } // namespace mozilla diff --git a/dom/cache/Context.h b/dom/cache/Context.h index b626468514..19856a7e16 100644 --- a/dom/cache/Context.h +++ b/dom/cache/Context.h @@ -111,13 +111,14 @@ public: // will run on the QuotaManager IO thread. Note, this Action must // be execute synchronously. static already_AddRefed - Create(Manager* aManager, Action* aQuotaIOThreadAction, Context* aOldContext); + Create(Manager* aManager, nsIThread* aTarget, + Action* aInitAction, Context* aOldContext); // Execute given action on the target once the quota manager has been // initialized. // // Only callable from the thread that created the Context. - void Dispatch(nsIEventTarget* aTarget, Action* aAction); + void Dispatch(Action* aAction); // Cancel any Actions running or waiting to run. This should allow the // Context to be released and Listener::RemoveContext() will be called @@ -151,7 +152,13 @@ public: return mQuotaInfo; } + // Tell the Context that some state information has been orphaned in the + // data store and won't be cleaned up. The Context will leave the marker + // in place to trigger cleanup the next times its opened. + void NoteOrphanedData(); + private: + class Data; class QuotaInitRunnable; class ActionRunnable; @@ -169,10 +176,11 @@ private: nsRefPtr mAction; }; - explicit Context(Manager* aManager); + Context(Manager* aManager, nsIThread* aTarget); ~Context(); + void Init(Action* aInitAction, Context* aOldContext); void Start(); - void DispatchAction(nsIEventTarget* aTarget, Action* aAction); + void DispatchAction(Action* aAction, bool aDoomData = false); void OnQuotaInit(nsresult aRv, const QuotaInfo& aQuotaInfo, nsMainThreadPtrHandle& aOfflineStorage); @@ -182,8 +190,14 @@ private: void SetNextContext(Context* aNextContext); + void + DoomTargetData(); + nsRefPtr mManager; + nsCOMPtr mTarget; + nsRefPtr mData; State mState; + bool mOrphanedData; QuotaInfo mQuotaInfo; nsRefPtr mInitRunnable; nsTArray mPendingActions; diff --git a/dom/cache/DBAction.cpp b/dom/cache/DBAction.cpp index 3bfedce06a..d5a6c5b1dc 100644 --- a/dom/cache/DBAction.cpp +++ b/dom/cache/DBAction.cpp @@ -35,7 +35,8 @@ DBAction::~DBAction() } void -DBAction::RunOnTarget(Resolver* aResolver, const QuotaInfo& aQuotaInfo) +DBAction::RunOnTarget(Resolver* aResolver, const QuotaInfo& aQuotaInfo, + Data* aOptionalData) { MOZ_ASSERT(!NS_IsMainThread()); MOZ_ASSERT(aResolver); @@ -60,12 +61,27 @@ DBAction::RunOnTarget(Resolver* aResolver, const QuotaInfo& aQuotaInfo) } nsCOMPtr conn; - rv = OpenConnection(aQuotaInfo, dbDir, getter_AddRefs(conn)); - if (NS_WARN_IF(NS_FAILED(rv))) { - aResolver->Resolve(rv); - return; + + // Attempt to reuse the connection opened by a previous Action. + if (aOptionalData) { + conn = aOptionalData->GetConnection(); + } + + // If there is no previous Action, then we must open one. + if (!conn) { + rv = OpenConnection(aQuotaInfo, dbDir, getter_AddRefs(conn)); + if (NS_WARN_IF(NS_FAILED(rv))) { + aResolver->Resolve(rv); + return; + } + MOZ_ASSERT(conn); + + // Save this connection in the shared Data object so later Actions can + // use it. This avoids opening a new connection for every Action. + if (aOptionalData) { + aOptionalData->SetConnection(conn); + } } - MOZ_ASSERT(conn); RunWithDBOnTarget(aResolver, aQuotaInfo, dbDir, conn); } @@ -153,6 +169,7 @@ DBAction::OpenConnection(const QuotaInfo& aQuotaInfo, nsIFile* aDBDir, if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } rv = ss->OpenDatabaseWithFileURL(dbFileUrl, getter_AddRefs(conn)); + if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } } rv = db::InitializeConnection(conn); diff --git a/dom/cache/DBAction.h b/dom/cache/DBAction.h index 93ba341c43..d907e29b27 100644 --- a/dom/cache/DBAction.h +++ b/dom/cache/DBAction.h @@ -43,7 +43,8 @@ protected: private: virtual void - RunOnTarget(Resolver* aResolver, const QuotaInfo& aQuotaInfo) override; + RunOnTarget(Resolver* aResolver, const QuotaInfo& aQuotaInfo, + Data* aOptionalData) override; nsresult OpenConnection(const QuotaInfo& aQuotaInfo, nsIFile* aQuotaDir, mozIStorageConnection** aConnOut); diff --git a/dom/cache/FileUtils.cpp b/dom/cache/FileUtils.cpp index 1648578966..dfec03e200 100644 --- a/dom/cache/FileUtils.cpp +++ b/dom/cache/FileUtils.cpp @@ -319,6 +319,60 @@ BodyIdToFile(nsIFile* aBaseDir, const nsID& aId, BodyFileType aType, } // namespace +nsresult +CreateMarkerFile(const QuotaInfo& aQuotaInfo) +{ + nsCOMPtr marker; + nsresult rv = aQuotaInfo.mDir->Clone(getter_AddRefs(marker)); + if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } + + rv = marker->Append(NS_LITERAL_STRING("cache")); + if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } + + rv = marker->Append(NS_LITERAL_STRING("context_open.marker")); + if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } + + rv = marker->Create(nsIFile::NORMAL_FILE_TYPE, 0644); + if (rv == NS_ERROR_FILE_ALREADY_EXISTS) { + rv = NS_OK; + } + + // Note, we don't need to fsync here. We only care about actually + // writing the marker if later modifications to the Cache are + // actually flushed to the disk. If the OS crashes before the marker + // is written then we are ensured no other changes to the Cache were + // flushed either. + + return rv; +} + +nsresult +DeleteMarkerFile(const QuotaInfo& aQuotaInfo) +{ + nsCOMPtr marker; + nsresult rv = aQuotaInfo.mDir->Clone(getter_AddRefs(marker)); + if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } + + rv = marker->Append(NS_LITERAL_STRING("cache")); + if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } + + rv = marker->Append(NS_LITERAL_STRING("context_open.marker")); + if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } + + rv = marker->Remove(/* recursive = */ false); + if (rv == NS_ERROR_FILE_NOT_FOUND || + rv == NS_ERROR_FILE_TARGET_DOES_NOT_EXIST) { + rv = NS_OK; + } + + // Again, no fsync is necessary. If the OS crashes before the file + // removal is flushed, then the Cache will search for stale data on + // startup. This will cause the next Cache access to be a bit slow, but + // it seems appropriate after an OS crash. + + return NS_OK; +} + } // namespace cache } // namespace dom } // namespace mozilla diff --git a/dom/cache/FileUtils.h b/dom/cache/FileUtils.h index f6d7ab6c3d..38af32d3fd 100644 --- a/dom/cache/FileUtils.h +++ b/dom/cache/FileUtils.h @@ -50,6 +50,12 @@ BodyOpen(const QuotaInfo& aQuotaInfo, nsIFile* aBaseDir, const nsID& aId, nsresult BodyDeleteFiles(nsIFile* aBaseDir, const nsTArray& aIdList); +nsresult +CreateMarkerFile(const QuotaInfo& aQuotaInfo); + +nsresult +DeleteMarkerFile(const QuotaInfo& aQuotaInfo); + } // namespace cache } // namespace dom } // namespace mozilla diff --git a/dom/cache/Manager.cpp b/dom/cache/Manager.cpp index f06e99a06c..cf2548a31b 100644 --- a/dom/cache/Manager.cpp +++ b/dom/cache/Manager.cpp @@ -93,7 +93,7 @@ public: } virtual void - RunOnTarget(Resolver* aResolver, const QuotaInfo& aQuotaInfo) override + RunOnTarget(Resolver* aResolver, const QuotaInfo& aQuotaInfo, Data*) override { MOZ_ASSERT(aResolver); MOZ_ASSERT(aQuotaInfo.mDir); @@ -1303,12 +1303,13 @@ public: // no outstanding references, delete immediately nsRefPtr context = mManager->mContext; - // TODO: note that we need to check this cache for staleness on startup (bug 1110446) - if (!context->IsCanceled()) { + if (context->IsCanceled()) { + context->NoteOrphanedData(); + } else { context->CancelForCacheId(mCacheId); nsRefPtr action = new DeleteOrphanedCacheAction(mManager, mCacheId); - context->Dispatch(mManager->mIOThread, action); + context->Dispatch(action); } } } @@ -1457,9 +1458,26 @@ Manager::RemoveContext(Context* aContext) // Whether the Context destruction was triggered from the Manager going // idle or the underlying storage being invalidated, we should know we - // are closing before the Conext is destroyed. + // are closing before the Context is destroyed. MOZ_ASSERT(mState == Closing); + // Before forgetting the Context, check to see if we have any outstanding + // cache or body objects waiting for deletion. If so, note that we've + // orphaned data so it will be cleaned up on the next open. + for (uint32_t i = 0; i < mCacheIdRefs.Length(); ++i) { + if (mCacheIdRefs[i].mOrphaned) { + aContext->NoteOrphanedData(); + break; + } + } + + for (uint32_t i = 0; i < mBodyIdRefs.Length(); ++i) { + if (mBodyIdRefs[i].mOrphaned) { + aContext->NoteOrphanedData(); + break; + } + } + mContext = nullptr; // Once the context is gone, we can immediately remove ourself from the @@ -1511,13 +1529,18 @@ Manager::ReleaseCacheId(CacheId aCacheId) if (mCacheIdRefs[i].mCount == 0) { bool orphaned = mCacheIdRefs[i].mOrphaned; mCacheIdRefs.RemoveElementAt(i); - // TODO: note that we need to check this cache for staleness on startup (bug 1110446) nsRefPtr context = mContext; - if (orphaned && context && !context->IsCanceled()) { - context->CancelForCacheId(aCacheId); - nsRefPtr action = new DeleteOrphanedCacheAction(this, - aCacheId); - context->Dispatch(mIOThread, action); + // If the context is already gone, then orphan flag should have been + // set in RemoveContext(). + if (orphaned && context) { + if (context->IsCanceled()) { + context->NoteOrphanedData(); + } else { + context->CancelForCacheId(aCacheId); + nsRefPtr action = new DeleteOrphanedCacheAction(this, + aCacheId); + context->Dispatch(action); + } } } MaybeAllowContextToClose(); @@ -1555,11 +1578,16 @@ Manager::ReleaseBodyId(const nsID& aBodyId) if (mBodyIdRefs[i].mCount < 1) { bool orphaned = mBodyIdRefs[i].mOrphaned; mBodyIdRefs.RemoveElementAt(i); - // TODO: note that we need to check this body for staleness on startup (bug 1110446) nsRefPtr context = mContext; - if (orphaned && context && !context->IsCanceled()) { - nsRefPtr action = new DeleteOrphanedBodyAction(aBodyId); - context->Dispatch(mIOThread, action); + // If the context is already gone, then orphan flag should have been + // set in RemoveContext(). + if (orphaned && context) { + if (context->IsCanceled()) { + context->NoteOrphanedData(); + } else { + nsRefPtr action = new DeleteOrphanedBodyAction(aBodyId); + context->Dispatch(action); + } } } MaybeAllowContextToClose(); @@ -1635,7 +1663,7 @@ Manager::ExecuteCacheOp(Listener* aListener, CacheId aCacheId, MOZ_CRASH("Unknown Cache operation!"); } - context->Dispatch(mIOThread, action); + context->Dispatch(action); } void @@ -1682,7 +1710,7 @@ Manager::ExecuteStorageOp(Listener* aListener, Namespace aNamespace, MOZ_CRASH("Unknown CacheStorage operation!"); } - context->Dispatch(mIOThread, action); + context->Dispatch(action); } void @@ -1708,7 +1736,7 @@ Manager::ExecutePutAll(Listener* aListener, CacheId aCacheId, aPutList, aRequestStreamList, aResponseStreamList); - context->Dispatch(mIOThread, action); + context->Dispatch(action); } Manager::Manager(ManagerId* aManagerId, nsIThread* aIOThread) @@ -1752,7 +1780,8 @@ Manager::Init(Manager* aOldManager) // per Manager now, this lets us cleanly call Factory::Remove() once the // Context goes away. nsRefPtr setupAction = new SetupAction(); - nsRefPtr ref = Context::Create(this, setupAction, oldContext); + nsRefPtr ref = Context::Create(this, mIOThread, setupAction, + oldContext); mContext = ref; } @@ -1870,7 +1899,7 @@ Manager::NoteOrphanedBodyIdList(const nsTArray& aDeletedBodyIdList) nsRefPtr context = mContext; if (!deleteNowList.IsEmpty() && context && !context->IsCanceled()) { nsRefPtr action = new DeleteOrphanedBodyAction(deleteNowList); - context->Dispatch(mIOThread, action); + context->Dispatch(action); } } diff --git a/dom/cache/QuotaClient.cpp b/dom/cache/QuotaClient.cpp index 702dfb646c..fcc6d0a740 100644 --- a/dom/cache/QuotaClient.cpp +++ b/dom/cache/QuotaClient.cpp @@ -110,7 +110,7 @@ public: InitOrigin(PersistenceType aPersistenceType, const nsACString& aGroup, const nsACString& aOrigin, UsageInfo* aUsageInfo) override { - return NS_OK; + return GetUsageForOrigin(aPersistenceType, aGroup, aOrigin, aUsageInfo); } virtual nsresult @@ -153,6 +153,7 @@ public: if (isDir) { if (leafName.EqualsLiteral("morgue")) { rv = GetBodyUsage(file, aUsageInfo); + if (NS_WARN_IF(NS_FAILED(rv))) { return rv; } } else { NS_WARNING("Unknown Cache directory found!"); } @@ -160,10 +161,11 @@ public: continue; } - // Ignore transient sqlite files + // Ignore transient sqlite files and marker files if (leafName.EqualsLiteral("caches.sqlite-journal") || leafName.EqualsLiteral("caches.sqlite-shm") || - leafName.Find(NS_LITERAL_CSTRING("caches.sqlite-mj"), false, 0, 0) == 0) { + leafName.Find(NS_LITERAL_CSTRING("caches.sqlite-mj"), false, 0, 0) == 0 || + leafName.EqualsLiteral("context_open.marker")) { continue; } diff --git a/dom/cache/test/mochitest/mochitest.ini b/dom/cache/test/mochitest/mochitest.ini index 60df62647a..07fa4f8229 100644 --- a/dom/cache/test/mochitest/mochitest.ini +++ b/dom/cache/test/mochitest/mochitest.ini @@ -19,6 +19,7 @@ support-files = test_cache_put.js test_cache_requestCache.js test_cache_delete.js + test_cache_put_reorder.js [test_cache.html] [test_cache_add.html] @@ -31,3 +32,4 @@ support-files = [test_cache_put.html] [test_cache_requestCache.html] [test_cache_delete.html] +[test_cache_put_reorder.html] diff --git a/dom/cache/test/mochitest/test_cache_put_reorder.html b/dom/cache/test/mochitest/test_cache_put_reorder.html new file mode 100644 index 0000000000..33934176cd --- /dev/null +++ b/dom/cache/test/mochitest/test_cache_put_reorder.html @@ -0,0 +1,20 @@ + + + + + Ensure that using Cache.put to overwrite an entry will change its insertion order + + + + + + + + + diff --git a/dom/cache/test/mochitest/test_cache_put_reorder.js b/dom/cache/test/mochitest/test_cache_put_reorder.js new file mode 100644 index 0000000000..df8beecf93 --- /dev/null +++ b/dom/cache/test/mochitest/test_cache_put_reorder.js @@ -0,0 +1,31 @@ +var name = "putreorder" + context; +var c; + +var reqs = [ + "//mochi.test:8888/?foo" + context, + "//mochi.test:8888/?bar" + context, + "//mochi.test:8888/?baz" + context, +]; + +caches.open(name).then(function(cache) { + c = cache; + return c.addAll(reqs); +}).then(function() { + return c.put(reqs[1], new Response("overwritten")); +}).then(function() { + return c.keys(); +}).then(function(keys) { + is(keys.length, 3, "Correct number of entries expected"); + ok(keys[0].url.indexOf(reqs[0]) >= 0, "The first entry should be untouched"); + ok(keys[2].url.indexOf(reqs[1]) >= 0, "The second entry should be moved to the end"); + ok(keys[1].url.indexOf(reqs[2]) >= 0, "The third entry should now be the second one"); + return c.match(reqs[1]); +}).then(function(r) { + return r.text(); +}).then(function(body) { + is(body, "overwritten", "The body should be overwritten"); + return caches.delete(name); +}).then(function(deleted) { + ok(deleted, "The cache should be deleted successfully"); + testDone(); +}); diff --git a/dom/cache/test/mochitest/test_cache_restart.html b/dom/cache/test/mochitest/test_cache_restart.html new file mode 100644 index 0000000000..f01e929d57 --- /dev/null +++ b/dom/cache/test/mochitest/test_cache_restart.html @@ -0,0 +1,61 @@ + + + + + Test Cache with QuotaManager Restart + + + + + + + diff --git a/dom/fetch/FetchDriver.cpp b/dom/fetch/FetchDriver.cpp index 707363e377..2043009803 100644 --- a/dom/fetch/FetchDriver.cpp +++ b/dom/fetch/FetchDriver.cpp @@ -12,6 +12,7 @@ #include "nsIHttpChannel.h" #include "nsIHttpChannelInternal.h" #include "nsIHttpHeaderVisitor.h" +#include "nsIJARChannel.h" #include "nsIScriptSecurityManager.h" #include "nsIThreadRetargetableRequest.h" #include "nsIUploadChannel2.h" @@ -291,9 +292,9 @@ FetchDriver::BasicFetch() return FailWithNetworkError(); } - if (scheme.LowerCaseEqualsLiteral("file")) { - } else if (scheme.LowerCaseEqualsLiteral("http") || - scheme.LowerCaseEqualsLiteral("https")) { + if (scheme.LowerCaseEqualsLiteral("http") || + scheme.LowerCaseEqualsLiteral("https") || + scheme.LowerCaseEqualsLiteral("app")) { return HttpFetch(); } @@ -484,8 +485,10 @@ FetchDriver::HttpFetch(bool aCORSFlag, bool aCORSPreflightFlag, bool aAuthentica // While the spec also gates on the client being a ServiceWorker, we can't // infer that here. Instead we rely on callers to set the flag correctly. if (mRequest->SkipServiceWorker()) { - nsCOMPtr internalChan = do_QueryInterface(httpChan); - internalChan->ForceNoIntercept(); + if (httpChan) { + nsCOMPtr internalChan = do_QueryInterface(httpChan); + internalChan->ForceNoIntercept(); + } } nsCOMPtr listener = this; @@ -523,7 +526,7 @@ FetchDriver::HttpFetch(bool aCORSFlag, bool aCORSPreflightFlag, bool aAuthentica unsafeHeaders, getter_AddRefs(preflightChannel)); } else { - rv = chan->AsyncOpen(listener, nullptr); + rv = chan->AsyncOpen(listener, nullptr); } if (NS_WARN_IF(NS_FAILED(rv))) { @@ -656,28 +659,31 @@ FetchDriver::OnStartRequest(nsIRequest* aRequest, return rv; } - nsCOMPtr channel = do_QueryInterface(aRequest); - // For now we only support HTTP. - MOZ_ASSERT(channel); + nsRefPtr response; + nsCOMPtr httpChannel = do_QueryInterface(aRequest); + if (httpChannel) { + uint32_t responseStatus; + httpChannel->GetResponseStatus(&responseStatus); - aRequest->GetStatus(&rv); - if (NS_WARN_IF(NS_FAILED(rv))) { - FailWithNetworkError(); - return rv; - } + nsAutoCString statusText; + httpChannel->GetResponseStatusText(statusText); - uint32_t responseStatus; - channel->GetResponseStatus(&responseStatus); + response = new InternalResponse(responseStatus, statusText); - nsAutoCString statusText; - channel->GetResponseStatusText(statusText); + nsRefPtr visitor = new FillResponseHeaders(response); + rv = httpChannel->VisitResponseHeaders(visitor); + if (NS_WARN_IF(NS_FAILED(rv))) { + NS_WARNING("Failed to visit all headers."); + } + } else { + nsCOMPtr jarChannel = do_QueryInterface(aRequest); + // If it is not an http channel, it has to be a jar one. + MOZ_ASSERT(jarChannel); - nsRefPtr response = new InternalResponse(responseStatus, statusText); - - nsRefPtr visitor = new FillResponseHeaders(response); - rv = channel->VisitResponseHeaders(visitor); - if (NS_WARN_IF(NS_FAILED(rv))) { - NS_WARNING("Failed to visit all headers."); + // We simulate the http protocol for jar/app requests + uint32_t responseStatus = 200; + nsAutoCString statusText; + response = new InternalResponse(responseStatus, NS_LITERAL_CSTRING("OK")); } // We open a pipe so that we can immediately set the pipe's read end as the @@ -699,6 +705,7 @@ FetchDriver::OnStartRequest(nsIRequest* aRequest, response->SetBody(pipeInputStream); nsCOMPtr securityInfo; + nsCOMPtr channel = do_QueryInterface(aRequest); rv = channel->GetSecurityInfo(getter_AddRefs(securityInfo)); if (securityInfo) { response->SetSecurityInfo(securityInfo); diff --git a/dom/interfaces/notification/nsINotificationStorage.idl b/dom/interfaces/notification/nsINotificationStorage.idl index b6fda42a8b..2362bfbd58 100644 --- a/dom/interfaces/notification/nsINotificationStorage.idl +++ b/dom/interfaces/notification/nsINotificationStorage.idl @@ -41,7 +41,7 @@ interface nsINotificationStorageCallback : nsISupports /** * Interface for notification persistence layer. */ -[scriptable, uuid(f5145be6-e34b-468b-84da-c8c4c1ad60fe)] +[scriptable, uuid(cac01fb0-c2eb-4252-b2f4-5b1fac933bd4)] interface nsINotificationStorage : nsISupports { @@ -94,6 +94,20 @@ interface nsINotificationStorage : nsISupports */ void delete(in DOMString origin, in DOMString id); + + /** + * Notifications are not supposed to be persistent, according to spec, at + * least for now. But we want to be able to have this behavior on B2G. Thus, + * this method will check if the origin sending the notifications is a valid + * registered app with a manifest or not. Hence, a webpage that has none + * will have its notification sent and available (via Notification.get()) + * during the life time of the page. + * + * @param origin: Origin from which the notification is sent. + * + * @return boolean + */ + boolean canPut(in DOMString origin); }; %{C++ diff --git a/dom/notification/NotificationDB.jsm b/dom/notification/NotificationDB.jsm index fa5d54db6f..09cf91a85a 100644 --- a/dom/notification/NotificationDB.jsm +++ b/dom/notification/NotificationDB.jsm @@ -24,6 +24,10 @@ XPCOMUtils.defineLazyServiceGetter(this, "ppmm", "@mozilla.org/parentprocessmessagemanager;1", "nsIMessageListenerManager"); +XPCOMUtils.defineLazyServiceGetter(this, "notificationStorage", + "@mozilla.org/notificationStorage;1", + "nsINotificationStorage"); + const NOTIFICATION_STORE_DIR = OS.Constants.Path.profileDir; const NOTIFICATION_STORE_PATH = OS.Path.join(NOTIFICATION_STORE_DIR, "notificationstore.json"); @@ -77,14 +81,29 @@ let NotificationDB = { } }, + filterNonAppNotifications: function(notifications) { + let origins = Object.keys(notifications); + for (let origin of origins) { + let canPut = notificationStorage.canPut(origin); + if (!canPut) { + if (DEBUG) debug("Origin " + origin + " is not linked to an app manifest, deleting."); + delete notifications[origin]; + } + } + return notifications; + }, + // Attempt to read notification file, if it's not there we will create it. load: function() { var promise = OS.File.read(NOTIFICATION_STORE_PATH, { encoding: "utf-8"}); return promise.then( function onSuccess(data) { if (data.length > 0) { - this.notifications = JSON.parse(data); + // Preprocessing phase intends to cleanly separate any migration-related + // tasks. + this.notifications = this.filterNonAppNotifications(JSON.parse(data)); } + // populate the list of notifications by tag if (this.notifications) { for (var origin in this.notifications) { @@ -97,6 +116,7 @@ let NotificationDB = { } } } + this.loaded = true; }.bind(this), diff --git a/dom/notification/NotificationStorage.js b/dom/notification/NotificationStorage.js index 6c3470d865..eec321f763 100644 --- a/dom/notification/NotificationStorage.js +++ b/dom/notification/NotificationStorage.js @@ -23,6 +23,10 @@ XPCOMUtils.defineLazyServiceGetter(this, "cpmm", "@mozilla.org/childprocessmessagemanager;1", "nsIMessageSender"); +XPCOMUtils.defineLazyServiceGetter(this, "appsService", + "@mozilla.org/AppsService;1", + "nsIAppsService"); + const kMessageNotificationGetAllOk = "Notification:GetAll:Return:OK"; const kMessageNotificationGetAllKo = "Notification:GetAll:Return:KO"; const kMessageNotificationSaveKo = "Notification:Save:Return:KO"; @@ -45,6 +49,7 @@ function NotificationStorage() { this._requestCount = 0; Services.obs.addObserver(this, "xpcom-shutdown", false); + // Register for message listeners. this.registerListeners(); } @@ -65,12 +70,19 @@ NotificationStorage.prototype = { observe: function(aSubject, aTopic, aData) { if (DEBUG) debug("Topic: " + aTopic); - if (aTopic == "xpcom-shutdown") { + if (aTopic === "xpcom-shutdown") { Services.obs.removeObserver(this, "xpcom-shutdown"); this.unregisterListeners(); } }, + canPut: function(aOrigin) { + if (DEBUG) debug("Querying appService for: " + aOrigin); + let rv = !!appsService.getAppByManifestURL(aOrigin); + if (DEBUG) debug("appService returned: " + rv); + return rv; + }, + put: function(origin, id, title, dir, lang, body, tag, icon, alertName, data, behavior) { if (DEBUG) { debug("PUT: " + id + ": " + title); } @@ -105,10 +117,12 @@ NotificationStorage.prototype = { this._byTag[origin][tag] = notification; }; - cpmm.sendAsyncMessage("Notification:Save", { - origin: origin, - notification: notification - }); + if (this.canPut(origin)) { + cpmm.sendAsyncMessage("Notification:Save", { + origin: origin, + notification: notification + }); + } }, get: function(origin, tag, callback) { diff --git a/dom/tests/mochitest/fetch/app-protocol/README.txt b/dom/tests/mochitest/fetch/app-protocol/README.txt new file mode 100644 index 0000000000..7f5d392e1e --- /dev/null +++ b/dom/tests/mochitest/fetch/app-protocol/README.txt @@ -0,0 +1,2 @@ +application.zip contains foo.txt index.html and manifest.webapp. +Any change to one of these three files should be added to application.zip as well. diff --git a/dom/tests/mochitest/fetch/app-protocol/application.zip b/dom/tests/mochitest/fetch/app-protocol/application.zip new file mode 100644 index 0000000000..e11c80dabe Binary files /dev/null and b/dom/tests/mochitest/fetch/app-protocol/application.zip differ diff --git a/dom/tests/mochitest/fetch/app-protocol/foo.txt b/dom/tests/mochitest/fetch/app-protocol/foo.txt new file mode 100644 index 0000000000..2711cd6b37 --- /dev/null +++ b/dom/tests/mochitest/fetch/app-protocol/foo.txt @@ -0,0 +1 @@ +english sentence \ No newline at end of file diff --git a/dom/tests/mochitest/fetch/app-protocol/index.html b/dom/tests/mochitest/fetch/app-protocol/index.html new file mode 100644 index 0000000000..5bbea12623 --- /dev/null +++ b/dom/tests/mochitest/fetch/app-protocol/index.html @@ -0,0 +1,51 @@ + + + + Test app for bug 1161288 + + + + + diff --git a/dom/tests/mochitest/fetch/app-protocol/manifest.webapp b/dom/tests/mochitest/fetch/app-protocol/manifest.webapp new file mode 100644 index 0000000000..36a623585f --- /dev/null +++ b/dom/tests/mochitest/fetch/app-protocol/manifest.webapp @@ -0,0 +1,5 @@ +{ + "name": "App", + "launch_path": "/index.html", + "description": "Test app for bug 1161288" +} diff --git a/dom/tests/mochitest/fetch/app-protocol/update.webapp b/dom/tests/mochitest/fetch/app-protocol/update.webapp new file mode 100644 index 0000000000..dde6c30713 --- /dev/null +++ b/dom/tests/mochitest/fetch/app-protocol/update.webapp @@ -0,0 +1,6 @@ +{ + "name": "App", + "launch_path": "/index.html", + "description": "Test app for bug 1161288", + "package_path": "application.zip" +} diff --git a/dom/tests/mochitest/fetch/app-protocol/update.webapp^headers^ b/dom/tests/mochitest/fetch/app-protocol/update.webapp^headers^ new file mode 100644 index 0000000000..90818c6398 --- /dev/null +++ b/dom/tests/mochitest/fetch/app-protocol/update.webapp^headers^ @@ -0,0 +1 @@ +Content-Type: application/manifest+json diff --git a/dom/tests/mochitest/fetch/mochitest.ini b/dom/tests/mochitest/fetch/mochitest.ini index b0a1fda215..0831a800bf 100644 --- a/dom/tests/mochitest/fetch/mochitest.ini +++ b/dom/tests/mochitest/fetch/mochitest.ini @@ -1,5 +1,6 @@ [DEFAULT] support-files = + app-protocol/* fetch_test_framework.js test_fetch_basic.js test_fetch_basic_http.js @@ -13,6 +14,7 @@ support-files = [test_headers.html] [test_headers_mainthread.html] +[test_fetch_app_protocol.html] [test_fetch_basic.html] [test_fetch_basic_http.html] [test_fetch_cors.html] diff --git a/dom/tests/mochitest/fetch/test_fetch_app_protocol.html b/dom/tests/mochitest/fetch/test_fetch_app_protocol.html new file mode 100644 index 0000000000..39f7e01868 --- /dev/null +++ b/dom/tests/mochitest/fetch/test_fetch_app_protocol.html @@ -0,0 +1,126 @@ + + + + + Bug 1161288 - Support app:// origins on Fetch API + + + + +

+ +

+
+
+ + diff --git a/dom/tests/mochitest/notification/NotificationTest.js b/dom/tests/mochitest/notification/NotificationTest.js index 4d684efa3b..046460b989 100644 --- a/dom/tests/mochitest/notification/NotificationTest.js +++ b/dom/tests/mochitest/notification/NotificationTest.js @@ -44,6 +44,19 @@ var NotificationTest = (function () { })(tests); } + function fakeApp(aManifest) { + var aApp = { + "origin": "{mochitest}", + "manifestURL": aManifest + }; + + SpecialPowers.injectApp("{mochitest}", aApp); + } + + function unfakeApp() { + SpecialPowers.rejectApp("{mochitest}"); + } + var fakeCustomData = (function () { var buffer = new ArrayBuffer(2); var dv = new DataView(buffer).setInt16(0, 42, true); @@ -107,6 +120,10 @@ var NotificationTest = (function () { info: info, + fakeApp: fakeApp, + + unfakeApp: unfakeApp, + customDataMatches: function(dataObj) { var url = "http://www.domain.com"; try { diff --git a/dom/tests/mochitest/notification/mochitest.ini b/dom/tests/mochitest/notification/mochitest.ini index 5971458aba..abb5f6b0ab 100644 --- a/dom/tests/mochitest/notification/mochitest.ini +++ b/dom/tests/mochitest/notification/mochitest.ini @@ -8,3 +8,6 @@ support-files = [test_notification_storage.html] [test_bug931307.html] [test_notification_resend.html] +skip-if = e10s # On e10s, faking the app seems to be failing +[test_notification_noresend.html] +skip-if = (toolkit == 'gonk') # Mochitest on Gonk registers an app manifest that messes with the logic diff --git a/dom/tests/mochitest/notification/test_notification_noresend.html b/dom/tests/mochitest/notification/test_notification_noresend.html new file mode 100644 index 0000000000..7bd372db2a --- /dev/null +++ b/dom/tests/mochitest/notification/test_notification_noresend.html @@ -0,0 +1,88 @@ + + + + Testing mozResendAllNotifications() resend behavior for Pages + + + + + + +Bug 1159128 +

+ +

+
+
+
diff --git a/dom/tests/mochitest/notification/test_notification_resend.html b/dom/tests/mochitest/notification/test_notification_resend.html
index 1e2d49f964..da5a84559d 100644
--- a/dom/tests/mochitest/notification/test_notification_resend.html
+++ b/dom/tests/mochitest/notification/test_notification_resend.html
@@ -1,7 +1,7 @@
 
 
 
-  Testing mozResendAllNotifications() resend behavior
+  Testing mozResendAllNotifications() resend behavior for Apps
   
   
   
@@ -21,17 +21,6 @@
   SimpleTest.requestFlakyTimeout("untriaged");
 
   var steps = [
-    function (done) {
-      if (window.Notification) {
-        SpecialPowers.pushPrefEnv({"set": [
-          ["dom.ignore_webidl_scope_checks", true],
-          ]}, done);
-      } else {
-        ok(true, "Notifications are not enabled on the platform.");
-        done();
-      }
-    },
-
     function (done) {
       info("Set manifestURL");
       var request = window.navigator.mozApps.getSelf();
@@ -46,6 +35,18 @@
       };
     },
 
+    function (done) {
+      if (window.Notification) {
+        NotificationTest.fakeApp(manifestURL);
+        SpecialPowers.pushPrefEnv({"set": [
+          ["dom.ignore_webidl_scope_checks", true],
+          ]}, done);
+      } else {
+        ok(true, "Notifications are not enabled on the platform.");
+        done();
+      }
+    },
+
     function (done) {
       info("Test that we have mozChromeNotifications API");
       ok(('mozChromeNotifications' in navigator), "should have mozChromeNotifications API");
@@ -194,11 +195,12 @@
 
         notif2.close();
       });
-    }
+    },
   ];
 
   MockServices.register();
   NotificationTest.run(steps, function () {
+    NotificationTest.unfakeApp();
     MockServices.unregister();
   });
 
diff --git a/dom/workers/ServiceWorkerManager.cpp b/dom/workers/ServiceWorkerManager.cpp
index 64b99cac29..f96875b4dd 100644
--- a/dom/workers/ServiceWorkerManager.cpp
+++ b/dom/workers/ServiceWorkerManager.cpp
@@ -388,12 +388,20 @@ ServiceWorkerManager::~ServiceWorkerManager()
 {
   // The map will assert if it is not empty when destroyed.
   mRegistrationInfos.Clear();
+
   MOZ_ASSERT(!mActor);
 }
 
 void
 ServiceWorkerManager::Init()
 {
+  nsCOMPtr obs = mozilla::services::GetObserverService();
+  if (obs) {
+    DebugOnly rv;
+    rv = obs->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false /* ownsWeak */);
+    MOZ_ASSERT(NS_SUCCEEDED(rv));
+  }
+
   if (XRE_GetProcessType() == GeckoProcessType_Default) {
     nsRefPtr swr = ServiceWorkerRegistrar::Get();
     MOZ_ASSERT(swr);
@@ -402,11 +410,8 @@ ServiceWorkerManager::Init()
     swr->GetRegistrations(data);
     LoadRegistrations(data);
 
-    nsCOMPtr obs = mozilla::services::GetObserverService();
     if (obs) {
       DebugOnly rv;
-      rv = obs->AddObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID, false /* ownsWeak */);
-      MOZ_ASSERT(NS_SUCCEEDED(rv));
       rv = obs->AddObserver(this, PURGE_SESSION_HISTORY, false /* ownsWeak */);
       MOZ_ASSERT(NS_SUCCEEDED(rv));
       rv = obs->AddObserver(this, PURGE_DOMAIN_DATA, false /* ownsWeak */);
@@ -2319,7 +2324,7 @@ private:
 
     nsRefPtr swm = ServiceWorkerManager::GetInstance();
 
-    // We could be shutting down.
+    // Could it be that we are shutting down.
     if (swm->mActor) {
       swm->mActor->SendUnregister(principalInfo, NS_ConvertUTF8toUTF16(mScope));
     }
@@ -4414,21 +4419,22 @@ ServiceWorkerManager::Observe(nsISupports* aSubject,
                               const char* aTopic,
                               const char16_t* aData)
 {
-  MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
-
   if (strcmp(aTopic, PURGE_SESSION_HISTORY) == 0) {
+    MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
     RemoveAll();
     PropagateRemoveAll();
     return NS_OK;
   }
 
   if (strcmp(aTopic, PURGE_DOMAIN_DATA) == 0) {
+    MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
     nsAutoString domain(aData);
     RemoveAndPropagate(NS_ConvertUTF16toUTF8(domain));
     return NS_OK;
   }
 
   if (strcmp(aTopic, WEBAPPS_CLEAR_DATA) == 0) {
+    MOZ_ASSERT(XRE_GetProcessType() == GeckoProcessType_Default);
     nsCOMPtr params =
       do_QueryInterface(aSubject);
     if (NS_WARN_IF(!params)) {
@@ -4458,15 +4464,21 @@ ServiceWorkerManager::Observe(nsISupports* aSubject,
     }
 
     RemoveAllRegistrations(principal);
-  } else if (strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) {
+    return NS_OK;
+  }
+
+  if (strcmp(aTopic, NS_XPCOM_SHUTDOWN_OBSERVER_ID) == 0) {
     mShuttingDown = true;
 
     nsCOMPtr obs = mozilla::services::GetObserverService();
     if (obs) {
       obs->RemoveObserver(this, NS_XPCOM_SHUTDOWN_OBSERVER_ID);
-      obs->RemoveObserver(this, PURGE_SESSION_HISTORY);
-      obs->RemoveObserver(this, PURGE_DOMAIN_DATA);
-      obs->RemoveObserver(this, WEBAPPS_CLEAR_DATA);
+
+      if (XRE_GetProcessType() == GeckoProcessType_Default) {
+        obs->RemoveObserver(this, PURGE_SESSION_HISTORY);
+        obs->RemoveObserver(this, PURGE_DOMAIN_DATA);
+        obs->RemoveObserver(this, WEBAPPS_CLEAR_DATA);
+      }
     }
 
     if (mActor) {
@@ -4477,10 +4489,10 @@ ServiceWorkerManager::Observe(nsISupports* aSubject,
       unused << NS_WARN_IF(NS_FAILED(rv));
       mActor = nullptr;
     }
-  } else {
-    MOZ_CRASH("Received message we aren't supposed to be registered for!");
+    return NS_OK;
   }
 
+  MOZ_CRASH("Received message we aren't supposed to be registered for!");
   return NS_OK;
 }
 
diff --git a/testing/marionette/marionette-server.js b/testing/marionette/marionette-server.js
index 9249f1ccfd..837752e678 100644
--- a/testing/marionette/marionette-server.js
+++ b/testing/marionette/marionette-server.js
@@ -617,7 +617,7 @@ MarionetteServerConnection.prototype = {
 
     this.scriptTimeout = 10000;
     if (aRequest && aRequest.parameters) {
-      this.sessionId = aRequest.parameters.session_id ? aRequest.parameters.session_id : null;
+      this.sessionId = aRequest.parameters.sessionId || aRequest.parameters.session_id || null;
       logger.info("Session Id is set to: " + this.sessionId);
       try {
         this.setSessionCapabilities(aRequest.parameters.capabilities);
diff --git a/testing/specialpowers/content/SpecialPowersObserverAPI.js b/testing/specialpowers/content/SpecialPowersObserverAPI.js
index 3ef20b541d..12536e44b5 100644
--- a/testing/specialpowers/content/SpecialPowersObserverAPI.js
+++ b/testing/specialpowers/content/SpecialPowersObserverAPI.js
@@ -376,6 +376,33 @@ SpecialPowersObserverAPI.prototype = {
               scope.UserCustomizations._debug = aMessage.json.value;
               return;
             }
+          case "inject-app":
+	    {
+              let aAppId = aMessage.json.appId;
+	      let aApp   = aMessage.json.app;
+
+              let keys = Object.keys(Webapps.DOMApplicationRegistry.webapps);
+	      let exists = keys.indexOf(aAppId) !== -1;
+	      if (exists) {
+                return false;
+	      }
+
+              Webapps.DOMApplicationRegistry.webapps[aAppId] = aApp;
+	      return true;
+	    }
+	  case "reject-app":
+	    {
+              let aAppId = aMessage.json.appId;
+
+              let keys = Object.keys(Webapps.DOMApplicationRegistry.webapps);
+	      let exists = keys.indexOf(aAppId) !== -1;
+	      if (!exists) {
+                return false;
+	      }
+
+              delete Webapps.DOMApplicationRegistry.webapps[aAppId];
+	      return true;
+	    }
           default:
             throw new SpecialPowersException("Invalid operation for SPWebAppsService");
         }
diff --git a/testing/specialpowers/content/specialpowersAPI.js b/testing/specialpowers/content/specialpowersAPI.js
index 9f0d4d54f6..b000c190de 100644
--- a/testing/specialpowers/content/specialpowersAPI.js
+++ b/testing/specialpowers/content/specialpowersAPI.js
@@ -1135,6 +1135,23 @@ SpecialPowersAPI.prototype = {
     });
   },
 
+  // Force-registering an app in the registry
+  injectApp: function(aAppId, aApp) {
+    this._sendSyncMessage("SPWebAppService", {
+      op: "inject-app",
+      appId: aAppId,
+      app: aApp
+    });
+  },
+
+  // Removing app from the registry
+  rejectApp: function(aAppId) {
+    this._sendSyncMessage("SPWebAppService", {
+      op: "reject-app",
+      appId: aAppId
+    });
+  },
+
   _proxiedObservers: {
     "specialpowers-http-notify-request": function(aMessage) {
       let uri = aMessage.json.uri;