fix bugs, optimize code in dm and dms

Signed-off-by: xiaojianfeng <xiaojianfeng3@huawei.com>
Change-Id: Ic11ba2306fdbc24721a71e53d85aadbc8b2a3ab5

dm/src/display.cpp

@@ -108,7 +108,7 @@ void Display::UpdateDisplayInfo() const
 
 float Display::GetVirtualPixelRatio() const
 {
-    // TODO: Should get from DMS
+    // Should get from DMS
     if ((pImpl_->GetDisplayInfo()->GetWidth() == LARGE_SCREEN_WIDTH)
         || (pImpl_->GetDisplayInfo()->GetHeight() == LARGE_SCREEN_WIDTH)) {
         return 2.0f;

dm/src/display_manager.cpp

@@ -33,6 +33,7 @@ WM_IMPLEMENT_SINGLE_INSTANCE(DisplayManager)
 
 class DisplayManager::Impl : public RefBase {
 public:
+    ~Impl();
     static inline SingletonDelegator<DisplayManager> delegator;
     bool CheckRectValid(const Media::Rect& rect, int32_t oriHeight, int32_t oriWidth) const;
     bool CheckSizeValid(const Media::Size& size, int32_t oriHeight, int32_t oriWidth) const;
@@ -187,6 +188,30 @@ void DisplayManager::Impl::ClearDisplayStateCallback()
     }
 }
 
+DisplayManager::Impl::~Impl()
+{
+    std::lock_guard<std::recursive_mutex> lock(mutex_);
+    bool res = true;
+    if (displayManagerListener_ != nullptr) {
+        res = SingletonContainer::Get<DisplayManagerAdapter>().UnregisterDisplayManagerAgent(
+            displayManagerListener_, DisplayManagerAgentType::DISPLAY_EVENT_LISTENER);
+    }
+    displayManagerListener_ = nullptr;
+    if (!res) {
+        WLOGFW("UnregisterDisplayManagerAgent DISPLAY_EVENT_LISTENER failed !");
+    }
+    res = true;
+    if (powerEventListenerAgent_ != nullptr) {
+        res = SingletonContainer::Get<DisplayManagerAdapter>().UnregisterDisplayManagerAgent(
+            powerEventListenerAgent_, DisplayManagerAgentType::DISPLAY_POWER_EVENT_LISTENER);
+    }
+    powerEventListenerAgent_ = nullptr;
+    if (!res) {
+        WLOGFW("UnregisterDisplayManagerAgent DISPLAY_POWER_EVENT_LISTENER failed !");
+    }
+    ClearDisplayStateCallback();
+}
+
 DisplayManager::DisplayManager() : pImpl_(new Impl())
 {
 }

dm/src/screen.cpp

@@ -30,7 +30,7 @@ public:
         screenInfo_ = info;
     }
     ~Impl() = default;
-    DEFINE_VAR_FUNC_GET_SET(sptr<ScreenInfo>, ScreenInfo, screenInfo);
+    DEFINE_VAR_FUNC_GET_SET_WITH_LOCK(sptr<ScreenInfo>, ScreenInfo, screenInfo);
 };
 
 Screen::Screen(sptr<ScreenInfo> info)

dm/src/screen_group.cpp

@@ -30,7 +30,7 @@ public:
     }
     ~Impl() = default;
 
-    DEFINE_VAR_FUNC_GET_SET(sptr<ScreenGroupInfo>, ScreenGroupInfo, screenGroupInfo);
+    DEFINE_VAR_FUNC_GET_SET_WITH_LOCK(sptr<ScreenGroupInfo>, ScreenGroupInfo, screenGroupInfo);
 };
 
 ScreenGroup::ScreenGroup(sptr<ScreenGroupInfo> info)

dm/src/screen_manager.cpp

