-
Notifications
You must be signed in to change notification settings - Fork 33
refactor: move session related functionality out of Helper #686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Reviewer's GuideRefactors session handling by extracting Session and all user session/socket/XWayland management logic from Helper into a dedicated SessionManager singleton, updates call sites to use it, slightly renames/rootContainer accessor, and alphabetizes various includes and forward declarations. Sequence diagram for updated active session handling via SessionManagersequenceDiagram
actor User
participant GreeterProxy
participant UserModel
participant SessionManager
participant Helper
participant Treeland
User->>GreeterProxy: choose session to log in
GreeterProxy->>UserModel: set user logged in
UserModel-->>GreeterProxy: userLoggedIn(username, id)
GreeterProxy->>SessionManager: userLoggedIn(username, id)
activate SessionManager
SessionManager->>SessionManager: ensureSession(id, username)
SessionManager->>Helper: addSocket(WSocket*) when creating session
SessionManager->>Helper: createXWayland() when creating session
SessionManager-->>SessionManager: updateActiveUserSession(username, id)
SessionManager->>Helper: activateSurface(nullptr)
SessionManager->>SessionManager: disable previous session socket
SessionManager->>SessionManager: enable new session socket
SessionManager-->>SessionManager: socketFileChanged()
SessionManager-->>Treeland: sessionChanged()
deactivate SessionManager
Treeland-->>Treeland: SessionChanged D-Bus signal to treeland_sd
Treeland-->>User: new session environment active
Class diagram for new Session and SessionManager extractionclassDiagram
class Session {
+int id
+uid_t uid
+QString username
+WSocket* socket
+WXWayland* xwayland
+quint32 noTitlebarAtom
+SettingManager* settingManager
+QThread* settingManagerThread
+~Session()
+aboutToBeDestroyed()
}
class SessionManager {
+static SessionManager* m_instance
+std::weak_ptr~Session~ m_activeSession
+QList~std::shared_ptr~Session~~ m_sessions
+SessionManager(QObject* parent)
+~SessionManager()
+static SessionManager* instance()
+QList~std::shared_ptr~Session~~ sessions() const
+std::weak_ptr~Session~ activeSession() const
+bool activeSocketEnabled() const
+void setActiveSocketEnabled(bool newEnabled)
+std::shared_ptr~Session~ ensureSession(int id, QString username)
+void updateActiveUserSession(const QString& username, int id)
+void removeSession(std::shared_ptr~Session~ session)
+std::shared_ptr~Session~ sessionForId(int id) const
+std::shared_ptr~Session~ sessionForUid(uid_t uid) const
+std::shared_ptr~Session~ sessionForUser(const QString& username) const
+std::shared_ptr~Session~ sessionForXWayland(WXWayland* xwayland) const
+std::shared_ptr~Session~ sessionForSocket(WSocket* socket) const
+WXWayland* xwaylandForUid(uid_t uid) const
+WSocket* waylandSocketForUid(uid_t uid) const
+WSocket* globalWaylandSocket() const
+WXWayland* globalXWayland() const
+socketFileChanged()
+sessionChanged()
}
class Helper {
+static Helper* m_instance
-RootSurfaceContainer* m_rootSurfaceContainer
+RootSurfaceContainer* rootSurfaceContainer() const
+void addSocket(WSocket* socket)
+WXWayland* createXWayland()
+ShellHandler* shellHandler() const
+WOutputRenderWindow* window() const
+void activateSurface(SurfaceWrapper* wrapper)
}
class TreelandPrivate {
+SessionManager* sessionManager
+Helper* helper
}
class Treeland {
+Treeland()
+SessionChanged()
}
class GreeterProxy {
+void logout()
+void connected()
+void onSessionNew(const QString& id, const QDBusObjectPath& path)
+void onSessionRemoved(const QString& id, const QDBusObjectPath& path)
}
class ShortcutManagerV2 {
+void onSessionChanged()
}
class UserModel {
+void userLoggedIn(const QString& username, int id)
}
SessionManager --* Session : manages
Helper ..> SessionManager : uses_singleton
TreelandPrivate o--> SessionManager : owns
TreelandPrivate o--> Helper : owns
Treeland ..> TreelandPrivate : uses
GreeterProxy ..> SessionManager : uses_singleton
ShortcutManagerV2 ..> SessionManager : uses_singleton
UserModel ..> SessionManager : emits_to_updateActiveUserSession
Session ..> Helper : calls_shellHandler_and_window
SessionManager ..> Helper : uses_addSocket_and_createXWayland
Focused class diagram for Helper and session-related responsibilities after refactorclassDiagram
class Helper {
+static Helper* m_instance
-RootSurfaceContainer* m_rootSurfaceContainer
+RootSurfaceContainer* rootSurfaceContainer() const
+void addSocket(WSocket* socket)
+WXWayland* createXWayland()
+ShellHandler* shellHandler() const
+WOutputRenderWindow* window() const
+void activateSurface(SurfaceWrapper* wrapper)
+void init(Treeland::Treeland* treeland)
}
class SessionManager {
+static SessionManager* instance()
+QList~std::shared_ptr~Session~~ sessions() const
+std::weak_ptr~Session~ activeSession() const
+bool activeSocketEnabled() const
+void setActiveSocketEnabled(bool newEnabled)
+std::shared_ptr~Session~ ensureSession(int id, QString username)
+void updateActiveUserSession(const QString& username, int id)
+void removeSession(std::shared_ptr~Session~ session)
+std::shared_ptr~Session~ sessionForXWayland(WXWayland* xwayland) const
+WXWayland* xwaylandForUid(uid_t uid) const
+WSocket* waylandSocketForUid(uid_t uid) const
+WSocket* globalWaylandSocket() const
+WXWayland* globalXWayland() const
+socketFileChanged()
+sessionChanged()
}
class RootSurfaceContainer {
+QList~WOutput*~ outputs() const
+WOutput* primaryOutput() const
+WOutputRenderWindow* window() const
}
class ShellHandler {
+WXWayland* createXWayland(WServer* server, WSeat* seat, qw_compositor* compositor, bool lazy)
+void removeXWayland(WXWayland* xwayland)
}
Helper ..> SessionManager : delegates_session_logic
SessionManager ..> Helper : uses_addSocket
SessionManager ..> Helper : uses_createXWayland
Helper --> RootSurfaceContainer : owns
Helper --> ShellHandler : owns_or_accesses
SessionManager ..> Session : manages
Helper ..> RootSurfaceContainer : rootSurfaceContainer_renamed_accessor
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey - I've found 4 issues, and left some high level feedback:
- SessionManager::~Session() and Session::~Session() both delete the same xwayland/socket pointers, which will lead to double-deletion; centralize ownership (either let Session clean them up or only reset pointers in SessionManager) so each resource is freed in exactly one place.
- Several call sites (e.g. Helper::onSurfaceWrapperAdded, ShortcutManagerV2::onSessionChanged, GreeterProxy) assume SessionManager::instance() is non-null; consider making SessionManager a lazy-initialized singleton or adding null checks/guards so these paths cannot crash if SessionManager has not been constructed yet.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- SessionManager::~Session() and Session::~Session() both delete the same xwayland/socket pointers, which will lead to double-deletion; centralize ownership (either let Session clean them up or only reset pointers in SessionManager) so each resource is freed in exactly one place.
- Several call sites (e.g. Helper::onSurfaceWrapperAdded, ShortcutManagerV2::onSessionChanged, GreeterProxy) assume SessionManager::instance() is non-null; consider making SessionManager a lazy-initialized singleton or adding null checks/guards so these paths cannot crash if SessionManager has not been constructed yet.
## Individual Comments
### Comment 1
<location> `src/seat/helper.cpp:1371-1379` </location>
<code_context>
- xdgOutputManager->setFilter([this](WClient *client) { return !isXWaylandClient(client); });
- xwaylandOutputManager->setFilter([this](WClient *client) { return isXWaylandClient(client); });
+
+ auto isXwaylandClient = [this](WClient *client) {
+ for (auto session : SessionManager::instance()->sessions()) {
+ if (session && session->xwayland && session->xwayland->waylandClient() == client) {
+ return true;
+ }
+ }
+ return false;
+ };
+ xdgOutputManager->setFilter([&isXwaylandClient](WClient *client) { return !isXwaylandClient(client); });
+ xwaylandOutputManager->setFilter([&isXwaylandClient](WClient *client) { return isXwaylandClient(client); });
// User dde does not has a real Logind session, so just pass 0 as id
</code_context>
<issue_to_address>
**issue (bug_risk):** The `setFilter` lambdas capture `isXwaylandClient` by reference, which will dangle after `Helper::init` returns.
`isXwaylandClient` is a local lambda in `Helper::init`, but the filter lambdas passed to `setFilter` outlive `init`. Capturing it by reference (`[&isXwaylandClient]`) therefore creates a dangling reference and undefined behavior. Capture it by value instead (e.g. `[isXwaylandClient](WClient *client) { ... }`) or duplicate the logic in each filter.
</issue_to_address>
### Comment 2
<location> `src/session/session.cpp:238-80` </location>
<code_context>
- }
- session->socket = socket;
- }
- if (!session->xwayland) {
- auto *xwayland = createXWayland(session->socket);
- if (!xwayland) {
- delete session->socket;
- m_sessions.removeOne(session);
- return nullptr;
</code_context>
<issue_to_address>
**issue (bug_risk):** On XWayland creation failure for an existing session, `session->socket` is deleted but not nulled, which can lead to double delete later.
In the existing-session path of `ensureSession`, if `createXWayland(session->socket)` fails you `delete session->socket`, but `Session` is still managed by `std::shared_ptr` and its destructor later does `if (socket) delete socket;`. Since `socket` isn’t nulled, this causes a double delete. Please either set `session->socket = nullptr;` after deleting, or (preferably) make `Session` solely own and delete the socket so it isn’t freed here at all.
</issue_to_address>
### Comment 3
<location> `src/session/session.h:19` </location>
<code_context>
-class qw_ext_foreign_toplevel_image_capture_source_manager_v1;
-QW_END_NAMESPACE
-
-struct Session : QObject {
- Q_OBJECT
-public:
</code_context>
<issue_to_address>
**issue (complexity):** Consider encapsulating Session’s state, narrowing SessionManager’s public surface, and using keyed containers so lookups and future evolution remain simpler and clearer without changing behavior.
You can reduce complexity and tighten the design without changing behavior by:
### 1. Encapsulate `Session` internals
Right now all fields are public and the type is a `struct` inheriting `QObject`. That leaks implementation details and encourages external code to poke at internals.
Make it a `class` with minimal accessors for what call‑sites actually need (e.g. `uid`, `username`, `socket`, `xwayland`), and keep the rest private:
```cpp
class Session : public QObject {
Q_OBJECT
public:
explicit Session(QObject *parent = nullptr);
int id() const { return m_id; }
uid_t uid() const { return m_uid; }
const QString &username() const { return m_username; }
WSocket *socket() const { return m_socket; }
WXWayland *xwayland() const { return m_xwayland; }
void setSocket(WSocket *socket) { m_socket = socket; }
void setXWayland(WXWayland *xwayland) { m_xwayland = xwayland; }
// add setters for id/uid/username only if needed
~Session() override;
Q_SIGNALS:
void aboutToBeDestroyed();
private:
int m_id = 0;
uid_t m_uid = 0;
QString m_username;
WSocket *m_socket = nullptr;
WXWayland *m_xwayland = nullptr;
quint32 m_noTitlebarAtom = XCB_ATOM_NONE;
SettingManager *m_settingManager = nullptr;
QThread *m_settingManagerThread = nullptr;
};
```
Call‑sites that currently access fields directly will then use `session->socket()` / `session->xwayland()` etc. Functionality is unchanged but the data model is clearer and easier to evolve.
### 2. Narrow `SessionManager`’s public API via internal helpers
You can keep all existing behavior but reduce surface area by implementing the “special” lookups in terms of one or two generic methods, and making some of them `private` or `static` helpers in the `.cpp`.
Header:
```cpp
struct SessionManager : QObject {
Q_OBJECT
public:
// keep generic entry points
std::shared_ptr<Session> sessionForId(int id) const;
std::shared_ptr<Session> sessionForUid(uid_t uid) const;
std::shared_ptr<Session> sessionForUser(const QString &username) const;
// expose only what really needs to be public:
WSocket *waylandSocketForUid(uid_t uid) const;
WXWayland *xwaylandForUid(uid_t uid) const;
// consider dropping these from the public API if they are only used in one place:
WSocket *globalWaylandSocket() const;
WXWayland *globalXWayland() const;
// ...
};
```
`.cpp` (no behavior change, just indirection):
```cpp
WSocket *SessionManager::waylandSocketForUid(uid_t uid) const
{
if (auto s = sessionForUid(uid)) {
return s->socket();
}
return nullptr;
}
WXWayland *SessionManager::xwaylandForUid(uid_t uid) const
{
if (auto s = sessionForUid(uid)) {
return s->xwayland();
}
return nullptr;
}
WSocket *SessionManager::globalWaylandSocket() const
{
auto pw = getpwnam("dde");
return pw ? waylandSocketForUid(pw->pw_uid) : nullptr;
}
WXWayland *SessionManager::globalXWayland() const
{
auto pw = getpwnam("dde");
return pw ? xwaylandForUid(pw->pw_uid) : nullptr;
}
```
If `globalWaylandSocket()` / `globalXWayland()` are only used in one module, consider moving them as free functions or making them private helpers to keep `SessionManager`’s public API smaller.
### 3. Use keyed containers internally to simplify lookups
All your lookup functions iterate over `m_sessions`. You can keep the existing API and add internal maps keyed by the most common lookups:
```cpp
struct SessionManager : QObject {
// ...
private:
std::weak_ptr<Session> m_activeSession;
QList<std::shared_ptr<Session>> m_sessions;
QHash<int, std::shared_ptr<Session>> m_sessionsById;
QHash<uid_t, std::shared_ptr<Session>> m_sessionsByUid;
QHash<QString, std::shared_ptr<Session>> m_sessionsByUser;
};
```
Then implement `ensureSession` / `removeSession` to maintain these maps:
```cpp
std::shared_ptr<Session> SessionManager::ensureSession(int id, QString username)
{
if (auto existing = sessionForId(id)) {
return existing;
}
auto session = std::make_shared<Session>();
// initialize id/uid/username/...
m_sessions.append(session);
m_sessionsById.insert(id, session);
m_sessionsByUser.insert(username, session);
// fill m_sessionsByUid when uid is known
return session;
}
void SessionManager::removeSession(std::shared_ptr<Session> session)
{
m_sessions.removeAll(session);
m_sessionsById.remove(session->id());
m_sessionsByUid.remove(session->uid());
m_sessionsByUser.remove(session->username());
}
```
And the lookups become trivial and O(1):
```cpp
std::shared_ptr<Session> SessionManager::sessionForId(int id) const
{
return m_sessionsById.value(id);
}
std::shared_ptr<Session> SessionManager::sessionForUid(uid_t uid) const
{
return m_sessionsByUid.value(uid);
}
std::shared_ptr<Session> SessionManager::sessionForUser(const QString &username) const
{
return m_sessionsByUser.value(username);
}
```
This keeps all existing methods but removes repeated iteration logic and makes the intent clearer.
### 4. Plan to remove the singleton (future‑compatible)
If you must keep the singleton right now, you can still make it easier to replace later by constraining its use:
- Prefer passing `SessionManager *` into constructors of components that need it.
- Avoid calling `SessionManager::instance()` deep in leaf code.
Later, switching to an injected/owned instance becomes mechanical:
```cpp
class Helper {
public:
Helper(QObject *parent = nullptr)
: m_sessionManager(new SessionManager(this))
{}
private:
SessionManager *m_sessionManager;
};
```
You don’t need to drop the singleton immediately, but reducing direct `instance()` usage will reduce the global‑state footprint and cognitive load over time.
</issue_to_address>
### Comment 4
<location> `src/session/session.cpp:37` </location>
<code_context>
return data;
}
-Session::~Session()
-{
- qCDebug(treelandCore) << "Deleting session for uid:" << uid << socket;
</code_context>
<issue_to_address>
**issue (complexity):** Consider centralizing resource ownership in Session and extracting shared helpers in SessionManager to simplify lifetime management and reduce repeated lookup/creation logic.
You can reduce the complexity here without changing behaviour by tightening ownership and factoring out some repeated logic.
### 1. Centralize teardown in `Session`
Right now `Session::~Session` and `SessionManager::~SessionManager` both manually delete `socket` and `xwayland`, which is easy to get wrong.
Let `Session` fully own its resources, and let `SessionManager` only manage `std::shared_ptr<Session>`.
**Session.h / Session.cpp**
```cpp
Session::~Session()
{
qCDebug(treelandCore) << "Deleting session for uid:" << uid << socket;
Q_EMIT aboutToBeDestroyed();
if (settingManagerThread) {
settingManagerThread->quit();
settingManagerThread->wait(QDeadlineTimer(25000));
}
delete settingManager;
settingManager = nullptr;
// Centralize XWayland teardown here
if (xwayland) {
Helper::instance()->shellHandler()->removeXWayland(xwayland);
delete xwayland;
xwayland = nullptr;
}
// Centralize socket teardown here
delete socket;
socket = nullptr;
}
```
**SessionManager::~SessionManager**
```cpp
SessionManager::~SessionManager()
{
// shared_ptr will trigger Session::~Session where teardown is centralized
m_sessions.clear();
}
```
This removes duplicated cleanup logic and makes lifetime reasoning simpler.
---
### 2. Extract helpers from `ensureSession`
`ensureSession` is doing multiple things: session lookup, socket creation, XWayland creation, settings thread setup, and error rollback. Pulling some of this into private helpers will make it easier to follow and test.
**SessionManager.h**
```cpp
private:
WSocket *createWSocket();
WXWayland *createXWayland(WSocket *socket);
void setupSessionSettings(Session &session);
```
**SessionManager.cpp**
```cpp
WSocket *SessionManager::createWSocket()
{
auto socket = new WSocket(true, this);
if (!socket->autoCreate()) {
qCCritical(treelandCore) << "Failed to create Wayland socket";
delete socket;
return nullptr;
}
connect(socket, &WSocket::fullServerNameChanged, this, [this] {
if (m_activeSession.lock())
Q_EMIT socketFileChanged();
});
Helper::instance()->addSocket(socket);
return socket;
}
WXWayland *SessionManager::createXWayland(WSocket *socket)
{
auto *xwayland = Helper::instance()->createXWayland();
if (!xwayland) {
qCCritical(treelandCore) << "Failed to create XWayland instance";
return nullptr;
}
xwayland->setOwnsSocket(socket);
connect(xwayland, &WXWayland::ready, this, [this, xwayland] {
if (auto session = sessionForXWayland(xwayland)) {
setupSessionSettings(*session);
}
});
return xwayland;
}
void SessionManager::setupSessionSettings(Session &session)
{
session.noTitlebarAtom =
internAtom(session.xwayland->xcbConnection(), _DEEPIN_NO_TITLEBAR, false);
if (!session.noTitlebarAtom) {
qCWarning(treelandInput) << "Failed to intern atom:" << _DEEPIN_NO_TITLEBAR;
}
session.settingManager = new SettingManager(session.xwayland->xcbConnection(),
session.xwayland);
session.settingManagerThread = new QThread(session.xwayland);
session.settingManager->moveToThread(session.settingManagerThread);
const qreal scale =
Helper::instance()->rootSurfaceContainer()->window()->effectiveDevicePixelRatio();
const auto renderWindow = Helper::instance()->window();
connect(session.settingManagerThread, &QThread::started, this,
[&session, scale, renderWindow] {
QMetaObject::invokeMethod(
session.settingManager,
[&session, scale]() {
session.settingManager->setGlobalScale(scale);
session.settingManager->apply();
},
Qt::QueuedConnection);
QObject::connect(
renderWindow,
&WOutputRenderWindow::effectiveDevicePixelRatioChanged,
session.settingManager,
[&session](qreal dpr) {
session.settingManager->setGlobalScale(dpr);
session.settingManager->apply();
},
Qt::QueuedConnection);
});
connect(session.settingManagerThread, &QThread::finished,
session.settingManagerThread, &QThread::deleteLater);
session.settingManagerThread->start();
}
```
Then `ensureSession` becomes much flatter:
```cpp
std::shared_ptr<Session> SessionManager::ensureSession(int id, QString username)
{
if (auto session = sessionForUser(username)) {
if (!session->socket) {
auto *socket = createWSocket();
if (!socket) {
m_sessions.removeOne(session);
return nullptr;
}
session->socket = socket;
}
if (!session->xwayland) {
auto *xwayland = createXWayland(session->socket);
if (!xwayland) {
delete session->socket;
m_sessions.removeOne(session);
return nullptr;
}
session->xwayland = xwayland;
}
return session;
}
auto session = std::make_shared<Session>();
session->id = id;
session->username = username;
session->uid = getpwnam(username.toLocal8Bit().data())->pw_uid;
session->socket = createWSocket();
if (!session->socket) {
return nullptr;
}
session->xwayland = createXWayland(session->socket);
if (!session->xwayland) {
return nullptr;
}
m_sessions.append(session);
return session;
}
```
This keeps the same behaviour but significantly reduces nesting and local lambdas.
---
### 3. Deduplicate linear search over `m_sessions`
The `sessionFor*` functions all re-implement a linear scan. A small internal helper reduces boilerplate and makes it easier to change storage later.
```cpp
template <typename Predicate>
std::shared_ptr<Session> SessionManager::findSession(Predicate pred) const
{
for (const auto &session : m_sessions) {
if (session && pred(*session))
return session;
}
return nullptr;
}
std::shared_ptr<Session> SessionManager::sessionForId(int id) const
{
return findSession([&](const Session &s) { return s.id == id; });
}
std::shared_ptr<Session> SessionManager::sessionForUid(uid_t uid) const
{
return findSession([&](const Session &s) { return s.uid == uid; });
}
std::shared_ptr<Session> SessionManager::sessionForUser(const QString &username) const
{
return findSession([&](const Session &s) { return s.username == username; });
}
std::shared_ptr<Session> SessionManager::sessionForXWayland(WXWayland *xwayland) const
{
return findSession([&](const Session &s) { return s.xwayland == xwayland; });
}
std::shared_ptr<Session> SessionManager::sessionForSocket(WSocket *socket) const
{
return findSession([&](const Session &s) { return s.socket == socket; });
}
```
This keeps functionality intact while cutting a lot of repetition and making the lookup semantics explicit.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: misaka18931 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@misaka18931: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors session management by extracting session-related functionality from the Helper class into a new dedicated SessionManager singleton. The goal is to improve code organization and separation of concerns.
Changes:
- Created new
SessionManagerclass withSessionstruct in dedicatedsession/directory - Moved all session management logic (create, remove, lookup, activation) from
HelpertoSessionManager - Reordered header includes and forward declarations alphabetically in
helper.handhelper.cpp
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/session/session.h | New header defining Session struct and SessionManager singleton class |
| src/session/session.cpp | Implementation of SessionManager with all session management logic moved from Helper |
| src/seat/helper.h | Removed Session/session-related declarations, reordered includes alphabetically, changed rootContainer to rootSurfaceContainer |
| src/seat/helper.cpp | Removed session implementation, added createXWayland() method, updated to delegate to SessionManager |
| src/modules/shortcut/shortcutrunner.cpp | Updated rootContainer() call to rootSurfaceContainer() |
| src/modules/shortcut/shortcutmanager.cpp | Updated to use SessionManager::instance() instead of Helper |
| src/modules/foreign-toplevel/foreigntoplevelmanagerv1.cpp | Updated rootContainer() call to rootSurfaceContainer() |
| src/greeter/greeterproxy.cpp | Updated to use SessionManager::instance() instead of Helper for session access |
| src/core/treeland.cpp | Created SessionManager instance and updated to use SessionManager for session operations |
| src/CMakeLists.txt | Added new session.cpp and session.h files to build |
|
TAG Bot New tag: 0.8.1 |
cf47127 to
cf24ebf
Compare
Changes: - reordered header & forward class decls alphabetically in helper.h and helper.cpp - removed duplicate getter for `m_rootSurfaceContainer` in `Helper`. - Moved `class Session` and session management related member functions into the new `class SessionManager` SessionManager is designed to be an independent singleton that exclusively manages session creation, invalidation and its related WSockets and WWayland instances.
cf24ebf to
ca0cb68
Compare
|
|
||
| if (CmdLine::ref().run().has_value()) { | ||
| auto exec = [runCmd = CmdLine::ref().run().value(), d] { | ||
| auto exec = [runCmd = CmdLine::ref().run().value(), d, &defaultSession] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个 defaultSession 不是个指针吗,用&有啥意义吗
| // assert(wlroots: assert(wl_list_empty(&cur->events.button.listener_list))) | ||
| // failed during quit(If the quit call is from the cursor's button press/release event) | ||
| connect(qmlEngine, &QQmlEngine::quit, q, &Treeland::quit, Qt::QueuedConnection); | ||
| sessionManager = new SessionManager(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个对象在哪里销毁的?
|
|
||
| d->init(); | ||
|
|
||
| auto defaultSession = d->sessionManager->defaultSession(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这个是defaultSession还是globalSession,如果之前是叫global的,就不要改名叫default。
| Q_EMIT isLoggedInChanged(); | ||
| auto session = Helper::instance()->activeSession().lock(); | ||
| SocketWriter(d->socket) << quint32(GreeterMessages::Logout) << session->id; | ||
| auto session = SessionManager::instance()->activeSession().lock(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
单例建议只保留一个 Helper 对象,用法上可以 Helper::instance()->sessionManager()->activeSession().lock()
所有的内部接口都通过Helper对象暴露。
| xwaylandOutputManager->setFilter([this](WClient *client) { return isXWaylandClient(client); }); | ||
|
|
||
| static const auto isXWaylandClient = [](WClient *client) { | ||
| for (auto session : SessionManager::instance()->sessions()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
确保 SessionManager::instance()->sessions() 是个 const 类型,如果不是,可以用 const auto &xxx = SessionManager::instance()->sessions(); 存储一个临时值。
Changes:
class Sessionand session management related member functions into the newclass SessionManagerSessionManager is designed to be an independent singleton that exclusively manages session creation, invalidation and its related WSockets and WWayland instances.
Summary by Sourcery
Extract session and socket management from Helper into a dedicated SessionManager singleton and update callers to use it.
Enhancements:
Build: