Bug 1233812 - Fix possible race in accessing nsAppShell instance; r=snorp

When getting nsAppShell from another thread, there could be a race with
nsAppShell being destroyed on the main thread. This patch makes the raw
nsAppShell pointer only accessible from the main thread, and use a
static mutex to coordinate accessing nsAppShell from other threads.
This commit is contained in:
Jim Chen 2015-12-23 22:03:35 -05:00
parent 5326296b3f
commit 896c5bd072
7 changed files with 84 additions and 51 deletions

View File

@ -512,9 +512,9 @@ AndroidBridge::ShowAlertNotification(const nsAString& aImageUrl,
const nsAString& aAlertName,
nsIPrincipal* aPrincipal)
{
if (nsAppShell::gAppShell && aAlertListener) {
if (aAlertListener) {
// This will remove any observers already registered for this id
nsAppShell::gAppShell->PostEvent(AndroidGeckoEvent::MakeAddObserver(aAlertName, aAlertListener));
nsAppShell::PostEvent(AndroidGeckoEvent::MakeAddObserver(aAlertName, aAlertListener));
}
nsAutoString host;
@ -1706,15 +1706,17 @@ AndroidBridge::PumpMessageLoop()
NS_IMETHODIMP nsAndroidBridge::GetBrowserApp(nsIAndroidBrowserApp * *aBrowserApp)
{
if (nsAppShell::gAppShell)
nsAppShell::gAppShell->GetBrowserApp(aBrowserApp);
nsAppShell* const appShell = nsAppShell::Get();
if (appShell)
appShell->GetBrowserApp(aBrowserApp);
return NS_OK;
}
NS_IMETHODIMP nsAndroidBridge::SetBrowserApp(nsIAndroidBrowserApp *aBrowserApp)
{
if (nsAppShell::gAppShell)
nsAppShell::gAppShell->SetBrowserApp(aBrowserApp);
nsAppShell* const appShell = nsAppShell::Get();
if (appShell)
appShell->SetBrowserApp(aBrowserApp);
return NS_OK;
}

View File

@ -77,7 +77,7 @@ AndroidContentController::HandleSingleTap(const CSSPoint& aPoint,
CSSIntPoint rounded = RoundedToInt(point);
nsCString data = nsPrintfCString("{ \"x\": %d, \"y\": %d }", rounded.x, rounded.y);
nsAppShell::gAppShell->PostEvent(AndroidGeckoEvent::MakeBroadcastEvent(
nsAppShell::PostEvent(AndroidGeckoEvent::MakeBroadcastEvent(
NS_LITERAL_CSTRING("Gesture:SingleTap"), data));
}

View File

@ -61,8 +61,7 @@ NS_EXPORT void JNICALL
Java_org_mozilla_gecko_GeckoAppShell_notifyGeckoOfEvent(JNIEnv *jenv, jclass jc, jobject event)
{
// poke the appshell
if (nsAppShell::gAppShell)
nsAppShell::gAppShell->PostEvent(AndroidGeckoEvent::MakeFromJavaObject(jenv, event));
nsAppShell::PostEvent(AndroidGeckoEvent::MakeFromJavaObject(jenv, event));
}
NS_EXPORT void JNICALL
@ -342,7 +341,7 @@ Java_org_mozilla_gecko_gfx_NativePanZoomController_handleTouchEvent(JNIEnv* env,
MultiTouchInput input = wrapper->MakeMultiTouchInput(nsWindow::TopWindow());
delete wrapper;
if (input.mType < 0 || !nsAppShell::gAppShell) {
if (input.mType < 0) {
return false;
}
@ -350,7 +349,7 @@ Java_org_mozilla_gecko_gfx_NativePanZoomController_handleTouchEvent(JNIEnv* env,
uint64_t blockId;
nsEventStatus status = controller->ReceiveInputEvent(input, &guid, &blockId);
if (status != nsEventStatus_eConsumeNoDefault) {
nsAppShell::gAppShell->PostEvent(AndroidGeckoEvent::MakeApzInputEvent(input, guid, blockId, status));
nsAppShell::PostEvent(AndroidGeckoEvent::MakeApzInputEvent(input, guid, blockId, status));
}
return true;
}

View File

@ -26,7 +26,9 @@ struct PrefsHelper
bool observe)
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(nsAppShell::gAppShell);
nsAppShell* const appShell = nsAppShell::Get();
MOZ_ASSERT(appShell);
nsTArray<jni::Object::LocalRef> namesRefArray(prefNames.GetElements());
const size_t len = namesRefArray.Length();
@ -48,7 +50,7 @@ struct PrefsHelper
}
nsIAndroidBrowserApp* browserApp = nullptr;
nsAppShell::gAppShell->GetBrowserApp(&browserApp);
appShell->GetBrowserApp(&browserApp);
MOZ_ASSERT(browserApp);
if (observe) {
@ -63,10 +65,12 @@ struct PrefsHelper
static void RemovePrefsObserver(int32_t requestId)
{
MOZ_ASSERT(NS_IsMainThread());
MOZ_ASSERT(nsAppShell::gAppShell);
nsAppShell* const appShell = nsAppShell::Get();
MOZ_ASSERT(appShell);
nsIAndroidBrowserApp* browserApp = nullptr;
nsAppShell::gAppShell->GetBrowserApp(&browserApp);
appShell->GetBrowserApp(&browserApp);
MOZ_ASSERT(browserApp);
browserApp->RemovePreferenceObservers(requestId);

View File

@ -75,7 +75,8 @@ PRLogModuleInfo *gWidgetLog = nullptr;
nsIGeolocationUpdate *gLocationCallback = nullptr;
nsAutoPtr<mozilla::AndroidGeckoEvent> gLastSizeChange;
nsAppShell *nsAppShell::gAppShell = nullptr;
nsAppShell* nsAppShell::sAppShell;
StaticMutex nsAppShell::sAppShellLock;
NS_IMPL_ISUPPORTS_INHERITED(nsAppShell, nsBaseAppShell, nsIObserver)
@ -185,7 +186,10 @@ public:
nsAppShell::nsAppShell()
{
gAppShell = this;
{
StaticMutexAutoLock lock(sAppShellLock);
sAppShell = this;
}
if (!XRE_IsParentProcess()) {
return;
@ -217,7 +221,10 @@ nsAppShell::~nsAppShell()
NS_WARNING("Discarded event on shutdown");
}
gAppShell = nullptr;
{
StaticMutexAutoLock lock(sAppShellLock);
sAppShell = nullptr;
}
if (sPowerManagerService) {
sPowerManagerService->RemoveWakeLockListener(sWakeLockListener);
@ -398,7 +405,11 @@ public:
void
nsAppShell::PostEvent(AndroidGeckoEvent* event)
{
mEventQueue.Post(mozilla::MakeUnique<LegacyGeckoEvent>(event));
mozilla::StaticMutexAutoLock lock(sAppShellLock);
if (!sAppShell) {
return;
}
sAppShell->mEventQueue.Post(mozilla::MakeUnique<LegacyGeckoEvent>(event));
}
void
@ -410,7 +421,7 @@ nsAppShell::LegacyGeckoEvent::Run()
switch (curEvent->Type()) {
case AndroidGeckoEvent::NATIVE_POKE:
nsAppShell::gAppShell->NativeEventCallback();
nsAppShell::Get()->NativeEventCallback();
break;
case AndroidGeckoEvent::SENSOR_EVENT: {
@ -519,19 +530,19 @@ nsAppShell::LegacyGeckoEvent::Run()
}
case AndroidGeckoEvent::THUMBNAIL: {
if (!nsAppShell::gAppShell->mBrowserApp)
if (!nsAppShell::Get()->mBrowserApp)
break;
int32_t tabId = curEvent->MetaState();
const nsTArray<nsIntPoint>& points = curEvent->Points();
RefCountedJavaObject* buffer = curEvent->ByteBuffer();
RefPtr<ThumbnailRunnable> sr = new ThumbnailRunnable(nsAppShell::gAppShell->mBrowserApp, tabId, points, buffer);
RefPtr<ThumbnailRunnable> sr = new ThumbnailRunnable(nsAppShell::Get()->mBrowserApp, tabId, points, buffer);
MessageLoop::current()->PostIdleTask(FROM_HERE, NewRunnableMethod(sr.get(), &ThumbnailRunnable::Run));
break;
}
case AndroidGeckoEvent::ZOOMEDVIEW: {
if (!nsAppShell::gAppShell->mBrowserApp)
if (!nsAppShell::Get()->mBrowserApp)
break;
int32_t tabId = curEvent->MetaState();
const nsTArray<nsIntPoint>& points = curEvent->Points();
@ -541,7 +552,7 @@ nsAppShell::LegacyGeckoEvent::Run()
nsCOMPtr<nsIDOMWindow> domWindow;
nsCOMPtr<nsIBrowserTab> tab;
nsAppShell::gAppShell->mBrowserApp->GetBrowserTab(tabId, getter_AddRefs(tab));
nsAppShell::Get()->mBrowserApp->GetBrowserTab(tabId, getter_AddRefs(tab));
if (!tab) {
NS_ERROR("Can't find tab!");
break;
@ -577,7 +588,7 @@ nsAppShell::LegacyGeckoEvent::Run()
break;
nsCOMPtr<nsIUITelemetryObserver> obs;
nsAppShell::gAppShell->mBrowserApp->GetUITelemetryObserver(getter_AddRefs(obs));
nsAppShell::Get()->mBrowserApp->GetUITelemetryObserver(getter_AddRefs(obs));
if (!obs)
break;
@ -594,7 +605,7 @@ nsAppShell::LegacyGeckoEvent::Run()
break;
nsCOMPtr<nsIUITelemetryObserver> obs;
nsAppShell::gAppShell->mBrowserApp->GetUITelemetryObserver(getter_AddRefs(obs));
nsAppShell::Get()->mBrowserApp->GetUITelemetryObserver(getter_AddRefs(obs));
if (!obs)
break;
@ -610,7 +621,7 @@ nsAppShell::LegacyGeckoEvent::Run()
break;
nsCOMPtr<nsIUITelemetryObserver> obs;
nsAppShell::gAppShell->mBrowserApp->GetUITelemetryObserver(getter_AddRefs(obs));
nsAppShell::Get()->mBrowserApp->GetUITelemetryObserver(getter_AddRefs(obs));
if (!obs)
break;
@ -713,7 +724,7 @@ nsAppShell::LegacyGeckoEvent::Run()
case AndroidGeckoEvent::CALL_OBSERVER:
{
nsCOMPtr<nsIObserver> observer;
nsAppShell::gAppShell->mObserversHash.Get(curEvent->Characters(), getter_AddRefs(observer));
nsAppShell::Get()->mObserversHash.Get(curEvent->Characters(), getter_AddRefs(observer));
if (observer) {
observer->Observe(nullptr, NS_ConvertUTF16toUTF8(curEvent->CharactersExtra()).get(),
@ -726,11 +737,11 @@ nsAppShell::LegacyGeckoEvent::Run()
}
case AndroidGeckoEvent::REMOVE_OBSERVER:
nsAppShell::gAppShell->mObserversHash.Remove(curEvent->Characters());
nsAppShell::Get()->mObserversHash.Remove(curEvent->Characters());
break;
case AndroidGeckoEvent::ADD_OBSERVER:
nsAppShell::gAppShell->AddObserver(curEvent->Characters(), curEvent->Observer());
nsAppShell::Get()->AddObserver(curEvent->Characters(), curEvent->Observer());
break;
case AndroidGeckoEvent::LOW_MEMORY:
@ -850,7 +861,7 @@ nsAppShell::LegacyGeckoEvent::PostTo(mozilla::LinkedList<Event>& queue)
case AndroidGeckoEvent::MOTION_EVENT:
case AndroidGeckoEvent::APZ_INPUT_EVENT:
if (nsAppShell::gAppShell->mAllowCoalescingTouches) {
if (sAppShell->mAllowCoalescingTouches) {
Event* const event = queue.getLast();
if (event && event->HasSameTypeAs(this) && ae->CanCoalesceWith(
static_cast<LegacyGeckoEvent*>(event)->ae.get())) {
@ -891,18 +902,21 @@ namespace mozilla {
bool ProcessNextEvent()
{
if (!nsAppShell::gAppShell) {
nsAppShell* const appShell = nsAppShell::Get();
if (!appShell) {
return false;
}
return nsAppShell::gAppShell->ProcessNextNativeEvent(true) ? true : false;
return appShell->ProcessNextNativeEvent(true) ? true : false;
}
void NotifyEvent()
{
if (nsAppShell::gAppShell) {
nsAppShell::gAppShell->NotifyNativeEvent();
nsAppShell* const appShell = nsAppShell::Get();
if (!appShell) {
return;
}
appShell->NotifyNativeEvent();
}
}

View File

@ -10,6 +10,7 @@
#include "mozilla/LinkedList.h"
#include "mozilla/Monitor.h"
#include "mozilla/Move.h"
#include "mozilla/StaticMutex.h"
#include "mozilla/UniquePtr.h"
#include "mozilla/unused.h"
#include "mozilla/jni/Natives.h"
@ -69,7 +70,11 @@ public:
void Run() override { return lambda(); }
};
static nsAppShell *gAppShell;
static nsAppShell* Get()
{
MOZ_ASSERT(NS_IsMainThread());
return sAppShell;
}
nsAppShell();
@ -84,26 +89,32 @@ public:
// Post a subclass of Event.
// e.g. PostEvent(mozilla::MakeUnique<MyEvent>());
template<typename T, typename D>
void PostEvent(mozilla::UniquePtr<T, D>&& event)
static void PostEvent(mozilla::UniquePtr<T, D>&& event)
{
mEventQueue.Post(mozilla::Move(event));
mozilla::StaticMutexAutoLock lock(sAppShellLock);
if (!sAppShell) {
return;
}
sAppShell->mEventQueue.Post(mozilla::Move(event));
}
// Post a event that will call a lambda
// e.g. PostEvent([=] { /* do something */ });
template<typename T>
void PostEvent(T&& lambda)
static void PostEvent(T&& lambda)
{
mEventQueue.Post(mozilla::MakeUnique<LambdaEvent<T>>(
mozilla::StaticMutexAutoLock lock(sAppShellLock);
if (!sAppShell) {
return;
}
sAppShell->mEventQueue.Post(mozilla::MakeUnique<LambdaEvent<T>>(
mozilla::Move(lambda)));
}
void PostEvent(mozilla::AndroidGeckoEvent* event);
static void PostEvent(mozilla::AndroidGeckoEvent* event);
void ResendLastResizeEvent(nsWindow* aDest);
void OnResume() {}
void SetBrowserApp(nsIAndroidBrowserApp* aBrowserApp) {
mBrowserApp = aBrowserApp;
}
@ -113,6 +124,9 @@ public:
}
protected:
static nsAppShell* sAppShell;
static mozilla::StaticMutex sAppShellLock;
virtual ~nsAppShell();
nsresult AddObserver(const nsAString &aObserverKey, nsIObserver *aObserver);
@ -190,7 +204,7 @@ public:
template<class Functor>
static void OnNativeCall(Functor&& call)
{
nsAppShell::gAppShell->PostEvent(mozilla::Move(call));
nsAppShell::PostEvent(mozilla::Move(call));
}
};

View File

@ -230,7 +230,7 @@ public:
// can get a head start on opening our window.
return aCall();
}
return nsAppShell::gAppShell->PostEvent(mozilla::MakeUnique<
return nsAppShell::PostEvent(mozilla::MakeUnique<
WindowEvent<Functor>>(mozilla::Move(aCall)));
}
@ -432,7 +432,7 @@ class nsWindow::GLControllerSupport final
finished = true;
lock.NotifyAll();
};
nsAppShell::gAppShell->PostEvent(
nsAppShell::PostEvent(
mozilla::MakeUnique<GLControllerEvent>(
new nsAppShell::LambdaEvent<decltype(callAndNotify)>(
mozilla::Move(callAndNotify))));
@ -473,7 +473,7 @@ public:
return aCall();
}
nsAppShell::gAppShell->PostEvent(
nsAppShell::PostEvent(
mozilla::MakeUnique<GLControllerEvent>(
new GeckoViewSupport::WindowEvent<Functor>(
mozilla::Move(aCall))));
@ -491,7 +491,7 @@ public:
~GLControllerSupport()
{
GLController::GlobalRef glController(mozilla::Move(mGLController));
nsAppShell::gAppShell->PostEvent([glController] {
nsAppShell::PostEvent([glController] {
GLControllerSupport::DisposeNative(GLController::LocalRef(
jni::GetGeckoThreadEnv(), glController));
});
@ -1220,7 +1220,7 @@ nsWindow::BringToFront()
}
// force a window resize
nsAppShell::gAppShell->ResendLastResizeEvent(newTop);
nsAppShell::Get()->ResendLastResizeEvent(newTop);
RedrawAll();
}
@ -2289,7 +2289,7 @@ nsWindow::GeckoViewSupport::PostFlushIMEChanges(FlushChangesFlag aFlags)
// Keep a strong reference to the window to keep 'this' alive.
RefPtr<nsWindow> window(&this->window);
nsAppShell::gAppShell->PostEvent([this, window, aFlags] {
nsAppShell::PostEvent([this, window, aFlags] {
if (!window->Destroyed()) {
FlushIMEChanges(aFlags);
}
@ -2501,7 +2501,7 @@ nsWindow::GeckoViewSupport::SetInputContext(const InputContext& aContext,
RefPtr<nsWindow> window(&this->window);
mIMEUpdatingContext = true;
nsAppShell::gAppShell->PostEvent([this, window] {
nsAppShell::PostEvent([this, window] {
mIMEUpdatingContext = false;
if (window->Destroyed()) {
return;