[crd host][linux] Fix GnomeRemoteDesktopSession in multi-process host [chromium/src : main]

0 views
Skip to first unread message

Yuwei Huang (Gerrit)

unread,
Feb 12, 2026, 9:27:46 PM (9 days ago) Feb 12
to Lambros Lambrou, chromium...@chromium.org, chromotin...@chromium.org
Attention needed from Lambros Lambrou

Yuwei Huang voted and added 1 comment

Votes added by Yuwei Huang

Auto-Submit+1
Commit-Queue+1

1 comment

Patchset-level comments
File-level comment, Patchset 2 (Latest):
Yuwei Huang . resolved

PTAL thanks!

Open in Gerrit

Related details

Attention is currently required from:
  • Lambros Lambrou
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I631b91103674b55890ce572c19f0b860e466956c
Gerrit-Change-Number: 7573298
Gerrit-PatchSet: 2
Gerrit-Owner: Yuwei Huang <yuw...@chromium.org>
Gerrit-Reviewer: Lambros Lambrou <lambros...@chromium.org>
Gerrit-Reviewer: Yuwei Huang <yuw...@chromium.org>
Gerrit-Attention: Lambros Lambrou <lambros...@chromium.org>
Gerrit-Comment-Date: Fri, 13 Feb 2026 02:27:40 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Lambros Lambrou (Gerrit)

unread,
Feb 12, 2026, 10:19:28 PM (9 days ago) Feb 12
to Yuwei Huang, Chromium LUCI CQ, chromium...@chromium.org, chromotin...@chromium.org

Lambros Lambrou voted and added 3 comments

Votes added by Lambros Lambrou

Code-Review+1

3 comments

File remoting/host/desktop_process_main.cc
Line 12, Patchset 2:#include "base/files/file_descriptor_watcher_posix.h"
Lambros Lambrou . unresolved

Should this be in a platform-specific section?

Line 92, Patchset 2: std::unique_ptr<base::FileDescriptorWatcher> file_descriptor_watcher;
file_descriptor_watcher = std::make_unique<base::FileDescriptorWatcher>(
io_task_runner->task_runner());
Lambros Lambrou . unresolved

It looks like you want to create and destroy a temporary object on this thread. Can this be simplified to:

```suggestion
  std::make_unique<base::FileDescriptorWatcher>(
      io_task_runner->task_runner());
```

or maybe create it on the stack:

