[AT-SPI] Rework intercepting key events 18/278818/8
authorArtur Świgoń <a.swigon@samsung.com>
Fri, 15 Sep 2023 09:11:24 +0000 (11:11 +0200)
committerArtur Świgoń <a.swigon@samsung.com>
Fri, 1 Mar 2024 09:40:18 +0000 (10:40 +0100)
- Make KeyEvent intercepting asynchronous to prevent deadlocks.
- Support intercepting on windows other than the main window.

Change-Id: I0f8683c82900483498479c501c0555f2cf1683d0

dali/devel-api/adaptor-framework/accessibility-bridge.h
dali/devel-api/adaptor-framework/accessibility.h
dali/internal/accessibility/bridge/bridge-impl.cpp
dali/internal/accessibility/bridge/dummy/dummy-atspi.h
dali/internal/adaptor/common/adaptor-impl.cpp
dali/internal/adaptor/common/adaptor-impl.h
dali/internal/window-system/common/window-impl.cpp
dali/internal/window-system/common/window-impl.h

index 38ccd70..6d30da6 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_ADAPTOR_ACCESSIBILITY_BRIDGE_H
 
 /*
- * Copyright (c) 2021 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2024 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,6 +20,7 @@
 
 // EXTERNAL INCLUDES
 #include <dali/public-api/actors/actor.h>
+#include <dali/public-api/events/key-event.h>
 #include <dali/public-api/math/rect.h>
 #include <functional>
 #include <memory>
@@ -364,14 +365,11 @@ struct DALI_ADAPTOR_API Bridge
    * Screen-reader might receive this event and reply, that given keycode is consumed. In that case
    * further processing of the keycode should be ignored.
    *
-   * @param[in] type Key event type
-   * @param[in] keyCode Key code
-   * @param[in] keyName Key name
-   * @param[in] timeStamp Time stamp
-   * @param[in] isText Whether it's text or not
-   * @return Whether this event is consumed or not
-   **/
-  virtual Consumed Emit(KeyEventType type, unsigned int keyCode, const std::string& keyName, unsigned int timeStamp, bool isText) = 0;
+   * @param[in] keyEvent The key event
+   * @param[in] callback Notification if the event was consumed
+   * @return true if the event was emitted
+   */
+  virtual bool EmitKeyEvent(Dali::KeyEvent keyEvent, std::function<void(Dali::KeyEvent, bool)> callback) = 0;
 
   /**
    * @brief Reads given text by screen reader
index e4ff562..d6ff27c 100644 (file)
@@ -594,26 +594,6 @@ private:
 };\r
 \r
 /**\r
- * @brief Enumeration describing type of key event\r
- * @see Adaptor::AccessibilityObserver::OnAccessibleKeyEvent\r
- */\r
-enum class KeyEventType\r
-{\r
-  KEY_PRESSED,\r
-  KEY_RELEASED,\r
-};\r
-\r
-/**\r
- * @brief Enumeration with human readable values describing state of event\r
- * @see Dali::Accessibility::Bridge::Emit\r
- */\r
-enum class Consumed\r
-{\r
-  NO,\r
-  YES\r
-};\r
-\r
-/**\r
  * @brief Helper class representing two dimensional point with integer coordinates\r
  */\r
 struct DALI_ADAPTOR_API Point\r
index 3851cbc..2fd6c49 100644 (file)
@@ -94,43 +94,43 @@ public:
   BridgeImpl() = default;
 
   /**
-   * @copydoc Dali::Accessibility::Bridge::Emit()
+   * @copydoc Dali::Accessibility::Bridge::EmitKeyEvent()
    */
