Bug 1731737 [Linux] Call nsDragService::RunScheduledTask() event handler faster and disable its re-entrance r=emilio

We sometimes miss reply to D&D motion event due to delayed DnsDragService::RunScheduledTask() call.
We can't call D&D handler directly from drag-motion event as it causes nested recursions and re-entrance
in glib loop so use higher priority for the delayed handler call at least.

Disable re-entrance of nsDragService::RunScheduledTask() which may happen when
we get D&D data from drag_drop event.

Provide more loggig to D&D code.

Differential Revision: https://phabricator.services.mozilla.com/D126182

widget/gtk/nsDragService.cpp

@@ -30,6 +30,7 @@
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/PresShell.h"
 #include "mozilla/ScopeExit.h"
+#include "mozilla/AutoRestore.h"
 #include "mozilla/WidgetUtilsGtk.h"
 #include "GRefPtr.h"
 
@@ -112,7 +113,8 @@ static void invisibleSourceDragDataGet(GtkWidget* aWidget,
 
 nsDragService::nsDragService()
     : mScheduledTask(eDragTaskNone),
-      mTaskSource(0)
+      mTaskSource(0),
+      mScheduledTaskIsRunning(false)
 #ifdef MOZ_WAYLAND
       ,
       mPendingWaylandDataOffer(nullptr),
@@ -481,6 +483,8 @@ nsDragService::EndDragSession(bool aDoneDrag, uint32_t aKeyModifiers) {
 #ifdef MOZ_WAYLAND
   mTargetWaylandDataOfferForRemote = nullptr;
 #endif
+  mTargetWindow = nullptr;
+  mPendingWindow = nullptr;
 
   return nsBaseDragService::EndDragSession(aDoneDrag, aKeyModifiers);
 }
@@ -978,6 +982,7 @@ void nsDragService::ReplyToDragMotion(GdkDragContext* aDragContext) {
     }
   }
 
+  LOGDRAGSERVICE(("  gdk_drag_status() action %d", action));
   gdk_drag_status(aDragContext, action, mTargetTime);
 }
 
@@ -1142,7 +1147,7 @@ void nsDragService::GetTargetDragData(GdkAtom aFlavor) {
     }
   }
 #ifdef MOZ_WAYLAND