```suggestion
  base::FileDescriptorWatcher fd_watcher(
     io_task_runner->task_runner());
```
File remoting/host/linux/gnome_remote_desktop_session.cc
Line 97, Patchset 2: return std::ranges::find(xdg_current_desktop_values, "GNOME") !=
xdg_current_desktop_values.end();
Lambros Lambrou . unresolved
```suggestion
  return std::ranges::contains(xdg_current_desktop_values, "GNOME");
```
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I631b91103674b55890ce572c19f0b860e466956c
    Gerrit-Change-Number: 7573298
    Gerrit-PatchSet: 2
    Gerrit-Owner: Yuwei Huang <yuw...@chromium.org>
    Gerrit-Reviewer: Lambros Lambrou <lambros...@chromium.org>
    Gerrit-Reviewer: Yuwei Huang <yuw...@chromium.org>
    Gerrit-Comment-Date: Fri, 13 Feb 2026 03:19:16 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Lambros Lambrou (Gerrit)

    unread,
    Feb 12, 2026, 10:21:33 PM (9 days ago) Feb 12
    to Yuwei Huang, Chromium LUCI CQ, chromium...@chromium.org, chromotin...@chromium.org
    Attention needed from Yuwei Huang

    Lambros Lambrou voted and added 1 comment

    Votes added by Lambros Lambrou

    Code-Review+1

    1 comment

    File remoting/host/desktop_process_main.cc
    Line 12, Patchset 2:#include "base/files/file_descriptor_watcher_posix.h"
    Lambros Lambrou . resolved

    Should this be in a platform-specific section?

    Lambros Lambrou

    Ah, you've fixed this already 😊

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Yuwei Huang
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    • requirement satisfiedReview-Enforcement
    Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
    Gerrit-MessageType: comment
    Gerrit-Project: chromium/src
    Gerrit-Branch: main
    Gerrit-Change-Id: I631b91103674b55890ce572c19f0b860e466956c
    Gerrit-Change-Number: 7573298
    Gerrit-PatchSet: 3
    Gerrit-Owner: Yuwei Huang <yuw...@chromium.org>
    Gerrit-Reviewer: Lambros Lambrou <lambros...@chromium.org>
    Gerrit-Reviewer: Yuwei Huang <yuw...@chromium.org>
    Gerrit-Attention: Yuwei Huang <yuw...@chromium.org>
    Gerrit-Comment-Date: Fri, 13 Feb 2026 03:21:21 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Lambros Lambrou <lambros...@chromium.org>
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Yuwei Huang (Gerrit)

    unread,
    Feb 13, 2026, 3:06:45 AM (9 days ago) Feb 13
    to Lambros Lambrou, Chromium LUCI CQ, chromium...@chromium.org, chromotin...@chromium.org

    Yuwei Huang voted and added 3 comments

    Votes added by Yuwei Huang

    Auto-Submit+1
    Commit-Queue+2

    3 comments

    Patchset-level comments
    File-level comment, Patchset 4 (Latest):
    Yuwei Huang . resolved

    Thanks!

    File remoting/host/desktop_process_main.cc
    Line 92, Patchset 2: std::unique_ptr<base::FileDescriptorWatcher> file_descriptor_watcher;
    file_descriptor_watcher = std::make_unique<base::FileDescriptorWatcher>(
    io_task_runner->task_runner());
    Lambros Lambrou . resolved

    It looks like you want to create and destroy a temporary object on this thread. Can this be simplified to:

    ```suggestion
      std::make_unique<base::FileDescriptorWatcher>(
          io_task_runner->task_runner());
    ```

    or maybe create it on the stack:

    ```suggestion
      base::FileDescriptorWatcher fd_watcher(
         io_task_runner->task_runner());
    ```
    Yuwei Huang

    Technically speaking it's not "temporary", since it needs to be alive as long as the RunLoop is running. Creating the watcher on the stack works, so I just made it this way.

    File remoting/host/linux/gnome_remote_desktop_session.cc
    Line 97, Patchset 2: return std::ranges::find(xdg_current_desktop_values, "GNOME") !=
    xdg_current_desktop_values.end();
    Lambros Lambrou . resolved
    ```suggestion
      return std::ranges::contains(xdg_current_desktop_values, "GNOME");
    ```
    Yuwei Huang

    Done. That's much better :)

    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement satisfiedCode-Owners
      • requirement satisfiedCode-Review
      • requirement satisfiedReview-Enforcement
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: comment
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I631b91103674b55890ce572c19f0b860e466956c
      Gerrit-Change-Number: 7573298
      Gerrit-PatchSet: 4
      Gerrit-Owner: Yuwei Huang <yuw...@chromium.org>
      Gerrit-Reviewer: Lambros Lambrou <lambros...@chromium.org>
      Gerrit-Reviewer: Yuwei Huang <yuw...@chromium.org>
      Gerrit-Comment-Date: Fri, 13 Feb 2026 08:06:34 +0000
      satisfied_requirement
      open
      diffy

      Chromium LUCI CQ (Gerrit)

      unread,
      Feb 13, 2026, 3:19:20 AM (9 days ago) Feb 13
      to Yuwei Huang, Lambros Lambrou, chromium...@chromium.org, chromotin...@chromium.org

      Chromium LUCI CQ submitted the change with unreviewed changes

      Unreviewed changes

      3 is the latest approved patch-set.
      The change was submitted with unreviewed changes in the following files:

      ```
      The name of the file: remoting/host/linux/gnome_remote_desktop_session.cc
      Insertions: 1, Deletions: 2.

      @@ -94,8 +94,7 @@
         auto xdg_current_desktop_values = base::SplitString(
             xdg_current_desktop, ":", base::WhitespaceHandling::TRIM_WHITESPACE,
             base::SplitResult::SPLIT_WANT_NONEMPTY);
      -  return std::ranges::find(xdg_current_desktop_values, "GNOME") !=
      -         xdg_current_desktop_values.end();
      +  return std::ranges::contains(xdg_current_desktop_values, "GNOME");
       }
       
       // static
      ```
      ```
      The name of the file: remoting/host/desktop_process_main.cc
      Insertions: 1, Deletions: 3.

      @@ -93,9 +93,7 @@
         // Allow the main thread (which is not an I/O thread) to use
         // FileDescriptorWatcher. The constructor of FileDescriptorWatcher registers
         // itself in a thread local storage.
      -  std::unique_ptr<base::FileDescriptorWatcher> file_descriptor_watcher;
      -  file_descriptor_watcher = std::make_unique<base::FileDescriptorWatcher>(
      -      io_task_runner->task_runner());
      +  base::FileDescriptorWatcher fd_watcher(io_task_runner->task_runner());
       #endif
       
         mojo::core::ScopedIPCSupport ipc_support(
      ```

      Change information

      Commit message:
      [crd host][linux] Fix GnomeRemoteDesktopSession in multi-process host

      1. XDG_CURRENT_DESKTOP is a colon-separated list, and for the greeter
         session, it turns out to be `GNOME-Greeter:GNOME`, so this CL fixes
         the logic checking XDG_CURRENT_DESKTOP.
      2. EiSenderSession uses FileDescriptorWatcher to watch for the EIS FD.
         However, it seems to be run on the main UI thread, which does not
         have an associated FileDescriptorWatcher, causing segfault. This CL
         fixes it.
      3. GDM does not launch the GNOME shell with the `--headless` flag, so
         GnomeHeadlessDetector doesn't work. However, given the desktop
         process in multi-process mode will always be run in a GDM remote
         display, we can assume that the session is always headless for now.
      Bug: 475611769
      Change-Id: I631b91103674b55890ce572c19f0b860e466956c
      Reviewed-by: Lambros Lambrou <lambros...@chromium.org>
      Auto-Submit: Yuwei Huang <yuw...@chromium.org>
      Commit-Queue: Yuwei Huang <yuw...@chromium.org>
      Cr-Commit-Position: refs/heads/main@{#1584483}
      Files:
      • M remoting/host/desktop_process_main.cc
      • M remoting/host/linux/gnome_remote_desktop_session.cc
      Change size: S
      Delta: 2 files changed, 32 insertions(+), 1 deletion(-)
      Branch: refs/heads/main
      Submit Requirements:
      • requirement satisfiedCode-Review: +1 by Lambros Lambrou
      Open in Gerrit
      Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
      Gerrit-MessageType: merged
      Gerrit-Project: chromium/src
      Gerrit-Branch: main
      Gerrit-Change-Id: I631b91103674b55890ce572c19f0b860e466956c
      Gerrit-Change-Number: 7573298
      Gerrit-PatchSet: 5
      Gerrit-Owner: Yuwei Huang <yuw...@chromium.org>
      Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
      Gerrit-Reviewer: Lambros Lambrou <lambros...@chromium.org>
      Gerrit-Reviewer: Yuwei Huang <yuw...@chromium.org>
      open
      diffy
      satisfied_requirement
      Reply all
      Reply to author
      Forward
      0 new messages