Backed out changeset a6f8452cbd4b (bug 1367850) for Android mochitest crashes with EnqueueTask, e.g. bugs 1376668, 1376669, 1376670, test_ext_background_generated_url.html. r=backout a=backout
authorSebastian Hengst <archaeopteryx@coole-files.de>
Thu, 29 Jun 2017 03:19:50 +0200
changeset 366497 9af23c413a1f8d337b19b4f8450e241e91b71136
parent 366496 217b7fcf58944f927118b465769faeb1e613130a
child 366498 61b71b3d8354994a136c785a25a205433cadcf1c
child 366644 1898c946f69c0d3accd7e70f93bc5a0a3269b083
push id91995
push userarchaeopteryx@coole-files.de
push dateThu, 29 Jun 2017 01:22:33 +0000
treeherdermozilla-inbound@61b71b3d8354 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbackout, backout
bugs1367850, 1376668, 1376669, 1376670
milestone56.0a1
backs outa6f8452cbd4b2b57a431dfcdc8f0b82461b40f2e
first release with
nightly linux32
9af23c413a1f / 56.0a1 / 20170629100321 / files
nightly linux64
9af23c413a1f / 56.0a1 / 20170629100321 / files
nightly mac
9af23c413a1f / 56.0a1 / 20170629100331 / files
nightly win32
9af23c413a1f / 56.0a1 / 20170629030206 / files
nightly win64
9af23c413a1f / 56.0a1 / 20170629030206 / files
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
releases
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Backed out changeset a6f8452cbd4b (bug 1367850) for Android mochitest crashes with EnqueueTask, e.g. bugs 1376668, 1376669, 1376670, test_ext_background_generated_url.html. r=backout a=backout MozReview-Commit-ID: HiaJZ8N9J8O
widget/android/AndroidBridge.cpp
widget/android/AndroidBridge.h
widget/android/AndroidUiThread.cpp
widget/android/AndroidUiThread.h
widget/android/nsAppShell.cpp
widget/android/nsWindow.cpp
--- a/widget/android/AndroidBridge.cpp
+++ b/widget/android/AndroidBridge.cpp
@@ -148,16 +148,17 @@ AndroidBridge::DeconstructBridge()
     }
 }
 
 AndroidBridge::~AndroidBridge()
 {
 }
 
 AndroidBridge::AndroidBridge()