-  else {
+  else if (mTargetWaylandDataOffer) {
     mTargetDragData = mTargetWaylandDataOffer->GetDragData(
         gdk_atom_name(aFlavor), &mTargetDragDataLen);
     mTargetDragDataReceived = true;
@@ -1847,6 +1852,16 @@ gboolean nsDragService::ScheduleDropEvent(nsWindow* aWindow,
   return TRUE;
 }
 
+#ifdef MOZ_LOGGING
+const char* nsDragService::GetDragServiceTaskName(DragTask aTask) {
+  static const char* taskNames[] = {"eDragTaskNone", "eDragTaskMotion",
+                                    "eDragTaskLeave", "eDragTaskDrop",
+                                    "eDragTaskSourceEnd"};
+  MOZ_ASSERT(size_t(aTask) < ArrayLength(taskNames));
+  return taskNames[aTask];
+}
+#endif
+
 gboolean nsDragService::Schedule(DragTask aTask, nsWindow* aWindow,
                                  GdkDragContext* aDragContext,
                                  RefPtr<DataOffer> aWaylandDataOffer,
@@ -1861,9 +1876,15 @@ gboolean nsDragService::Schedule(DragTask aTask, nsWindow* aWindow,
   // within the allowed time).  Otherwise, if we haven't yet run a scheduled
   // drop or end task, just say that we are not ready to receive another
   // drop.
+  LOGDRAGSERVICE(("nsDragService::Schedule() task %s window %p\n",
+                  GetDragServiceTaskName(aTask), aWindow));
+
   if (mScheduledTask == eDragTaskSourceEnd ||
-      (mScheduledTask == eDragTaskDrop && aTask != eDragTaskSourceEnd))
+      (mScheduledTask == eDragTaskDrop && aTask != eDragTaskSourceEnd)) {
+    LOGDRAGSERVICE(("   task does not fit recent task %s, quit!\n",
+                    GetDragServiceTaskName(mScheduledTask)));
     return FALSE;
+  }
 
   mScheduledTask = aTask;
   mPendingWindow = aWindow;
@@ -1875,14 +1896,15 @@ gboolean nsDragService::Schedule(DragTask aTask, nsWindow* aWindow,
   mPendingTime = aTime;
 
   if (!mTaskSource) {
-    // High priority is used here because the native events involved have
+    // High priority is used here because we want to process motion events
-    // already waited at default priority.  Perhaps a lower than default
+    // right after drag_motion event handler which is called by Gtk.
-    // priority could be used for motion tasks because there is a chance
+    // An ideal scenario is to call TaskDispatchCallback() directly here
-    // that a leave or drop is waiting, but managing different priorities
+    // but we can't do that. TaskDispatchCallback() spins gtk event loop
-    // may not be worth the effort.  Motion tasks shouldn't queue up as
+    // while nsDragService::Schedule() is already called from event loop
-    // they should be throttled based on replies.
+    // (by drag_motion* gtk_widget events) so that direct call will cause
-    mTaskSource =
+    // nested recursion.
-        g_idle_add_full(G_PRIORITY_HIGH, TaskDispatchCallback, this, nullptr);
+    mTaskSource = g_timeout_add_full(G_PRIORITY_HIGH, 0, TaskDispatchCallback,
+                                     this, nullptr);
   }
   return TRUE;
 }
@@ -1893,9 +1915,22 @@ gboolean nsDragService::TaskDispatchCallback(gpointer data) {
 }
 
 gboolean nsDragService::RunScheduledTask() {
+  LOGDRAGSERVICE(
+      ("nsDragService::RunScheduledTask() task %s mTargetWindow %p "
+       "mPendingWindow %p\n",
+       GetDragServiceTaskName(mScheduledTask), mTargetWindow.get(),
+       mPendingWindow.get()));
+
+  // Don't run RunScheduledTask() twice. As we use it in main thread only
+  // we don't need to be thread safe here.
+  if (mScheduledTaskIsRunning) {
+    return FALSE;
+  }
+  AutoRestore<bool> guard(mScheduledTaskIsRunning);
+  mScheduledTaskIsRunning = true;
+
   if (mTargetWindow && mTargetWindow != mPendingWindow) {
-    LOGDRAGSERVICE(
+    LOGDRAGSERVICE(("  dispatch eDragExit (%p)\n", mTargetWindow.get()));
-        ("nsDragService: dispatch drag leave (%p)\n", mTargetWindow.get()));
     mTargetWindow->DispatchDragEvent(eDragExit, mTargetWindowPoint, 0);
 
     if (!mSourceNode) {
@@ -1921,6 +1956,7 @@ gboolean nsDragService::RunScheduledTask() {
   mTargetWindowPoint = mPendingWindowPoint;
 
   if (task == eDragTaskLeave || task == eDragTaskSourceEnd) {
+    LOGDRAGSERVICE(("  quit, task %s\n", GetDragServiceTaskName(task)));
     if (task == eDragTaskSourceEnd) {
       // Dispatch drag end events.
       EndDragSession(true, GetCurrentModifiers());
@@ -1939,7 +1975,11 @@ gboolean nsDragService::RunScheduledTask() {
   // (The leave event is not scheduled if a drop task is still scheduled.)
   // We still reply appropriately to indicate that the drop will or didn't
   // succeeed.
-  mTargetWidget = mTargetWindow->GetMozContainerWidget();
+  mTargetWidget =
+      mTargetWindow ? mTargetWindow->GetMozContainerWidget() : nullptr;
+  LOGDRAGSERVICE(("  start drag session mTargetWindow %p mTargetWidget %p\n",
+                  mTargetWindow.get(), mTargetWidget.get()));
+
   mTargetDragContext = std::move(mPendingDragContext);
 #ifdef MOZ_WAYLAND
   mTargetWaylandDataOffer = std::move(mPendingWaylandDataOffer);
@@ -1971,6 +2011,7 @@ gboolean nsDragService::RunScheduledTask() {
   // contain a position.  However, we can't assume the same when the Motif
   // protocol is used.
   if (task == eDragTaskMotion || positionHasChanged) {
+    LOGDRAGSERVICE(("  process motion event\n"));
     UpdateDragAction();
     TakeDragEventDispatchedToChildProcess();  // Clear the old value.
     DispatchMotionEvents();
@@ -1996,12 +2037,14 @@ gboolean nsDragService::RunScheduledTask() {
   }
 
   if (task == eDragTaskDrop) {
+    LOGDRAGSERVICE(("  process drop task\n"));
     gboolean success = DispatchDropEvent();
 
     // Perhaps we should set the del parameter to TRUE when the drag
     // action is move, but we don't know whether the data was successfully
     // transferred.
     if (mTargetDragContext) {
+      LOGDRAGSERVICE(("  drag finished\n"));
       gtk_drag_finish(mTargetDragContext, success,
                       /* del = */ FALSE, mTargetTime);
     }
@@ -2015,6 +2058,7 @@ gboolean nsDragService::RunScheduledTask() {
   }
 
   // We're done with the drag context.
+  LOGDRAGSERVICE(("  clear mTargetWindow mTargetWidget and other data\n"));
   mTargetWidget = nullptr;
   mTargetDragContext = nullptr;
 #ifdef MOZ_WAYLAND
@@ -2030,6 +2074,7 @@ gboolean nsDragService::RunScheduledTask() {
 
   // We have no task scheduled.
   // Returning false removes the task source from the event loop.
+  LOGDRAGSERVICE(("  remove task source\n"));
   mTaskSource = 0;
   return FALSE;
 }
@@ -2045,6 +2090,7 @@ void nsDragService::UpdateDragAction() {
   // more appropriate.  GdkDragContext::actions should be used to set
   // dataTransfer.effectAllowed, which doesn't currently happen with
   // external sources.
+  LOGDRAGSERVICE(("nsDragService::UpdateDragAction()\n"));
 
   // default is to do nothing
   int action = nsIDragService::DRAGDROP_ACTION_NONE;
@@ -2080,6 +2126,8 @@ void nsDragService::UpdateDragAction() {
 
 NS_IMETHODIMP
 nsDragService::UpdateDragEffect() {
+  LOGDRAGSERVICE(
+      ("nsDragService::UpdateDragEffect() from e10s child process\n"));
   if (mTargetDragContextForRemote) {
     ReplyToDragMotion(mTargetDragContextForRemote);
     mTargetDragContextForRemote = nullptr;

widget/gtk/nsDragService.h

@@ -101,6 +101,7 @@ class nsDragService final : public nsBaseDragService, public nsIObserver {
 
   // set the drag icon during drag-begin
   void SetDragIcon(GdkDragContext* aContext);
+  gboolean IsDragActive() { return mScheduledTask != eDragTaskNone; }
 
  protected:
   virtual ~nsDragService();
@@ -121,6 +122,7 @@ class nsDragService final : public nsBaseDragService, public nsIObserver {
   // mTaskSource is the GSource id for the task that is either scheduled
   // or currently running.  It is 0 if no task is scheduled or running.
   guint mTaskSource;
+  bool mScheduledTaskIsRunning;
 
   // target/destination side vars
   // These variables keep track of the state of the current drag.
@@ -207,6 +209,9 @@ class nsDragService final : public nsBaseDragService, public nsIObserver {
   void ReplyToDragMotion(GdkDragContext* aDragContext);
 #ifdef MOZ_WAYLAND
   void ReplyToDragMotion(RefPtr<DataOffer> aDragContext);
+#endif
+#ifdef MOZ_LOGGING
+  const char* GetDragServiceTaskName(nsDragService::DragTask aTask);
 #endif
   gboolean DispatchDropEvent();
   static uint32_t GetCurrentModifiers();

widget/gtk/nsWindow.cpp

@@ -8154,14 +8154,18 @@ static gboolean drag_motion_event_cb(GtkWidget* aWidget,
 void WindowDragLeaveHandler(GtkWidget* aWidget) {
   LOGDRAG(("WindowDragLeaveHandler()\n"));
 
+  RefPtr<nsDragService> dragService = nsDragService::GetInstance();
+  if (!dragService->IsDragActive()) {
+    LOGDRAG(("    Already finished.\n"));
+    return;
+  }
+
   RefPtr<nsWindow> window = get_window_for_gtk_widget(aWidget);
   if (!window) {
     LOGDRAG(("    Failed - can't find nsWindow!\n"));
     return;
   }
 
-  RefPtr<nsDragService> dragService = nsDragService::GetInstance();
-
   nsWindow* mostRecentDragWindow = dragService->GetMostRecentDestWindow();
   if (!mostRecentDragWindow) {
     // This can happen when the target will not accept a drop.  A GTK drag