Merge pull request #254 from morphis/f/fix-ptr-references

Fix cyclic reference between PlatformPolicy and WindowManager and rework DisplayManager

src/anbox/cmds/session_manager.cpp

@@ -177,8 +177,6 @@ anbox::cmds::SessionManager::SessionManager(const BusFactory &bus_factory)
       display_frame = window_size_;
 
     auto policy = std::make_shared<ubuntu::PlatformPolicy>(input_manager, display_frame, single_window_);
-    // FIXME this needs to be removed and solved differently behind the scenes
-    registerDisplayManager(policy);
 
     auto app_db = std::make_shared<application::Database>();
 

src/anbox/graphics/emugl/DisplayManager.cpp

@@ -17,22 +17,22 @@
 
 #include "DisplayManager.h"
 
-namespace {
-std::shared_ptr<DisplayManager> display_mgr;
-
-class NullDisplayManager : public DisplayManager {
- public:
-  DisplayInfo display_info() const override { return {1280, 720}; }
-};
+namespace anbox {
+namespace graphics {
+namespace emugl {
+std::shared_ptr<DisplayInfo> DisplayInfo::get() {
+  static auto info = std::make_shared<DisplayInfo>();
+  return info;
 }
 
-DisplayManager::~DisplayManager() {}
-
-std::shared_ptr<DisplayManager> DisplayManager::get() {
-  if (!display_mgr) display_mgr = std::make_shared<NullDisplayManager>();
-  return display_mgr;
+void DisplayInfo::set_resolution(const std::uint32_t &vertical, const std::uint32_t horizontal) {
+  vertical_resolution_ = vertical;
+  horizontal_resolution_ = horizontal;
 }
 
-void registerDisplayManager(const std::shared_ptr<DisplayManager> &mgr) {
-  display_mgr = mgr;
-}
+std::uint32_t DisplayInfo::vertical_resolution() const { return vertical_resolution_; }
+
+std::uint32_t DisplayInfo::horizontal_resolution() const { return horizontal_resolution_; }
+} // namespace emugl
+} // namespace graphics
+} // namespace anbox

src/anbox/graphics/emugl/DisplayManager.h

@@ -15,25 +15,32 @@
  *
  */
 
-#ifndef DISPLAY_MANAGER_H_
-#define DISPLAY_MANAGER_H_
+#ifndef ANBOX_GRAPHICS_EMUGL_DISPLAY_INFO_H_
+#define ANBOX_GRAPHICS_EMUGL_DISPLAY_INFO_H_
 
+#include <cstdint>
 #include <memory>
 
-class DisplayManager {
+namespace anbox {
+namespace graphics {
+namespace emugl {
+class DisplayInfo {
  public:
-  virtual ~DisplayManager();
+  DisplayInfo() = default;
 
-  struct DisplayInfo {
-    int horizontal_resolution;
-    int vertical_resolution;
-  };
+  static std::shared_ptr<DisplayInfo> get();
 
-  virtual DisplayInfo display_info() const = 0;
+  void set_resolution(const std::uint32_t &vertical, const std::uint32_t horizontal);
 
-  static std::shared_ptr<DisplayManager> get();
-};
+  std::uint32_t vertical_resolution() const;
+  std::uint32_t horizontal_resolution() const;
 
-void registerDisplayManager(const std::shared_ptr<DisplayManager> &mgr);
+ private:
+  std::uint32_t vertical_resolution_ = 1280;
+  std::uint32_t horizontal_resolution_ = 720;
+};
+} // namespace emugl
+} // namespace graphics
+} // namespace anbox
 
 #endif

src/anbox/graphics/emugl/RenderControl.cpp

@@ -169,10 +169,10 @@ static EGLint rcGetFBParam(EGLint param) {
 
   switch (param) {
     case FB_WIDTH:
-      ret = DisplayManager::get()->display_info().horizontal_resolution;
+      ret = static_cast<EGLint>(anbox::graphics::emugl::DisplayInfo::get()->horizontal_resolution());
       break;
     case FB_HEIGHT:
-      ret = DisplayManager::get()->display_info().vertical_resolution;
+      ret = static_cast<EGLint>(anbox::graphics::emugl::DisplayInfo::get()->vertical_resolution());
       break;
     case FB_XDPI:
       ret = 72;  // XXX: should be implemented
@@ -360,12 +360,12 @@ int rcGetNumDisplays() {
 
 int rcGetDisplayWidth(uint32_t display_id) {
   (void)display_id;
-  return DisplayManager::get()->display_info().horizontal_resolution;
+  return static_cast<int>(anbox::graphics::emugl::DisplayInfo::get()->horizontal_resolution());
 }
 
 int rcGetDisplayHeight(uint32_t display_id) {
   (void)display_id;
-  return DisplayManager::get()->display_info().vertical_resolution;
+  return static_cast<int>(anbox::graphics::emugl::DisplayInfo::get()->vertical_resolution());
 }
 
 int rcGetDisplayDpiX(uint32_t display_id) {

src/anbox/ubuntu/platform_policy.cpp

@@ -68,8 +68,7 @@ PlatformPolicy::PlatformPolicy(
     window_size_immutable_ = true;
   }
 
-  display_info_.horizontal_resolution = display_frame.width();
-  display_info_.vertical_resolution = display_frame.height();
+  graphics::emugl::DisplayInfo::get()->set_resolution(display_frame.width(), display_frame.height());
 
   pointer_ = input_manager->create_device();
   pointer_->set_name("anbox-pointer");
@@ -285,10 +284,6 @@ void PlatformPolicy::window_resized(const Window::Id &id,
   }
 }
 
-DisplayManager::DisplayInfo PlatformPolicy::display_info() const {
-  return display_info_;
-}
-
 void PlatformPolicy::set_clipboard_data(const ClipboardData &data) {
   if (data.text.empty())
     return;

src/anbox/ubuntu/platform_policy.h

@@ -41,8 +41,7 @@ class Manager;
 namespace ubuntu {
 class PlatformPolicy : public std::enable_shared_from_this<PlatformPolicy>,
                        public platform::Policy,
-                       public Window::Observer,
-                       public DisplayManager {
+                       public Window::Observer {
  public:
   PlatformPolicy(const std::shared_ptr<input::Manager> &input_manager,
                  const graphics::Rect &static_display_frame = graphics::Rect::Invalid,
@@ -61,8 +60,6 @@ class PlatformPolicy : public std::enable_shared_from_this<PlatformPolicy>,
   void window_resized(const Window::Id &id, const std::int32_t &width,
                       const std::int32_t &height) override;
 
-  DisplayInfo display_info() const override;
-
   void set_renderer(const std::shared_ptr<Renderer> &renderer);
   void set_window_manager(const std::shared_ptr<wm::Manager> &window_manager);
 
@@ -89,7 +86,6 @@ class PlatformPolicy : public std::enable_shared_from_this<PlatformPolicy>,
   bool event_thread_running_;
   std::shared_ptr<input::Device> pointer_;
   std::shared_ptr<input::Device> keyboard_;
-  DisplayManager::DisplayInfo display_info_;
   bool window_size_immutable_ = false;
   bool single_window_ = false;
 };

src/anbox/wm/multi_window_manager.cpp

@@ -25,7 +25,7 @@
 
 namespace anbox {
 namespace wm {
-MultiWindowManager::MultiWindowManager(const std::shared_ptr<platform::Policy> &policy,
+MultiWindowManager::MultiWindowManager(const std::weak_ptr<platform::Policy> &policy,
                                        const std::shared_ptr<bridge::AndroidApiStub> &android_api_stub,
                                        const std::shared_ptr<application::Database> &app_db)
     : platform_policy_(policy), android_api_stub_(android_api_stub), app_db_(app_db) {}
@@ -67,9 +67,16 @@ void MultiWindowManager::apply_window_state_update(const WindowState::List &upda
     if (app.valid())
       title = app.name;
 
-    auto platform_window = platform_policy_->create_window(window.task(), window.frame(), title);
-    platform_window->attach();
-    windows_.insert({window.task(), platform_window});
+    if (auto p = platform_policy_.lock()) {
+      auto w = p->create_window(window.task(), window.frame(), title);
+      if (w) {
+        w->attach();
+        windows_.insert({window.task(), w});
+      } else {
+        // FIXME can we call this here safely or do we need to schedule the removal?
+        remove_task(window.task());
+      }
+    }
   }
 
   // Send updates we collected per task down to the corresponding window

src/anbox/wm/multi_window_manager.h

@@ -37,7 +37,7 @@ class Policy;
 namespace wm {
 class MultiWindowManager : public Manager {
  public:
-  MultiWindowManager(const std::shared_ptr<platform::Policy> &policy,
+  MultiWindowManager(const std::weak_ptr<platform::Policy> &policy,
                      const std::shared_ptr<bridge::AndroidApiStub> &android_api_stub,
                      const std::shared_ptr<application::Database> &app_db);
   ~MultiWindowManager();
@@ -53,7 +53,7 @@ class MultiWindowManager : public Manager {
 
  private:
   std::mutex mutex_;
-  std::shared_ptr<platform::Policy> platform_policy_;
+  std::weak_ptr<platform::Policy> platform_policy_;
   std::shared_ptr<bridge::AndroidApiStub> android_api_stub_;
   std::shared_ptr<application::Database> app_db_;
   std::map<Task::Id, std::shared_ptr<Window>> windows_;

src/anbox/wm/single_window_manager.cpp

@@ -26,7 +26,7 @@
 
 namespace anbox {
 namespace wm {
-SingleWindowManager::SingleWindowManager(const std::shared_ptr<platform::Policy> &policy,
+SingleWindowManager::SingleWindowManager(const std::weak_ptr<platform::Policy> &policy,
                                          const graphics::Rect &window_size,
                                          const std::shared_ptr<application::Database> &app_db)
     : platform_policy_(policy), window_size_(window_size), app_db_(app_db) {}
@@ -34,9 +34,13 @@ SingleWindowManager::SingleWindowManager(const std::shared_ptr<platform::Policy>
 SingleWindowManager::~SingleWindowManager() {}
 
 void SingleWindowManager::setup() {
-  window_ = platform_policy_->create_window(0, window_size_, "Anbox - Android in a Box");
-  if (!window_->attach())
-    WARNING("Failed to attach window to renderer");
+  if (auto p = platform_policy_.lock()) {
+    window_ = p->create_window(0, window_size_, "Anbox - Android in a Box");
+    if (!window_->attach())
+      WARNING("Failed to attach window to renderer");
+  } else {
+    throw std::runtime_error("Can't create window as we don't have a platform abstraction");
+  }
 }
 
 void SingleWindowManager::apply_window_state_update(const WindowState::List &updated, const WindowState::List &removed) {

src/anbox/wm/single_window_manager.h

@@ -35,7 +35,7 @@ namespace wm {
 class Window;
 class SingleWindowManager : public Manager {
  public:
-  SingleWindowManager(const std::shared_ptr<platform::Policy> &policy,
+  SingleWindowManager(const std::weak_ptr<platform::Policy> &policy,
                       const graphics::Rect &window_size,
                       const std::shared_ptr<application::Database> &app_db);
   ~SingleWindowManager();
@@ -52,7 +52,7 @@ class SingleWindowManager : public Manager {
   void remove_task(const Task::Id &task) override;
 
  private:
-  std::shared_ptr<platform::Policy> platform_policy_;
+  std::weak_ptr<platform::Policy> platform_policy_;
   graphics::Rect window_size_;
   std::shared_ptr<application::Database> app_db_;
   std::shared_ptr<Window> window_;