[Sharing] Remove SharingMessage delegate type and rename SharingFCMSender [chromium/src : main]

0 views
Skip to first unread message

Hira Mahmood (Gerrit)

unread,
May 1, 2026, 2:04:55 PMMay 1
to plantree, Peter Beverloo, Mahmoud Rashad, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
Attention needed from Mahmoud Rashad and plantree

Hira Mahmood added 2 comments

File components/sharing_message/sharing_fcm_sender.h
Line 48, Patchset 3:class SharingFCMSender : public syncer::SyncServiceObserver {
Hira Mahmood . unresolved

This class should be renamed. It's confusing for it to be called SharingFCMSender when it also handles iOS/Chime push messages.

plantree

Good point. Will do.

Hira Mahmood

Not all FCM messages are push notification messages, some are also data messages. WDYT about naming the class SharingChannelSender since the class now handles both FCM and iOS Push channels?

File components/sharing_message/sharing_fcm_sender.cc
Line 103, Patchset 3:void SharingFCMSender::SendUnencryptedMessageToDevice(
Hira Mahmood . resolved

This function assumes the iOS/chime push message type so we should rename it to reflect that now that it's being moved out of SharingIOSPushSender.

Same for `SendEncryptedMessageToDevice` above, it assumes an FCM message (creates and passes in fcm_configuration) so we should rename the function.

plantree

Gotcha. Will update.

Hira Mahmood

Thanks!

Open in Gerrit

Related details

Attention is currently required from:
  • Mahmoud Rashad
  • plantree
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • 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: Ie17764c7ed95269eba2c1867ba921836aaef738e
Gerrit-Change-Number: 7784870
Gerrit-PatchSet: 8
Gerrit-Owner: plantree <pengyu...@microsoft.com>
Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
Gerrit-CC: Peter Beverloo <pe...@chromium.org>
Gerrit-CC: gwsq
Gerrit-Attention: plantree <pengyu...@microsoft.com>
Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
Gerrit-Comment-Date: Fri, 01 May 2026 18:04:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: plantree <pengyu...@microsoft.com>
Comment-In-Reply-To: Hira Mahmood <hiram...@google.com>
satisfied_requirement
unsatisfied_requirement
open
diffy

plantree (Gerrit)

unread,
May 6, 2026, 6:58:42 AMMay 6
to Peter Beverloo, Hira Mahmood, Mahmoud Rashad, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
Attention needed from Hira Mahmood and Mahmoud Rashad

plantree added 1 comment

File components/sharing_message/sharing_fcm_sender.h
Line 48, Patchset 3:class SharingFCMSender : public syncer::SyncServiceObserver {
Hira Mahmood . unresolved

This class should be renamed. It's confusing for it to be called SharingFCMSender when it also handles iOS/Chime push messages.

plantree

Good point. Will do.

Hira Mahmood

Not all FCM messages are push notification messages, some are also data messages. WDYT about naming the class SharingChannelSender since the class now handles both FCM and iOS Push channels?

plantree

Good point — renamed to SharingChannelSender.

Open in Gerrit

Related details

Attention is currently required from:
  • Hira Mahmood
  • Mahmoud Rashad
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • 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: Ie17764c7ed95269eba2c1867ba921836aaef738e
Gerrit-Change-Number: 7784870
Gerrit-PatchSet: 9
Gerrit-Owner: plantree <pengyu...@microsoft.com>
Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
Gerrit-CC: Peter Beverloo <pe...@chromium.org>
Gerrit-CC: gwsq
Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
Gerrit-Attention: Hira Mahmood <hiram...@google.com>
Gerrit-Comment-Date: Wed, 06 May 2026 10:58:33 +0000
satisfied_requirement
unsatisfied_requirement
open
diffy

plantree (Gerrit)

unread,
May 7, 2026, 12:50:38 AMMay 7
to Peter Beverloo, Hira Mahmood, Mahmoud Rashad, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
Attention needed from Hira Mahmood and Mahmoud Rashad

plantree added 1 comment

Patchset-level comments
File-level comment, Patchset 10 (Latest):
plantree . resolved

PTAL, thanks.

Open in Gerrit

Related details

Attention is currently required from:
  • Hira Mahmood
  • Mahmoud Rashad
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • 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: Ie17764c7ed95269eba2c1867ba921836aaef738e
Gerrit-Change-Number: 7784870
Gerrit-PatchSet: 10
Gerrit-Owner: plantree <pengyu...@microsoft.com>
Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
Gerrit-CC: Peter Beverloo <pe...@chromium.org>
Gerrit-CC: gwsq
Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
Gerrit-Attention: Hira Mahmood <hiram...@google.com>
Gerrit-Comment-Date: Thu, 07 May 2026 04:50:11 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Hira Mahmood (Gerrit)

unread,
May 7, 2026, 10:49:52 AMMay 7
to plantree, Peter Beverloo, Mahmoud Rashad, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
Attention needed from Mahmoud Rashad and plantree

Hira Mahmood voted and added 1 comment

Votes added by Hira Mahmood

Code-Review+1

1 comment

Patchset-level comments
Hira Mahmood . resolved

lgtm, thanks!

Open in Gerrit

Related details

Attention is currently required from:
  • Mahmoud Rashad
  • plantree
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • 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: Ie17764c7ed95269eba2c1867ba921836aaef738e
Gerrit-Change-Number: 7784870
Gerrit-PatchSet: 10
Gerrit-Owner: plantree <pengyu...@microsoft.com>
Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
Gerrit-CC: Peter Beverloo <pe...@chromium.org>
Gerrit-CC: gwsq
Gerrit-Attention: plantree <pengyu...@microsoft.com>
Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
Gerrit-Comment-Date: Thu, 07 May 2026 14:49:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

plantree (Gerrit)

unread,
May 7, 2026, 9:00:08 PMMay 7
to Hira Mahmood, Peter Beverloo, Mahmoud Rashad, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
Attention needed from Mahmoud Rashad

plantree added 1 comment

Patchset-level comments
plantree . resolved

Hi @mmra...@google.com, please take a look when you have time. Thanks.

Open in Gerrit

Related details

Attention is currently required from:
  • Mahmoud Rashad
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • 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: Ie17764c7ed95269eba2c1867ba921836aaef738e
Gerrit-Change-Number: 7784870
Gerrit-PatchSet: 10
Gerrit-Owner: plantree <pengyu...@microsoft.com>
Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
Gerrit-CC: Peter Beverloo <pe...@chromium.org>
Gerrit-CC: gwsq
Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
Gerrit-Comment-Date: Fri, 08 May 2026 00:59:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

plantree (Gerrit)

unread,
May 8, 2026, 8:40:54 PMMay 8
to Peter Beverloo, Darryl James, Hira Mahmood, Mahmoud Rashad, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
Attention needed from Darryl James, Mahmoud Rashad and Peter Beverloo

plantree added 1 comment

File components/sharing_message/sharing_fcm_sender.h
Line 48, Patchset 3:class SharingFCMSender : public syncer::SyncServiceObserver {
Hira Mahmood . resolved

This class should be renamed. It's confusing for it to be called SharingFCMSender when it also handles iOS/Chime push messages.

plantree

Good point. Will do.

Hira Mahmood

Not all FCM messages are push notification messages, some are also data messages. WDYT about naming the class SharingChannelSender since the class now handles both FCM and iOS Push channels?

plantree

Good point — renamed to SharingChannelSender.

plantree

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Darryl James
  • Mahmoud Rashad
  • Peter Beverloo
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: Ie17764c7ed95269eba2c1867ba921836aaef738e
    Gerrit-Change-Number: 7784870
    Gerrit-PatchSet: 10
    Gerrit-Owner: plantree <pengyu...@microsoft.com>
    Gerrit-Reviewer: Darryl James <dlj...@chromium.org>
    Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
    Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
    Gerrit-Reviewer: Peter Beverloo <pe...@chromium.org>
    Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
    Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
    Gerrit-Attention: Darryl James <dlj...@chromium.org>
    Gerrit-Attention: Peter Beverloo <pe...@chromium.org>
    Gerrit-Comment-Date: Sat, 09 May 2026 00:40:17 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: No
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Mahmoud Rashad (Gerrit)

    unread,
    May 11, 2026, 10:20:23 AMMay 11
    to plantree, Peter Beverloo, Darryl James, Hira Mahmood, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
    Attention needed from Darryl James, Peter Beverloo and plantree

    Mahmoud Rashad voted and added 2 comments

    Votes added by Mahmoud Rashad

    Code-Review+1

    2 comments

    Patchset-level comments
    Mahmoud Rashad . resolved

    still lgtm % comment, thanks!

    File components/sharing_message/mock_sharing_service.cc
    Line 55, Patchset 10 (Latest): std::make_unique<SharingFCMHandler>(
    Mahmoud Rashad . unresolved
    The parameters and comments here don't match the signature of `SharingFCMHandler`'s constructor. It expects `gcm_driver`, `device_info_tracker`, `sharing_channel_sender`, and `handler_registry`.
    ```suggestion
              std::make_unique<SharingFCMHandler>(
                  /*gcm_driver=*/nullptr,
                  /*device_info_tracker=*/nullptr,
                  /*sharing_channel_sender=*/nullptr,
                  /*handler_registry=*/nullptr),
    ```
    Open in Gerrit

    Related details

    Attention is currently required from:
    • Darryl James
    • Peter Beverloo
    • plantree
    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: Ie17764c7ed95269eba2c1867ba921836aaef738e
      Gerrit-Change-Number: 7784870
      Gerrit-PatchSet: 10
      Gerrit-Owner: plantree <pengyu...@microsoft.com>
      Gerrit-Reviewer: Darryl James <dlj...@chromium.org>
      Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
      Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
      Gerrit-Reviewer: Peter Beverloo <pe...@chromium.org>
      Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
      Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
      Gerrit-CC: gwsq
      Gerrit-Attention: plantree <pengyu...@microsoft.com>
      Gerrit-Attention: Darryl James <dlj...@chromium.org>
      Gerrit-Attention: Peter Beverloo <pe...@chromium.org>
      Gerrit-Comment-Date: Mon, 11 May 2026 14:20:09 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Darryl James (Gerrit)

      unread,
      May 11, 2026, 2:03:08 PMMay 11
      to plantree, Peter Beverloo, Hira Mahmood, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
      Attention needed from Peter Beverloo and plantree

      Darryl James added 1 comment

      Patchset-level comments
      Darryl James . resolved

      Removing myself due to overlapping ownership. Let me know if I am needed on this CL; thanks!

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Peter Beverloo
      • plantree
      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: Ie17764c7ed95269eba2c1867ba921836aaef738e
      Gerrit-Change-Number: 7784870
      Gerrit-PatchSet: 10
      Gerrit-Owner: plantree <pengyu...@microsoft.com>
      Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
      Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
      Gerrit-Reviewer: Peter Beverloo <pe...@chromium.org>
      Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
      Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
      Gerrit-CC: gwsq
      Gerrit-Attention: plantree <pengyu...@microsoft.com>
      Gerrit-Attention: Peter Beverloo <pe...@chromium.org>
      Gerrit-Comment-Date: Mon, 11 May 2026 18:02:53 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      plantree (Gerrit)

      unread,
      May 11, 2026, 10:15:17 PMMay 11
      to Mahmoud Rashad, Peter Beverloo, Hira Mahmood, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
      Attention needed from Hira Mahmood, Mahmoud Rashad and Peter Beverloo

      plantree added 1 comment

      File components/sharing_message/mock_sharing_service.cc
      Line 55, Patchset 10: std::make_unique<SharingFCMHandler>(
      Mahmoud Rashad . unresolved
      The parameters and comments here don't match the signature of `SharingFCMHandler`'s constructor. It expects `gcm_driver`, `device_info_tracker`, `sharing_channel_sender`, and `handler_registry`.
      ```suggestion
                std::make_unique<SharingFCMHandler>(
                    /*gcm_driver=*/nullptr,
                    /*device_info_tracker=*/nullptr,
                    /*sharing_channel_sender=*/nullptr,
                    /*handler_registry=*/nullptr),
      ```
      plantree

      That makes sense and thanks for pointing it out.

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Hira Mahmood
      • Mahmoud Rashad
      • Peter Beverloo
      Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement satisfiedCode-Owners
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • 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: Ie17764c7ed95269eba2c1867ba921836aaef738e
        Gerrit-Change-Number: 7784870
        Gerrit-PatchSet: 12
        Gerrit-Owner: plantree <pengyu...@microsoft.com>
        Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
        Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
        Gerrit-Reviewer: Peter Beverloo <pe...@chromium.org>
        Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
        Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
        Gerrit-CC: gwsq
        Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
        Gerrit-Attention: Hira Mahmood <hiram...@google.com>
        Gerrit-Attention: Peter Beverloo <pe...@chromium.org>
        Gerrit-Comment-Date: Tue, 12 May 2026 02:14:49 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Mahmoud Rashad <mmra...@google.com>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        plantree (Gerrit)

        unread,
        May 13, 2026, 6:48:48 AMMay 13
        to Mahmoud Rashad, Peter Beverloo, Hira Mahmood, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
        Attention needed from Hira Mahmood, Mahmoud Rashad and Peter Beverloo

        plantree added 1 comment

        Patchset-level comments
        File-level comment, Patchset 16 (Latest):
        plantree . resolved

        Have resolved the conflicts, please take a look. Thanks. mmra...@google.com hiram...@google.com

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Hira Mahmood
        • Mahmoud Rashad
        • Peter Beverloo
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement satisfiedCode-Owners
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • 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: Ie17764c7ed95269eba2c1867ba921836aaef738e
        Gerrit-Change-Number: 7784870
        Gerrit-PatchSet: 16
        Gerrit-Owner: plantree <pengyu...@microsoft.com>
        Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
        Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
        Gerrit-Reviewer: Peter Beverloo <pe...@chromium.org>
        Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
        Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
        Gerrit-CC: gwsq
        Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
        Gerrit-Attention: Hira Mahmood <hiram...@google.com>
        Gerrit-Attention: Peter Beverloo <pe...@chromium.org>
        Gerrit-Comment-Date: Wed, 13 May 2026 10:48:23 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        plantree (Gerrit)

        unread,
        May 13, 2026, 6:50:08 AMMay 13
        to Mahmoud Rashad, Peter Beverloo, Hira Mahmood, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
        Attention needed from Hira Mahmood, Mahmoud Rashad and Peter Beverloo

        plantree added 1 comment

        Patchset-level comments
        plantree . resolved

        Have resolved the conflicts, please take a look. Thanks. mmra...@google.com hiram...@google.com

        Gerrit-Comment-Date: Wed, 13 May 2026 10:49:37 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: plantree <pengyu...@microsoft.com>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Hira Mahmood (Gerrit)

        unread,
        May 14, 2026, 11:02:17 AMMay 14
        to plantree, Mahmoud Rashad, Peter Beverloo, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
        Attention needed from Mahmoud Rashad, Peter Beverloo and plantree

        Hira Mahmood voted Code-Review+1

        Code-Review+1
        Open in Gerrit

        Related details

        Attention is currently required from:
        • Mahmoud Rashad
        • Peter Beverloo
        • plantree
        Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement satisfiedCode-Owners
        • requirement is not satisfiedCode-Review
        • requirement is not satisfiedNo-Unresolved-Comments
        • 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: Ie17764c7ed95269eba2c1867ba921836aaef738e
        Gerrit-Change-Number: 7784870
        Gerrit-PatchSet: 16
        Gerrit-Owner: plantree <pengyu...@microsoft.com>
        Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
        Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
        Gerrit-Reviewer: Peter Beverloo <pe...@chromium.org>
        Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
        Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
        Gerrit-CC: gwsq
        Gerrit-Attention: plantree <pengyu...@microsoft.com>
        Gerrit-Attention: Mahmoud Rashad <mmra...@google.com>
        Gerrit-Attention: Peter Beverloo <pe...@chromium.org>
        Gerrit-Comment-Date: Thu, 14 May 2026 15:02:10 +0000
        Gerrit-HasComments: No
        Gerrit-Has-Labels: Yes
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Mahmoud Rashad (Gerrit)

        unread,
        May 15, 2026, 5:05:37 AMMay 15
        to plantree, Hira Mahmood, Peter Beverloo, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
        Attention needed from Peter Beverloo and plantree

        Mahmoud Rashad voted Code-Review+1

        Code-Review+1
        Open in Gerrit

        Related details

        Attention is currently required from:
        • Peter Beverloo
        • plantree
        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: Ie17764c7ed95269eba2c1867ba921836aaef738e
          Gerrit-Change-Number: 7784870
          Gerrit-PatchSet: 16
          Gerrit-Owner: plantree <pengyu...@microsoft.com>
          Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
          Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
          Gerrit-Reviewer: Peter Beverloo <pe...@chromium.org>
          Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
          Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
          Gerrit-CC: gwsq
          Gerrit-Attention: plantree <pengyu...@microsoft.com>
          Gerrit-Attention: Peter Beverloo <pe...@chromium.org>
          Gerrit-Comment-Date: Fri, 15 May 2026 09:05:17 +0000
          Gerrit-HasComments: No
          Gerrit-Has-Labels: Yes
          satisfied_requirement
          unsatisfied_requirement
          open
          diffy

          plantree (Gerrit)

          unread,
          May 17, 2026, 8:53:49 PM (12 days ago) May 17
          to Hira Mahmood, Peter Beverloo, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
          Attention needed from Peter Beverloo

          plantree added 1 comment

          File components/sharing_message/mock_sharing_service.cc
          Line 55, Patchset 10: std::make_unique<SharingFCMHandler>(
          Mahmoud Rashad . resolved
          The parameters and comments here don't match the signature of `SharingFCMHandler`'s constructor. It expects `gcm_driver`, `device_info_tracker`, `sharing_channel_sender`, and `handler_registry`.
          ```suggestion
                    std::make_unique<SharingFCMHandler>(
                        /*gcm_driver=*/nullptr,
                        /*device_info_tracker=*/nullptr,
                        /*sharing_channel_sender=*/nullptr,
                        /*handler_registry=*/nullptr),
          ```
          plantree

          That makes sense and thanks for pointing it out.

          plantree

          Marked as resolved.

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Peter Beverloo
          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: Ie17764c7ed95269eba2c1867ba921836aaef738e
            Gerrit-Change-Number: 7784870
            Gerrit-PatchSet: 16
            Gerrit-Owner: plantree <pengyu...@microsoft.com>
            Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
            Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
            Gerrit-Reviewer: Peter Beverloo <pe...@chromium.org>
            Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
            Gerrit-CC: Chromium Sync Reviews <chromium-s...@google.com>
            Gerrit-CC: gwsq
            Gerrit-Attention: Peter Beverloo <pe...@chromium.org>
            Gerrit-Comment-Date: Mon, 18 May 2026 00:53:21 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            Comment-In-Reply-To: plantree <pengyu...@microsoft.com>
            Comment-In-Reply-To: Mahmoud Rashad <mmra...@google.com>
            satisfied_requirement
            open
            diffy

            plantree (Gerrit)

            unread,
            May 17, 2026, 8:53:51 PM (12 days ago) May 17
            to Hira Mahmood, Peter Beverloo, Chromium Sync Reviews, Chromium LUCI CQ, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org
            Attention needed from Peter Beverloo

            plantree voted Commit-Queue+2

            Commit-Queue+2
            Gerrit-Comment-Date: Mon, 18 May 2026 00:53:25 +0000
            Gerrit-HasComments: No
            Gerrit-Has-Labels: Yes
            satisfied_requirement
            open
            diffy

            Chromium LUCI CQ (Gerrit)

            unread,
            May 17, 2026, 10:23:19 PM (12 days ago) May 17
            to plantree, Mahmoud Rashad, Hira Mahmood, Peter Beverloo, Chromium Sync Reviews, android-bu...@system.gserviceaccount.com, chromium...@chromium.org, zol...@webkit.org, blink-revi...@chromium.org, blink-...@chromium.org, chrome-intell...@chromium.org, penghuan...@chromium.org, webauthn...@chromium.org, cblume...@chromium.org, derinel+wat...@google.com, chrome-intelligence-te...@google.com, devtools...@chromium.org, ios-revie...@chromium.org, ios-r...@chromium.org, knollr+wat...@chromium.org, marq+...@chromium.org

            Chromium LUCI CQ submitted the change

            Change information

            Commit message:
            [Sharing] Remove SharingMessage delegate type and rename SharingFCMSender

            SharingMessageSender previously dispatched message sending through a
            SendMessageDelegate abstraction keyed by a DelegateType enum (kFCM,
            kIOSPush). The abstraction no longer pulls its weight: each delegate
            exposes one method specific to its type and they can all live in a
            single class.

            Merge SharingIOSPushSender into SharingFCMSender so a single class
            handles both encrypted FCM messages and unencrypted iOS/Chime push
            messages, then rename it to SharingChannelSender to reflect the broader
            responsibility. Drop the SendMessageDelegate / DelegateType machinery
            and construct SharingMessageSender with a single owned
            SharingChannelSender instead. Also rename SendEncryptedMessageToDevice /
            SendUnencryptedMessageToDevice to SendFcmMessageToDevice /
            SendIosPushMessageToDevice so the channel they target is explicit.
            Bug: 502086601
            Change-Id: Ie17764c7ed95269eba2c1867ba921836aaef738e
            Reviewed-by: Mahmoud Rashad <mmra...@google.com>
            Reviewed-by: Hira Mahmood <hiram...@google.com>
            Commit-Queue: plantree <pengyu...@microsoft.com>
            Cr-Commit-Position: refs/heads/main@{#1631942}
            Files:
            • M chrome/browser/sharing/click_to_call/click_to_call_utils_unittest.cc
            • M chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler.cc
            • M chrome/browser/sharing/glic_experimental_triggering/glic_experimental_triggering_message_handler_browsertest.cc
            • M chrome/browser/sharing/sharing_service_factory.cc
            • M chrome/browser/ui/desktop_to_mobile_promos/ios_promo_trigger_service.cc
            • M chrome/browser/ui/desktop_to_mobile_promos/ios_promo_trigger_service_unittest.cc
            • M chrome/browser/ui/views/sharing/sharing_browsertest.cc
            • M components/sharing_message/BUILD.gn
            • M components/sharing_message/ack_message_handler_unittest.cc
            • M components/sharing_message/ios_push/BUILD.gn
            • D components/sharing_message/ios_push/sharing_ios_push_sender.cc
            • D components/sharing_message/ios_push/sharing_ios_push_sender.h
            • M components/sharing_message/mock_sharing_message_sender.cc
            • M components/sharing_message/mock_sharing_message_sender.h
            • M components/sharing_message/mock_sharing_service.cc
            • M components/sharing_message/mock_sharing_service.h
            • R components/sharing_message/sharing_channel_sender.cc
            • R components/sharing_message/sharing_channel_sender.h
            • R components/sharing_message/sharing_channel_sender_unittest.cc
            • M components/sharing_message/sharing_fcm_handler.cc
            • M components/sharing_message/sharing_fcm_handler.h
            • M components/sharing_message/sharing_fcm_handler_unittest.cc
            • M components/sharing_message/sharing_message_sender.cc
            • M components/sharing_message/sharing_message_sender.h
            • M components/sharing_message/sharing_message_sender_unittest.cc
            • M components/sharing_message/sharing_service.cc
            • M components/sharing_message/sharing_service.h
            • M components/sharing_message/sharing_service_unittest.cc
            • M ios/chrome/browser/sharing_message/model/ios_sharing_service_factory.mm
            Change size: XL
            Delta: 29 files changed, 357 insertions(+), 742 deletions(-)
            Branch: refs/heads/main
            Submit Requirements:
            • requirement satisfiedCode-Review: +1 by Mahmoud Rashad, +1 by Hira Mahmood
            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: Ie17764c7ed95269eba2c1867ba921836aaef738e
            Gerrit-Change-Number: 7784870
            Gerrit-PatchSet: 17
            Gerrit-Owner: plantree <pengyu...@microsoft.com>
            Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
            Gerrit-Reviewer: Hira Mahmood <hiram...@google.com>
            Gerrit-Reviewer: Mahmoud Rashad <mmra...@google.com>
            Gerrit-Reviewer: Peter Beverloo <pe...@chromium.org>
            Gerrit-Reviewer: plantree <pengyu...@microsoft.com>
            open
            diffy
            satisfied_requirement
            Reply all
            Reply to author
            Forward
            0 new messages