-  Consumed Emit(KeyEventType type, unsigned int keyCode, const std::string& keyName, unsigned int timeStamp, bool isText) override
+  bool EmitKeyEvent(Dali::KeyEvent keyEvent, std::function<void(Dali::KeyEvent, bool)> callback) override
   {
+    using ArgumentTypes = std::tuple<uint32_t, int32_t, int32_t, int32_t, int32_t, std::string, bool>;
+
+    static const char* methodName = "NotifyListenersSync";
+
     if(!IsUp())
     {
-      return Consumed::NO;
+      return false;
     }
 
-    unsigned int keyType = 0;
+    uint32_t keyType   = (keyEvent.GetState() == Dali::KeyEvent::DOWN ? 0U : 1U);
+    auto     timeStamp = static_cast<std::int32_t>(keyEvent.GetTime());
+    bool     isText    = !keyEvent.GetKeyString().empty();
 
-    switch(type)
-    {
-      case KeyEventType::KEY_PRESSED:
-      {
-        keyType = 0;
-        break;
-      }
-      case KeyEventType::KEY_RELEASED:
+    ArgumentTypes arguments(keyType, 0, keyEvent.GetKeyCode(), 0, timeStamp, keyEvent.GetKeyName(), isText);
+
+    auto functor = [keyEvent = std::move(keyEvent), callback = std::move(callback)](DBus::ValueOrError<bool> reply) {
+      bool consumed = false;
+
+      if(!reply)
       {
-        keyType = 1;
-        break;
+        DALI_LOG_ERROR("%s call failed: %s", methodName, reply.getError().message.c_str());
       }
-      default:
+      else
       {
-        return Consumed::NO;
+        consumed = std::get<0>(reply.getValues());
       }
-    }
 
-    auto methodObject = mRegistryClient.method<bool(std::tuple<uint32_t, int32_t, int32_t, int32_t, int32_t, std::string, bool>)>("NotifyListenersSync");
-    auto result       = methodObject.call(std::tuple<uint32_t, int32_t, int32_t, int32_t, int32_t, std::string, bool>{keyType, 0, static_cast<int32_t>(keyCode), 0, static_cast<int32_t>(timeStamp), keyName, isText ? 1 : 0});
-    if(!result)
-    {
-      LOG() << result.getError().message;
-      return Consumed::NO;
-    }
-    return std::get<0>(result) ? Consumed::YES : Consumed::NO;
+      callback(std::move(keyEvent), consumed);
+    };
+
+    mRegistryClient.method<bool(ArgumentTypes)>(methodName).asyncCall(std::move(functor), arguments);
+
+    return true;
   }
 
   /**
index 9e0035a..a5ed289 100644 (file)
@@ -2,7 +2,7 @@
 #define DALI_ADAPTOR_DUMMY_ATSPI_H
 
 /*
- * Copyright (c) 2022 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2024 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -174,9 +174,9 @@ struct DummyBridge : Dali::Accessibility::Bridge
   {
   }
 
-  Accessibility::Consumed Emit(Accessibility::KeyEventType type, unsigned int keyCode, const std::string& keyName, unsigned int timeStamp, bool isText) override
+  bool EmitKeyEvent(Dali::KeyEvent keyEvent, std::function<void(Dali::KeyEvent, bool)> callback) override
   {
-    return Accessibility::Consumed::YES;
+    return false;
   }
 
   void Say(const std::string& text, bool discardable, std::function<void(std::string)> callback) override
index 33c1788..a77acc2 100644 (file)
@@ -325,24 +325,6 @@ void Adaptor::Initialize(GraphicsFactory& graphicsFactory)
   mConfigurationManager = Utils::MakeUnique<ConfigurationManager>(systemCachePath, mGraphics.get(), mThreadController);
 }
 
-void Adaptor::AccessibilityObserver::OnAccessibleKeyEvent(const Dali::KeyEvent& event)
-{
-  Accessibility::KeyEventType type;
-  if(event.GetState() == Dali::KeyEvent::DOWN)
-  {
-    type = Accessibility::KeyEventType::KEY_PRESSED;
-  }
-  else if(event.GetState() == Dali::KeyEvent::UP)
-  {
-    type = Accessibility::KeyEventType::KEY_RELEASED;
-  }
-  else
-  {
-    return;
-  }
-  Dali::Accessibility::Bridge::GetCurrentBridge()->Emit(type, event.GetKeyCode(), event.GetKeyName(), event.GetTime(), !event.GetKeyString().empty());
-}
-
 Adaptor::~Adaptor()
 {
   Accessibility::Bridge::GetCurrentBridge()->Terminate();
@@ -405,7 +387,6 @@ void Adaptor::Start()
   auto bridge  = Accessibility::Bridge::GetCurrentBridge();
   bridge->SetApplicationName(appName);
   bridge->Initialize();
-  Dali::Stage::GetCurrent().KeyEventSignal().Connect(&mAccessibilityObserver, &AccessibilityObserver::OnAccessibleKeyEvent);
 
   Dali::Internal::Adaptor::SceneHolder* defaultWindow = mWindows.front();
 
index aaff350..23459c3 100644 (file)
@@ -50,11 +50,6 @@ namespace Dali
 {
 class RenderSurfaceInterface;
 
-namespace Accessibility
-{
-class Bridge;
-}
-
 namespace Integration
 {
 class Core;
@@ -752,13 +747,6 @@ private:                                          // Data
 
   std::unique_ptr<Integration::AddOnManager> mAddOnManager; ///< Pointer to the addon manager
 
-  class AccessibilityObserver : public ConnectionTracker
-  {
-  public:
-    void OnAccessibleKeyEvent(const Dali::KeyEvent& event);
-  };
-  AccessibilityObserver mAccessibilityObserver;
-
 public:
   inline static Adaptor& GetImplementation(Dali::Adaptor& adaptor)
   {
index e77d63e..c43ae91 100644 (file)
@@ -1197,10 +1197,12 @@ void Window::OnAccessibilityEnabled()
   auto bridge     = Accessibility::Bridge::GetCurrentBridge();
   auto rootLayer  = mScene.GetRootLayer();
   auto accessible = Accessibility::Accessible::Get(rootLayer);
-  bridge->AddTopLevelWindow(accessible);
 
   DALI_LOG_RELEASE_INFO("Window (%p), WinId (%d), Accessibility is enabled\n", this, mNativeWindowId);
 
+  bridge->AddTopLevelWindow(accessible);
+  InterceptKeyEventSignal().Connect(this, &Window::OnAccessibilityInterceptKeyEvent);
+
   Dali::Window handle(this);
   if(!mIsEmittedWindowCreatedEvent)
   {
@@ -1228,8 +1230,38 @@ void Window::OnAccessibilityDisabled()
   auto bridge     = Accessibility::Bridge::GetCurrentBridge();
   auto rootLayer  = mScene.GetRootLayer();
   auto accessible = Accessibility::Accessible::Get(rootLayer);
-  bridge->RemoveTopLevelWindow(accessible);
+
   DALI_LOG_RELEASE_INFO("Window (%p), WinId (%d), Accessibility is disabled\n", this, mNativeWindowId);
+
+  InterceptKeyEventSignal().Disconnect(this, &Window::OnAccessibilityInterceptKeyEvent);
+  bridge->RemoveTopLevelWindow(accessible);
+}
+
+bool Window::OnAccessibilityInterceptKeyEvent(const Dali::KeyEvent& keyEvent)
+{
+  auto bridge = Accessibility::Bridge::GetCurrentBridge();
+
+  if(!bridge->IsUp() || keyEvent.IsNoInterceptModifier())
+  {
+    DALI_LOG_ERROR("This KeyEvent should not have been intercepted!");
+
+    return false;
+  }
+
+  auto callback = [handle = Dali::Window(this)](Dali::KeyEvent keyEvent, bool consumed) {
+    if(!consumed)
+    {
+      Dali::DevelKeyEvent::SetNoInterceptModifier(keyEvent, true);
+      Dali::DevelWindow::FeedKeyEvent(handle, keyEvent);
+    }
+  };
+
+  bool emitted = bridge->EmitKeyEvent(keyEvent, callback);
+
+  // If emitted, consume the event in order not to have to wait for the D-Bus call
+  // to finish. If the event turns out not to be consumed by the remote client,
+  // then it is fed back to the window from the D-Bus callback.
+  return emitted;
 }
 
 void Window::OnMoveCompleted(Dali::Window::WindowPosition& position)
index c138780..af90626 100644 (file)
@@ -691,6 +691,14 @@ private:
   void OnAccessibilityDisabled();
 
   /**
+   * @brief Called in Accessibility mode on every KeyEvent
+   *
+   * @param[in] keyEvent The key event
+   * @return Always true, meaning that the event is consumed
+   */
+  bool OnAccessibilityInterceptKeyEvent(const Dali::KeyEvent& keyEvent);
+
+  /**
    * @brief Called when the window rotation is finished.
    *
    * This signal is emmit when window rotation is finisehd and WindowRotationCompleted() is called.