@@ -32,7 +32,7 @@ namespace {
 class ScreenManager::Impl : public RefBase {
 public:
     Impl() = default;
-    ~Impl() = default;
+    ~Impl();
     static inline SingletonDelegator<ScreenManager> delegator;
     bool RegisterScreenListener(sptr<IScreenListener> listener);
     bool UnregisterScreenListener(sptr<IScreenListener> listener);
@@ -149,6 +149,21 @@ ScreenManager::~ScreenManager()
 {
 }
 
+ScreenManager::Impl::~Impl()
+{
+    std::lock_guard<std::recursive_mutex> lock(mutex_);
+    bool res = true;
+    if (screenManagerListener_ != nullptr) {
+        res = SingletonContainer::Get<ScreenManagerAdapter>().UnregisterDisplayManagerAgent(
+            screenManagerListener_,
+            DisplayManagerAgentType::SCREEN_EVENT_LISTENER);
+    }
+    screenManagerListener_ = nullptr;
+    if (!res) {
+        WLOGFW("UnregisterDisplayManagerAgent SCREEN_EVENT_LISTENER failed !");
+    }
+}
+
 sptr<Screen> ScreenManager::Impl::GetScreen(ScreenId screenId)
 {
     auto screenInfo = SingletonContainer::Get<ScreenManagerAdapter>().GetScreenInfo(screenId);
@@ -228,7 +243,7 @@ bool ScreenManager::Impl::RegisterScreenListener(sptr<IScreenListener> listener)
     } else {
         screenListeners_.insert(listener);
     }
-    return true;
+    return ret;
 }
 
 bool ScreenManager::RegisterScreenListener(sptr<IScreenListener> listener)
@@ -248,14 +263,15 @@ bool ScreenManager::Impl::UnregisterScreenListener(sptr<IScreenListener> listene
         WLOGFE("could not find this listener");
         return false;
     }
+    bool ret = true;
     screenListeners_.erase(iter);
-    if (screenListeners_.empty() && screenGroupMap_.empty() && screenManagerListener_ != nullptr) {
-        SingletonContainer::Get<ScreenManagerAdapter>().UnregisterDisplayManagerAgent(
+    if (screenListeners_.empty() && screenGroupListeners_.empty() && screenManagerListener_ != nullptr) {
+        ret = SingletonContainer::Get<ScreenManagerAdapter>().UnregisterDisplayManagerAgent(
             screenManagerListener_,
             DisplayManagerAgentType::SCREEN_EVENT_LISTENER);
         screenManagerListener_ = nullptr;
     }
-    return true;
+    return ret;
 }
 
 bool ScreenManager::UnregisterScreenListener(sptr<IScreenListener> listener)
@@ -303,14 +319,15 @@ bool ScreenManager::Impl::UnregisterScreenGroupListener(sptr<IScreenGroupListene
         WLOGFE("could not find this listener");
         return false;
     }
+    bool ret = true;
     screenGroupListeners_.erase(iter);
-    if (screenGroupListeners_.empty() && screenGroupMap_.empty() && screenManagerListener_ != nullptr) {
-        SingletonContainer::Get<ScreenManagerAdapter>().UnregisterDisplayManagerAgent(
+    if (screenGroupListeners_.empty() && screenGroupListeners_.empty() && screenManagerListener_ != nullptr) {
+        ret = SingletonContainer::Get<ScreenManagerAdapter>().UnregisterDisplayManagerAgent(
             screenManagerListener_,
             DisplayManagerAgentType::SCREEN_EVENT_LISTENER);
         screenManagerListener_ = nullptr;
     }
-    return true;
+    return ret;
 }
 
 bool ScreenManager::UnregisterScreenGroupListener(sptr<IScreenGroupListener> listener)

dmserver/include/abstract_screen_controller.h

@@ -50,7 +50,6 @@ public:
     std::vector<ScreenId> GetAllExpandOrMirrorScreenIds(std::vector<ScreenId>) const;
     sptr<AbstractScreenGroup> GetAbstractScreenGroup(ScreenId dmsScreenId);
     ScreenId GetDefaultAbstractScreenId();
-    ScreenId GetDefaultScreenId() const; // save default screenID got by GetDefaultAbstractScreenId as cache
     ScreenId ConvertToRsScreenId(ScreenId dmsScreenId);
     ScreenId ConvertToDmsScreenId(ScreenId rsScreenId);
     void RegisterAbstractScreenCallback(sptr<AbstractScreenCallback> cb);
@@ -97,6 +96,9 @@ private:
 
     class ScreenIdManager {
     public:
+        ScreenIdManager() = default;
+        ~ScreenIdManager() = default;
+        WM_DISALLOW_COPY_AND_MOVE(ScreenIdManager);
         ScreenId CreateAndGetNewScreenId(ScreenId rsScreenId);
         bool DeleteScreenId(ScreenId dmsScreenId);
         bool HasDmsScreenId(ScreenId dmsScreenId) const;
@@ -118,7 +120,6 @@ private:
     std::map<ScreenId, sptr<AbstractScreen>> dmsScreenMap_;
     std::map<ScreenId, sptr<AbstractScreenGroup>> dmsScreenGroupMap_;
     sptr<AbstractScreenCallback> abstractScreenCallback_;
-    ScreenId defaultScreenId = INVALID_SCREEN_ID;
 };
 } // namespace OHOS::Rosen
 #endif // FOUNDATION_DMSERVER_ABSTRACT_SCREEN_CONTROLLER_H

dmserver/include/display_manager_interface.h

@@ -60,7 +60,7 @@ public:
         TRANS_ID_SCREENGROUP_BASE = 1100,
         TRANS_ID_SCREEN_MAKE_MIRROR = TRANS_ID_SCREENGROUP_BASE,
         TRANS_ID_SCREEN_MAKE_EXPAND,
-        TRANS_ID_SCREEN_MAKE_MIRROR_OR_EXPAND_CANCELED,
+        TRANS_ID_REMOVE_VIRTUAL_SCREEN_FROM_SCREEN_GROUP,
         TRANS_ID_SCREEN_GAMUT_BASE = 1200,
         TRANS_ID_SCREEN_GET_SUPPORTED_COLOR_GAMUTS = TRANS_ID_SCREEN_GAMUT_BASE,
         TRANS_ID_SCREEN_GET_COLOR_GAMUT,

dmserver/src/abstract_screen_controller.cpp

@@ -157,7 +157,6 @@ ScreenId AbstractScreenController::GetDefaultAbstractScreenId()
     ScreenId defaultDmsScreenId;
     if (screenIdManager_.ConvertToDmsScreenId(rsDefaultId, defaultDmsScreenId)) {
         WLOGI("GetDefaultAbstractScreenId, screen:%{public}" PRIu64"", defaultDmsScreenId);
-        defaultScreenId = defaultDmsScreenId;
         return defaultDmsScreenId;
     }
     WLOGFI("GetDefaultAbstractScreenId, default screen is null, try to get.");
@@ -165,13 +164,7 @@ ScreenId AbstractScreenController::GetDefaultAbstractScreenId()
     if (defaultScreen == nullptr) {
         return SCREEN_ID_INVALID;
     }
-    defaultScreenId = defaultScreen->dmsId_;
-    return defaultScreenId;
-}
-
-ScreenId AbstractScreenController::GetDefaultScreenId() const
-{
-    return defaultScreenId;
+    return defaultScreen->dmsId_;
 }
 
 ScreenId AbstractScreenController::ConvertToRsScreenId(ScreenId dmsScreenId)
@@ -244,10 +237,15 @@ void AbstractScreenController::ProcessScreenConnected(ScreenId rsScreenId)
 sptr<AbstractScreen> AbstractScreenController::InitAndGetScreen(ScreenId rsScreenId)
 {
     ScreenId dmsScreenId = screenIdManager_.CreateAndGetNewScreenId(rsScreenId);
-    sptr<AbstractScreen> absScreen = new AbstractScreen(this, DEFAULT_SCREEN_NAME, dmsScreenId, rsScreenId);
+    sptr<AbstractScreen> absScreen =
+        new(std::nothrow) AbstractScreen(this, DEFAULT_SCREEN_NAME, dmsScreenId, rsScreenId);
+    if (absScreen == nullptr) {
+        WLOGFE("new AbstractScreen failed.");
+        return nullptr;
+    }
     if (!FillAbstractScreen(absScreen, rsScreenId)) {
         screenIdManager_.DeleteScreenId(dmsScreenId);
-        WLOGI("InitAndGetScreen failed.");
+        WLOGFE("InitAndGetScreen failed.");
         return nullptr;
     }
     dmsScreenMap_.insert(std::make_pair(dmsScreenId, absScreen));
@@ -258,10 +256,10 @@ sptr<AbstractScreen> AbstractScreenController::InitAndGetScreen(ScreenId rsScree
 
 void AbstractScreenController::ProcessScreenDisconnected(ScreenId rsScreenId)
 {
-    WLOGI("disconnect screen, screenId=%{public}" PRIu64"", rsScreenId);
+    WLOGFI("disconnect screen, screenId=%{public}" PRIu64"", rsScreenId);
     ScreenId dmsScreenId;
     if (!screenIdManager_.ConvertToDmsScreenId(rsScreenId, dmsScreenId)) {
-        WLOGE("disconnect screen, screenId=%{public}" PRIu64" is not in rs2DmsScreenIdMap_", rsScreenId);
+        WLOGFE("disconnect screen, screenId=%{public}" PRIu64" is not in rs2DmsScreenIdMap_", rsScreenId);
         return;
     }
     auto dmsScreenMapIter = dmsScreenMap_.find(dmsScreenId);
@@ -379,8 +377,12 @@ bool AbstractScreenController::CheckScreenInScreenGroup(sptr<AbstractScreen> scr
 sptr<AbstractScreenGroup> AbstractScreenController::AddAsFirstScreenLocked(sptr<AbstractScreen> newScreen)
 {
     ScreenId dmsGroupScreenId = screenIdManager_.CreateAndGetNewScreenId(SCREEN_ID_INVALID);
-    sptr<AbstractScreenGroup> screenGroup =
-        new AbstractScreenGroup(this, dmsGroupScreenId, SCREEN_ID_INVALID, ScreenCombination::SCREEN_MIRROR);
+    sptr<AbstractScreenGroup> screenGroup = new(std::nothrow) AbstractScreenGroup(this, dmsGroupScreenId,
+        SCREEN_ID_INVALID, ScreenCombination::SCREEN_MIRROR);
+    if (screenGroup == nullptr) {
+        WLOGE("new AbstractScreenGroup failed");
+        return nullptr;
+    }
     Point point;
     if (!screenGroup->AddChild(newScreen, point)) {
         WLOGE("fail to add screen to group. screen=%{public}" PRIu64"", newScreen->dmsId_);
@@ -426,7 +428,7 @@ ScreenId AbstractScreenController::CreateVirtualScreen(VirtualScreenOption optio
 {
     ScreenId rsId = rsInterface_.CreateVirtualScreen(option.name_, option.width_,
         option.height_, option.surface_, SCREEN_ID_INVALID, option.flags_);
-    WLOGFI("CreateVirtualScreen id: %{public}" PRIu64"", rsId);
+    WLOGFI("id: %{public}" PRIu64"", rsId);
     if (rsId == SCREEN_ID_INVALID) {
         return SCREEN_ID_INVALID;
     }
@@ -436,8 +438,12 @@ ScreenId AbstractScreenController::CreateVirtualScreen(VirtualScreenOption optio
         dmsScreenId = screenIdManager_.CreateAndGetNewScreenId(rsId);
         if (!option.isForShot_) {
             WLOGI("CreateVirtualScreen is not shot");
-            sptr<AbstractScreen> absScreen = new AbstractScreen(this, option.name_, dmsScreenId, rsId);
-            sptr<SupportedScreenModes> info = new SupportedScreenModes();
+            sptr<AbstractScreen> absScreen = new(std::nothrow) AbstractScreen(this, option.name_, dmsScreenId, rsId);
+            sptr<SupportedScreenModes> info = new(std::nothrow) SupportedScreenModes();
+            if (absScreen == nullptr || info == nullptr) {
+                WLOGFI("new AbstractScreen or SupportedScreenModes failed");
+                return SCREEN_ID_INVALID;
+            }
             info->width_ = option.width_;
             info->height_ = option.height_;
             auto defaultScreen = GetAbstractScreen(GetDefaultAbstractScreenId());
@@ -453,7 +459,7 @@ ScreenId AbstractScreenController::CreateVirtualScreen(VirtualScreenOption optio
             WLOGI("CreateVirtualScreen is shot");
         }
     } else {
-        WLOGFI("id: %{public}" PRIu64" exit", rsId);
+        WLOGFI("id: %{public}" PRIu64" appears in screenIdManager_. ", rsId);
     }
     return dmsScreenId;
 }
@@ -876,8 +882,8 @@ void AbstractScreenController::DumpScreenGroupInfo() const
 
 void AbstractScreenController::ScreenIdManager::DumpScreenIdInfo() const
 {
-    for (auto iter = dms2RsScreenIdMap_.begin(); iter != dms2RsScreenIdMap_.end(); iter++) {
-        WLOGI("%{public}20" PRIu64" %{public}20" PRIu64"", iter->first, iter->second);
+    for (auto& pair : dms2RsScreenIdMap_) {
+        WLOGI("%{public}20" PRIu64" %{public}20" PRIu64"", pair.first, pair.second);
     }
 };
 

dmserver/src/display_manager_proxy.cpp

@@ -872,7 +872,7 @@ void DisplayManagerProxy::RemoveVirtualScreenFromGroup(std::vector<ScreenId> scr
         WLOGFE("cancel make mirror or expand fail: write screens failed.");
         return;
     }
-    if (remote->SendRequest(TRANS_ID_SCREEN_MAKE_MIRROR_OR_EXPAND_CANCELED, data, reply, option) != ERR_NONE) {
+    if (remote->SendRequest(TRANS_ID_REMOVE_VIRTUAL_SCREEN_FROM_SCREEN_GROUP, data, reply, option) != ERR_NONE) {
         WLOGFW("cancel make mirror or expand fail: SendRequest failed");
     }
 }

dmserver/src/display_manager_service.cpp

@@ -93,7 +93,7 @@ ScreenId DisplayManagerService::GetScreenIdFromDisplayId(DisplayId displayId)
 
 DisplayId DisplayManagerService::GetDefaultDisplayId()
 {
-    ScreenId dmsScreenId = abstractScreenController_->GetDefaultScreenId();
+    ScreenId dmsScreenId = abstractScreenController_->GetDefaultAbstractScreenId();
     WLOGFI("GetDefaultDisplayId %{public}" PRIu64"", dmsScreenId);
     sptr<AbstractDisplay> display = GetDisplayByScreen(dmsScreenId);
     if (display == nullptr) {

dmserver/src/display_manager_stub.cpp

@@ -223,7 +223,7 @@ int32_t DisplayManagerStub::OnRemoteRequest(uint32_t code, MessageParcel &data,
             reply.WriteUint64(static_cast<uint64_t>(result));
             break;
         }
-        case TRANS_ID_SCREEN_MAKE_MIRROR_OR_EXPAND_CANCELED: {
+        case TRANS_ID_REMOVE_VIRTUAL_SCREEN_FROM_SCREEN_GROUP: {
             std::vector<ScreenId> screenId;
             if (!data.ReadUInt64Vector(&screenId)) {
                 WLOGE("fail to receive screens in stub.");

run_git_set_upstream.sh

@@ -1 +0,0 @@
-../../script/run_git_set_upstream.sh