+  : mUiTaskQueueLock("UiTaskQueue")
 {
     ALOG_BRIDGE("AndroidBridge::Init");
 
     JNIEnv* const jEnv = jni::GetGeckoThreadEnv();
     AutoLocalJNIFrame jniFrame(jEnv);
 
     mMessageQueue = java::GeckoThread::MsgQueue();
     auto msgQueueClass = Class::LocalRef::Adopt(
@@ -941,16 +942,119 @@ AndroidBridge::IsContentDocumentDisplaye
 {
     auto layerClient = GetJavaLayerClient(aWindow);
     if (!layerClient) {
         return false;
     }
     return layerClient->IsContentDocumentDisplayed();
 }
 
+class AndroidBridge::DelayedTask
+{
+    using TimeStamp = mozilla::TimeStamp;
+    using TimeDuration = mozilla::TimeDuration;
+
+public:
+    DelayedTask(already_AddRefed<nsIRunnable> aTask)
+        : mTask(aTask)
+        , mRunTime() // Null timestamp representing no delay.
+    {}
+
+    DelayedTask(already_AddRefed<nsIRunnable> aTask, int aDelayMs)
+        : mTask(aTask)
+        , mRunTime(TimeStamp::Now() + TimeDuration::FromMilliseconds(aDelayMs))
+    {}
+
+    bool IsEarlierThan(const DelayedTask& aOther) const
+    {
+        if (mRunTime) {
+            return aOther.mRunTime ? mRunTime < aOther.mRunTime : false;
+        }
+        // In the case of no delay, we're earlier if aOther has a delay.
+        // Otherwise, we're not earlier, to maintain task order.
+        return !!aOther.mRunTime;
+    }
+
+    int64_t MillisecondsToRunTime() const
+    {
+        if (mRunTime) {
+            return int64_t((mRunTime - TimeStamp::Now()).ToMilliseconds());
+        }
+        return 0;
+    }
+
+    already_AddRefed<nsIRunnable> TakeTask()
+    {
+        return mTask.forget();
+    }
+
+private:
+    nsCOMPtr<nsIRunnable> mTask;
+    const TimeStamp mRunTime;
+};
+
+
+void
+AndroidBridge::PostTaskToUiThread(already_AddRefed<nsIRunnable> aTask, int aDelayMs)
+{
+    // add the new task into the mUiTaskQueue, sorted with
+    // the earliest task first in the queue
+    size_t i;
+    DelayedTask newTask(aDelayMs ? DelayedTask(mozilla::Move(aTask), aDelayMs)
+                                 : DelayedTask(mozilla::Move(aTask)));
+
+    {
+        MutexAutoLock lock(mUiTaskQueueLock);
+
+        for (i = 0; i < mUiTaskQueue.Length(); i++) {
+            if (newTask.IsEarlierThan(mUiTaskQueue[i])) {
+                mUiTaskQueue.InsertElementAt(i, mozilla::Move(newTask));
+                break;
+            }
+        }
+
+        if (i == mUiTaskQueue.Length()) {
+            // We didn't insert the task, which means we should append it.
+            mUiTaskQueue.AppendElement(mozilla::Move(newTask));
+        }
+    }
+
+    if (i == 0) {
+        // if we're inserting it at the head of the queue, notify Java because
+        // we need to get a callback at an earlier time than the last scheduled
+        // callback
+        GeckoThread::RequestUiThreadCallback(int64_t(aDelayMs));
+    }
+}
+
+int64_t
+AndroidBridge::RunDelayedUiThreadTasks()
+{
+    MutexAutoLock lock(mUiTaskQueueLock);
+
+    while (!mUiTaskQueue.IsEmpty()) {
+        const int64_t timeLeft = mUiTaskQueue[0].MillisecondsToRunTime();
+        if (timeLeft > 0) {
+            // this task (and therefore all remaining tasks)
+            // have not yet reached their runtime. return the
+            // time left until we should be called again
+            return timeLeft;
+        }
+
+        // Retrieve task before unlocking/running.
+        nsCOMPtr<nsIRunnable> nextTask(mUiTaskQueue[0].TakeTask());
+        mUiTaskQueue.RemoveElementAt(0);
+
+        // Unlock to allow posting new tasks reentrantly.
+        MutexAutoUnlock unlock(mUiTaskQueueLock);
+        nextTask->Run();
+    }
+    return -1;
+}
+
 Object::LocalRef AndroidBridge::ChannelCreate(Object::Param stream) {
     JNIEnv* const env = GetEnvForThread();
     auto rv = Object::LocalRef::Adopt(env, env->CallStaticObjectMethod(
             sBridge->jChannels, sBridge->jChannelCreate, stream.Get()));
     MOZ_CATCH_JNI_EXCEPTION(env);
     return rv;
 }
 
--- a/widget/android/AndroidBridge.h
+++ b/widget/android/AndroidBridge.h
@@ -219,16 +219,25 @@ protected:
     jmethodID jCalculateLength;
 
     // some convinient types to have around
     jclass jStringClass;
 
     jni::Object::GlobalRef mMessageQueue;
     jfieldID mMessageQueueMessages;
     jmethodID mMessageQueueNext;
+
+private:
+    class DelayedTask;
+    nsTArray<DelayedTask> mUiTaskQueue;
+    mozilla::Mutex mUiTaskQueueLock;
+
+public:
+    void PostTaskToUiThread(already_AddRefed<nsIRunnable> aTask, int aDelayMs);
+    int64_t RunDelayedUiThreadTasks();
 };
 
 class AutoJNIClass {
 private:
     JNIEnv* const mEnv;
     const jclass mClass;
 
 public:
--- a/widget/android/AndroidUiThread.cpp
+++ b/widget/android/AndroidUiThread.cpp
@@ -1,47 +1,39 @@
 /* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+#include "AndroidBridge.h"
 #include "base/message_loop.h"
-#include "GeneratedJNIWrappers.h"
 #include "mozilla/Atomics.h"
-#include "mozilla/LinkedList.h"
 #include "mozilla/Monitor.h"
-#include "mozilla/Mutex.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/StaticPtr.h"
-#include "mozilla/TimeStamp.h"
 #include "nsThread.h"
 #include "nsThreadManager.h"
 #include "nsThreadUtils.h"
 
 using namespace mozilla;
 
 namespace {
 
 class AndroidUiThread;
-class AndroidUiTask;
 
-StaticAutoPtr<LinkedList<AndroidUiTask> > sTaskQueue;
-StaticAutoPtr<mozilla::Mutex> sTaskQueueLock;
 StaticRefPtr<AndroidUiThread> sThread;
 static bool sThreadDestroyed;
 static MessageLoop* sMessageLoop;
 static Atomic<Monitor*> sMessageLoopAccessMonitor;
 
-void EnqueueTask(already_AddRefed<nsIRunnable> aTask, int aDelayMs);
-
 /*
  * The AndroidUiThread is derived from nsThread so that nsIRunnable objects that get
  * dispatched may be intercepted. Only nsIRunnable objects that need to be synchronously
  * executed are passed into the nsThread to be queued. All other nsIRunnable object
- * are immediately dispatched to the Android UI thread.
+ * are immediately dispatched to the Android UI thread via the AndroidBridge.
  * AndroidUiThread is derived from nsThread instead of being an nsIEventTarget
  * wrapper that contains an nsThread object because if nsIRunnable objects with a
  * delay were dispatch directly to an nsThread object, such as obtained from
  * nsThreadManager::GetCurrentThread(), the nsIRunnable could get stuck in the
  * nsThread nsIRunnable queue. This is due to the fact that Android controls the
  * event loop in the Android UI thread and has no knowledge of when the nsThread
  * needs to be drained.
 */
@@ -51,38 +43,46 @@ class AndroidUiThread : public nsThread
 public:
   NS_DECL_ISUPPORTS_INHERITED
   AndroidUiThread() : nsThread(nsThread::NOT_MAIN_THREAD, 0)
   {}
 
   nsresult Dispatch(already_AddRefed<nsIRunnable> aEvent, uint32_t aFlags) override;
   nsresult DelayedDispatch(already_AddRefed<nsIRunnable> aEvent, uint32_t aDelayMs) override;
 
+  static int64_t RunDelayedTasksIfValid() {
+    if (!AndroidBridge::Bridge() ||
+        sThreadDestroyed) {
+      return -1;
+    }
+    return AndroidBridge::Bridge()->RunDelayedUiThreadTasks();
+  }
+
 private:
   ~AndroidUiThread()
   {}
 };
 
 NS_IMPL_ISUPPORTS_INHERITED0(AndroidUiThread, nsThread)
 
 NS_IMETHODIMP
 AndroidUiThread::Dispatch(already_AddRefed<nsIRunnable> aEvent, uint32_t aFlags)
 {
   if (aFlags & NS_DISPATCH_SYNC) {
     return nsThread::Dispatch(Move(aEvent), aFlags);
   } else {
-    EnqueueTask(Move(aEvent), 0);
+    AndroidBridge::Bridge()->PostTaskToUiThread(Move(aEvent), 0);
     return NS_OK;
   }
 }
 
 NS_IMETHODIMP
 AndroidUiThread::DelayedDispatch(already_AddRefed<nsIRunnable> aEvent, uint32_t aDelayMs)
 {
-  EnqueueTask(Move(aEvent), aDelayMs);
+  AndroidBridge::Bridge()->PostTaskToUiThread(Move(aEvent), aDelayMs);
   return NS_OK;
 }
 
 static void
 PumpEvents() {
   NS_ProcessPendingEvents(sThread.get());
 }
 
@@ -100,75 +100,32 @@ private:
   {}
 };
 
 NS_IMPL_ISUPPORTS(ThreadObserver, nsIThreadObserver)
 
 NS_IMETHODIMP
 ThreadObserver::OnDispatchedEvent(nsIThreadInternal *thread)
 {
-  EnqueueTask(NS_NewRunnableFunction("PumpEvents", &PumpEvents), 0);
+  AndroidBridge::Bridge()->PostTaskToUiThread(NS_NewRunnableFunction("PumpEvents", &PumpEvents), 0);
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ThreadObserver::OnProcessNextEvent(nsIThreadInternal *thread, bool mayWait)
 {
   return NS_OK;
 }
 
 NS_IMETHODIMP
 ThreadObserver::AfterProcessNextEvent(nsIThreadInternal *thread, bool eventWasProcessed)
 {
   return NS_OK;
 }
 
-class AndroidUiTask : public LinkedListElement<AndroidUiTask> {
-    using TimeStamp = mozilla::TimeStamp;
-    using TimeDuration = mozilla::TimeDuration;
-
-public:
-  AndroidUiTask(already_AddRefed<nsIRunnable> aTask)
-    : mTask(aTask)
-    , mRunTime() // Null timestamp representing no delay.
-  {}
-
-  AndroidUiTask(already_AddRefed<nsIRunnable> aTask, int aDelayMs)
-    : mTask(aTask)
-    , mRunTime(TimeStamp::Now() + TimeDuration::FromMilliseconds(aDelayMs))
-  {}
-
-  bool IsEarlierThan(const AndroidUiTask& aOther) const
-  {
-    if (mRunTime) {
-      return aOther.mRunTime ? mRunTime < aOther.mRunTime : false;
-    }
-    // In the case of no delay, we're earlier if aOther has a delay.
-    // Otherwise, we're not earlier, to maintain task order.
-    return !!aOther.mRunTime;
-  }
-
-  int64_t MillisecondsToRunTime() const
-  {
-    if (mRunTime) {
-      return int64_t((mRunTime - TimeStamp::Now()).ToMilliseconds());
-    }
-    return 0;
-  }
-
-  already_AddRefed<nsIRunnable> TakeTask()
-  {
-      return mTask.forget();
-  }
-
-private:
-  nsCOMPtr<nsIRunnable> mTask;
-  const TimeStamp mRunTime;
-};
-
 class CreateOnUiThread : public Runnable {
 public:
   CreateOnUiThread() : Runnable("CreateOnUiThread")
   {}
 
   NS_IMETHOD Run() override {
     MOZ_ASSERT(!sThreadDestroyed);
     MOZ_ASSERT(sMessageLoopAccessMonitor);
@@ -185,34 +142,25 @@ public:
 class DestroyOnUiThread : public Runnable {
 public:
   DestroyOnUiThread() : Runnable("DestroyOnUiThread"), mDestroyed(false)
   {}
 
   NS_IMETHOD Run() override {
     MOZ_ASSERT(!sThreadDestroyed);
     MOZ_ASSERT(sMessageLoopAccessMonitor);
-    MOZ_ASSERT(sTaskQueue);
     MonitorAutoLock lock(*sMessageLoopAccessMonitor);
-    sThreadDestroyed = true;
-
-    {
-      // Flush the queue
-      MutexAutoLock lock (*sTaskQueueLock);
-      while (AndroidUiTask* task = sTaskQueue->getFirst()) {
-        delete task;
-      }
-    }
 
     delete sMessageLoop;
     sMessageLoop = nullptr;
     MOZ_ASSERT(sThread);
     nsThreadManager::get().UnregisterCurrentThread(*sThread);
     sThread = nullptr;
     mDestroyed = true;
+    sThreadDestroyed = true;
     lock.NotifyAll();
     return NS_OK;
   }
 
   void WaitForDestruction()
   {
     MOZ_ASSERT(sMessageLoopAccessMonitor);
     MonitorAutoLock lock(*sMessageLoopAccessMonitor);
@@ -220,80 +168,39 @@ public:
       lock.Wait();
     }
   }
 
 private:
   bool mDestroyed;
 };
 
-void
-EnqueueTask(already_AddRefed<nsIRunnable> aTask, int aDelayMs)
-{
-
-  if (sThreadDestroyed) {
-    return;
-  }
-
-  // add the new task into the sTaskQueue, sorted with
-  // the earliest task first in the queue
-  AndroidUiTask* newTask = (aDelayMs ? new AndroidUiTask(mozilla::Move(aTask), aDelayMs)
-                                 : new AndroidUiTask(mozilla::Move(aTask)));
-
-  {
-    MOZ_ASSERT(sTaskQueue);
-    MOZ_ASSERT(sTaskQueueLock);
-    MutexAutoLock lock(*sTaskQueueLock);
-
-    AndroidUiTask* task = sTaskQueue->getFirst();
-
-    while (task) {
-      if (newTask->IsEarlierThan(*task)) {
-        task->setPrevious(newTask);
-        break;
-      }
-      task = task->getNext();
-    }
-
-    if (!newTask->isInList()) {
-      sTaskQueue->insertBack(newTask);
-    }
-  }
-
-  if (!newTask->getPrevious()) {
-    // if we're inserting it at the head of the queue, notify Java because
-    // we need to get a callback at an earlier time than the last scheduled
-    // callback
-    GeckoThread::RequestUiThreadCallback(int64_t(aDelayMs));
-  }
-}
-
 } // namespace
 
 namespace mozilla {
 
 void
 CreateAndroidUiThread()
 {
   MOZ_ASSERT(!sThread);
   MOZ_ASSERT(!sMessageLoopAccessMonitor);
-  sTaskQueue = new LinkedList<AndroidUiTask>();
-  sTaskQueueLock = new Mutex("AndroidUiThreadTaskQueueLock");
   sMessageLoopAccessMonitor = new Monitor("AndroidUiThreadMessageLoopAccessMonitor");
   sThreadDestroyed = false;
   RefPtr<CreateOnUiThread> runnable = new CreateOnUiThread;
-  EnqueueTask(do_AddRef(runnable), 0);
+  AndroidBridge::Bridge()->PostTaskToUiThread(do_AddRef(runnable), 0);
 }
 
 void
 DestroyAndroidUiThread()
 {
   MOZ_ASSERT(sThread);
+  // Insure the Android bridge has not already been deconstructed.
+  MOZ_ASSERT(AndroidBridge::Bridge() != nullptr);
   RefPtr<DestroyOnUiThread> runnable = new DestroyOnUiThread;
-  EnqueueTask(do_AddRef(runnable), 0);
+  AndroidBridge::Bridge()->PostTaskToUiThread(do_AddRef(runnable), 0);
   runnable->WaitForDestruction();
   delete sMessageLoopAccessMonitor;
   sMessageLoopAccessMonitor = nullptr;
 }
 
 MessageLoop*
 GetAndroidUiThreadMessageLoop()
 {
@@ -319,43 +226,9 @@ GetAndroidUiThread()
   MonitorAutoLock lock(*sMessageLoopAccessMonitor);
   while (!sThread) {
     lock.Wait();
   }
 
   return sThread;
 }
 
-int64_t
-RunAndroidUiTasks()
-{
-  MutexAutoLock lock(*sTaskQueueLock);
-
-  if (sThreadDestroyed) {
-    return -1;
-  }
-
-  while (!sTaskQueue->isEmpty()) {
-    AndroidUiTask* task = sTaskQueue->getFirst();
-    const int64_t timeLeft = task->MillisecondsToRunTime();
-    if (timeLeft > 0) {
-      // this task (and therefore all remaining tasks)
-      // have not yet reached their runtime. return the
-      // time left until we should be called again
-      return timeLeft;
-    }
-
-    // Retrieve task before unlocking/running.
-    nsCOMPtr<nsIRunnable> runnable(task->TakeTask());
-    // LinkedListElements auto remove from list upon destruction
-    delete task;
-
-    // Unlock to allow posting new tasks reentrantly.
-    MutexAutoUnlock unlock(*sTaskQueueLock);
-    runnable->Run();
-    if (sThreadDestroyed) {
-      return -1;
-    }
-  }
-  return -1;
-}
-
 } // namespace mozilla
--- a/widget/android/AndroidUiThread.h
+++ b/widget/android/AndroidUiThread.h
@@ -10,16 +10,15 @@
 #include <nsThread.h>
 
 class MessageLoop;
 
 namespace mozilla {
 
 void CreateAndroidUiThread();
 void DestroyAndroidUiThread();
-int64_t RunAndroidUiTasks();
 
 MessageLoop* GetAndroidUiThreadMessageLoop();
 RefPtr<nsThread> GetAndroidUiThread();
 
 } // namespace mozilla
 
 #endif // AndroidUiThread_h__
--- a/widget/android/nsAppShell.cpp
+++ b/widget/android/nsAppShell.cpp
@@ -237,17 +237,17 @@ public:
                 category.get(),
                 nullptr, // aOrigin
                 category.get(),
                 aData ? aData->ToString().get() : nullptr);
     }
 
     static int64_t RunUiThreadCallback()
     {
-        return RunAndroidUiTasks();
+        return AndroidUiThread::RunDelayedTasksIfValid();
     }
 };
 
 int32_t GeckoThreadSupport::sPauseCount;
 
 
 class GeckoAppShellSupport final
     : public java::GeckoAppShell::Natives<GeckoAppShellSupport>
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -417,23 +417,19 @@ public:
         // the race condition.
 
         typedef NativePanZoomController::GlobalRef NPZCRef;
         auto callDestroy = [] (const NPZCRef& npzc) {
             npzc->Destroy();
         };
 
         NativePanZoomController::GlobalRef npzc = mNPZC;
-        RefPtr<nsThread> uiThread = GetAndroidUiThread();
-        if (!uiThread) {
-            return;
-        }
-        uiThread->Dispatch(NewRunnableFunction(
+        AndroidBridge::Bridge()->PostTaskToUiThread(NewRunnableFunction(
                 static_cast<void(*)(const NPZCRef&)>(callDestroy),
-                mozilla::Move(npzc)), nsIThread::DISPATCH_NORMAL);
+                mozilla::Move(npzc)), 0);
     }
 
 public:
     void SetIsLongpressEnabled(bool aIsLongpressEnabled)
     {
         RefPtr<IAPZCTreeManager> controller;
 
         if (LockedWindowPtr window{mWindow}) {