Add Source-Specific Multicast (SSM) support to net/socket layer [chromium/src : main]

0 views
Skip to first unread message

Omar Ramadan (Gerrit)

unread,
Nov 21, 2025, 5:37:43 PM11/21/25
to chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Reilly Grant, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
Attention needed from Harald Alvestrand, Reilly Grant and Vlad Krot

Omar Ramadan added 3 comments

Patchset-level comments
File-level comment, Patchset 19:
Omar Ramadan . resolved

thanks @vk...@google.com for guidance. i've reduce the scope here to net/socket layer and moved the rest to CL7186747

Commit Message
Line 7, Patchset 10:Add Source-Specific Multicast (SSM) support to Direct Sockets API
Vlad Krot . resolved

Attach link to chrome entry for other reviewers.

Omar Ramadan

Done

File net/socket/udp_socket_unittest.cc
Line 2138, Patchset 10:TEST_F(UDPSocketTest, JoinSourceGroupIPv4) {
Vlad Krot . resolved

I would split the CL into multiple. The /net/socket/ directory should be in a separate CL from other layers to simplify job of the reviewers.

Omar Ramadan

Done

Open in Gerrit

Related details

Attention is currently required from:
  • Harald Alvestrand
  • Reilly Grant
  • Vlad Krot
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is blockingCode-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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
Gerrit-Change-Number: 7160820
Gerrit-PatchSet: 20
Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
Gerrit-Reviewer: Vlad Krot <vk...@google.com>
Gerrit-CC: Andrew Rayskiy <green...@google.com>
Gerrit-CC: Kentaro Hara <har...@chromium.org>
Gerrit-CC: Mike Taylor <mike...@chromium.org>
Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
Gerrit-CC: Simon Hangl <sim...@google.com>
Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
Gerrit-Attention: Reilly Grant <rei...@chromium.org>
Gerrit-Attention: Vlad Krot <vk...@google.com>
Gerrit-Comment-Date: Fri, 21 Nov 2025 22:37:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Vlad Krot <vk...@google.com>
satisfied_requirement
unsatisfied_requirement
blocking_requirement
open
diffy

Omar Ramadan (Gerrit)

unread,
Nov 21, 2025, 5:45:46 PM11/21/25
to chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Reilly Grant, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
Attention needed from Harald Alvestrand, Reilly Grant and Vlad Krot

Omar Ramadan added 2 comments

File third_party/blink/public/mojom/direct_sockets/direct_sockets.mojom
Line 113, Patchset 10:// Result codes for Direct Sockets operations.
Vlad Krot . resolved

unused?

Omar Ramadan

Done

File third_party/blink/renderer/modules/direct_sockets/multicast_controller.h
Line 31, Patchset 10: public:
Vlad Krot . resolved

If blink layer is modified, then it must be tested.
First of all unit tests: third_party/blink/renderer/modules/direct_sockets/multicast_controller_unittest.cc

Second, e2e tests on chrome or content layer:
content/browser/direct_sockets/direct_sockets_udp_browsertest.cc,
chrome/browser/direct_sockets/direct_sockets_apitest.cc

There should be complete cases, checking that source filtering works, and many other corner cases.

Omar Ramadan

addressed in CL7186747

Open in Gerrit

Related details

Attention is currently required from:
  • Harald Alvestrand
  • Reilly Grant
  • Vlad Krot
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is blockingCode-Review
    Gerrit-Comment-Date: Fri, 21 Nov 2025 22:45:27 +0000
    satisfied_requirement
    unsatisfied_requirement
    blocking_requirement
    open
    diffy

    Reilly Grant (Gerrit)

    unread,
    Nov 21, 2025, 6:26:29 PM11/21/25
    to Omar Ramadan, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Reilly Grant, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
    Attention needed from Harald Alvestrand, Omar Ramadan and Vlad Krot

    Reilly Grant added 2 comments

    File net/socket/udp_socket_posix.cc
    Line 1063, Patchset 20 (Latest): struct ip_mreq_source mreqs = {};
    Reilly Grant . unresolved

    Can you define helper functions to populate a `struct ip_mreq_source` or `struct group_source_req` from an `IPAddress` that we can use in both the join and leave methods?

    File net/socket/udp_socket_win.cc
    Line 1408, Patchset 20 (Latest): std::memcpy(&group->sin6_addr, group_address.bytes().data(),
    Reilly Grant . unresolved

    Why `memcpy` here instead of using `ToIn6Addr`?

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Harald Alvestrand
    • Omar Ramadan
    • Vlad Krot
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not satisfiedCode-Owners
      • requirement is blockingCode-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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
      Gerrit-Change-Number: 7160820
      Gerrit-PatchSet: 20
      Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
      Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
      Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
      Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
      Gerrit-Reviewer: Vlad Krot <vk...@google.com>
      Gerrit-CC: Andrew Rayskiy <green...@google.com>
      Gerrit-CC: Kentaro Hara <har...@chromium.org>
      Gerrit-CC: Mike Taylor <mike...@chromium.org>
      Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
      Gerrit-CC: Simon Hangl <sim...@google.com>
      Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
      Gerrit-Attention: Omar Ramadan <om...@blockcast.net>
      Gerrit-Attention: Vlad Krot <vk...@google.com>
      Gerrit-Comment-Date: Fri, 21 Nov 2025 23:26:21 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: No
      satisfied_requirement
      unsatisfied_requirement
      blocking_requirement
      open
      diffy

      Omar Ramadan (Gerrit)

      unread,
      Nov 21, 2025, 8:29:10 PM11/21/25
      to chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Reilly Grant, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
      Attention needed from Harald Alvestrand, Reilly Grant and Vlad Krot

      Omar Ramadan added 2 comments

      File net/socket/udp_socket_posix.cc
      Line 1063, Patchset 20: struct ip_mreq_source mreqs = {};
      Reilly Grant . resolved

      Can you define helper functions to populate a `struct ip_mreq_source` or `struct group_source_req` from an `IPAddress` that we can use in both the join and leave methods?

      Omar Ramadan

      added `CreateIPv4SourceGroupRequest` / `CreateIPv6SourceGroupRequest` helpers

      File net/socket/udp_socket_win.cc
      Line 1408, Patchset 20: std::memcpy(&group->sin6_addr, group_address.bytes().data(),
      Reilly Grant . resolved

      Why `memcpy` here instead of using `ToIn6Addr`?

      Omar Ramadan

      Done

      Open in Gerrit

      Related details

      Attention is currently required from:
      • Harald Alvestrand
      • Reilly Grant
      • Vlad Krot
      Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not satisfiedCode-Owners
        • requirement is blockingCode-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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
        Gerrit-Change-Number: 7160820
        Gerrit-PatchSet: 21
        Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
        Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
        Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
        Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
        Gerrit-Reviewer: Vlad Krot <vk...@google.com>
        Gerrit-CC: Andrew Rayskiy <green...@google.com>
        Gerrit-CC: Kentaro Hara <har...@chromium.org>
        Gerrit-CC: Mike Taylor <mike...@chromium.org>
        Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
        Gerrit-CC: Simon Hangl <sim...@google.com>
        Gerrit-Attention: Reilly Grant <rei...@chromium.org>
        Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
        Gerrit-Attention: Vlad Krot <vk...@google.com>
        Gerrit-Comment-Date: Sat, 22 Nov 2025 01:28:51 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Reilly Grant <rei...@chromium.org>
        satisfied_requirement
        unsatisfied_requirement
        blocking_requirement
        open
        diffy

        Reilly Grant (Gerrit)

        unread,
        Nov 21, 2025, 8:42:59 PM11/21/25
        to Omar Ramadan, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Reilly Grant, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
        Attention needed from Harald Alvestrand, Omar Ramadan and Vlad Krot

        Reilly Grant added 1 comment

        File net/socket/udp_socket_win.cc
        Line 1374, Patchset 22 (Latest): struct ip_mreq_source mreqs;
        Reilly Grant . unresolved

        Add struct building helpers here as well?

        Open in Gerrit

        Related details

        Attention is currently required from:
        • Harald Alvestrand
        • Omar Ramadan
        • Vlad Krot
        Submit Requirements:
          • requirement satisfiedCode-Coverage
          • requirement is not satisfiedCode-Owners
          • requirement is blockingCode-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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
          Gerrit-Change-Number: 7160820
          Gerrit-PatchSet: 22
          Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
          Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
          Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
          Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
          Gerrit-Reviewer: Vlad Krot <vk...@google.com>
          Gerrit-CC: Andrew Rayskiy <green...@google.com>
          Gerrit-CC: Kentaro Hara <har...@chromium.org>
          Gerrit-CC: Mike Taylor <mike...@chromium.org>
          Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
          Gerrit-CC: Simon Hangl <sim...@google.com>
          Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
          Gerrit-Attention: Omar Ramadan <om...@blockcast.net>
          Gerrit-Attention: Vlad Krot <vk...@google.com>
          Gerrit-Comment-Date: Sat, 22 Nov 2025 01:42:51 +0000
          Gerrit-HasComments: Yes
          Gerrit-Has-Labels: No
          satisfied_requirement
          unsatisfied_requirement
          blocking_requirement
          open
          diffy

          Omar Ramadan (Gerrit)

          unread,
          Nov 21, 2025, 9:07:16 PM11/21/25
          to chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Reilly Grant, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
          Attention needed from Harald Alvestrand, Reilly Grant and Vlad Krot

          Omar Ramadan added 1 comment

          File net/socket/udp_socket_win.cc
          Line 1374, Patchset 22: struct ip_mreq_source mreqs;
          Reilly Grant . resolved

          Add struct building helpers here as well?

          Omar Ramadan

          Done

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Harald Alvestrand
          • Reilly Grant
          • Vlad Krot
          Submit Requirements:
            • requirement satisfiedCode-Coverage
            • requirement is not satisfiedCode-Owners
            • requirement is blockingCode-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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
            Gerrit-Change-Number: 7160820
            Gerrit-PatchSet: 23
            Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
            Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
            Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
            Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
            Gerrit-Reviewer: Vlad Krot <vk...@google.com>
            Gerrit-CC: Andrew Rayskiy <green...@google.com>
            Gerrit-CC: Kentaro Hara <har...@chromium.org>
            Gerrit-CC: Mike Taylor <mike...@chromium.org>
            Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
            Gerrit-CC: Simon Hangl <sim...@google.com>
            Gerrit-Attention: Reilly Grant <rei...@chromium.org>
            Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
            Gerrit-Attention: Vlad Krot <vk...@google.com>
            Gerrit-Comment-Date: Sat, 22 Nov 2025 02:06:57 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: No
            Comment-In-Reply-To: Reilly Grant <rei...@chromium.org>
            satisfied_requirement
            unsatisfied_requirement
            blocking_requirement
            open
            diffy

            Reilly Grant (Gerrit)

            unread,
            Nov 21, 2025, 9:37:18 PM11/21/25
            to Omar Ramadan, Reilly Grant, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
            Attention needed from Harald Alvestrand, Omar Ramadan and Vlad Krot

            Reilly Grant voted and added 1 comment

            Votes added by Reilly Grant

            Code-Review+1

            1 comment

            Patchset-level comments
            Open in Gerrit

            Related details

            Attention is currently required from:
            • Harald Alvestrand
            • Omar Ramadan
            • Vlad Krot
            Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
            Gerrit-Attention: Omar Ramadan <om...@blockcast.net>
            Gerrit-Attention: Vlad Krot <vk...@google.com>
            Gerrit-Comment-Date: Sat, 22 Nov 2025 02:37:03 +0000
            Gerrit-HasComments: Yes
            Gerrit-Has-Labels: Yes
            satisfied_requirement
            unsatisfied_requirement
            blocking_requirement
            open
            diffy

            Vlad Krot (Gerrit)

            unread,
            Nov 24, 2025, 8:08:16 AM11/24/25
            to Omar Ramadan, Reilly Grant, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
            Attention needed from Harald Alvestrand and Omar Ramadan

            Vlad Krot voted and added 1 comment

            Votes added by Vlad Krot

            Code-Review+0

            1 comment

            Patchset-level comments
            Vlad Krot . resolved

            please include /net owners.

            Open in Gerrit

            Related details

            Attention is currently required from:
            • Harald Alvestrand
            • Omar Ramadan
            Submit Requirements:
              • requirement satisfiedCode-Coverage
              • requirement is not 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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
              Gerrit-Change-Number: 7160820
              Gerrit-PatchSet: 23
              Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
              Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
              Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
              Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
              Gerrit-Reviewer: Vlad Krot <vk...@google.com>
              Gerrit-CC: Andrew Rayskiy <green...@google.com>
              Gerrit-CC: Kentaro Hara <har...@chromium.org>
              Gerrit-CC: Mike Taylor <mike...@chromium.org>
              Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
              Gerrit-CC: Simon Hangl <sim...@google.com>
              Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
              Gerrit-Attention: Omar Ramadan <om...@blockcast.net>
              Gerrit-Comment-Date: Mon, 24 Nov 2025 13:07:52 +0000
              Gerrit-HasComments: Yes
              Gerrit-Has-Labels: Yes
              satisfied_requirement
              unsatisfied_requirement
              open
              diffy

              Omar Ramadan (Gerrit)

              unread,
              Nov 24, 2025, 11:26:33 AM11/24/25
              to Adam Rice, Kenichi Ishibashi, Nidhi Jaju, Reilly Grant, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
              Attention needed from Adam Rice, Harald Alvestrand, Kenichi Ishibashi and Nidhi Jaju

              Omar Ramadan added 1 comment

              Patchset-level comments
              Omar Ramadan . resolved

              requesting /net owners review
              thank you!

              Open in Gerrit

              Related details

              Attention is currently required from:
              • Adam Rice
              • Harald Alvestrand
              • Kenichi Ishibashi
              • Nidhi Jaju
              Submit Requirements:
              • requirement satisfiedCode-Coverage
              • requirement is not 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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
              Gerrit-Change-Number: 7160820
              Gerrit-PatchSet: 23
              Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
              Gerrit-Reviewer: Adam Rice <ri...@chromium.org>
              Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
              Gerrit-Reviewer: Kenichi Ishibashi <ba...@chromium.org>
              Gerrit-Reviewer: Nidhi Jaju <nidh...@chromium.org>
              Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
              Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
              Gerrit-Reviewer: Vlad Krot <vk...@google.com>
              Gerrit-CC: Andrew Rayskiy <green...@google.com>
              Gerrit-CC: Kentaro Hara <har...@chromium.org>
              Gerrit-CC: Mike Taylor <mike...@chromium.org>
              Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
              Gerrit-CC: Simon Hangl <sim...@google.com>
              Gerrit-Attention: Kenichi Ishibashi <ba...@chromium.org>
              Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
              Gerrit-Attention: Nidhi Jaju <nidh...@chromium.org>
              Gerrit-Attention: Adam Rice <ri...@chromium.org>
              Gerrit-Comment-Date: Mon, 24 Nov 2025 16:26:09 +0000
              Gerrit-HasComments: Yes
              Gerrit-Has-Labels: No
              satisfied_requirement
              unsatisfied_requirement
              open
              diffy

              Kenichi Ishibashi (Gerrit)

              unread,
              Nov 24, 2025, 9:39:28 PM11/24/25
              to Omar Ramadan, Adam Rice, Nidhi Jaju, Reilly Grant, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
              Attention needed from Adam Rice, Harald Alvestrand, Nidhi Jaju and Omar Ramadan

              Kenichi Ishibashi added 4 comments

              Patchset-level comments
              Kenichi Ishibashi . resolved

              Mostly looking good! Next time, please consider limiting the number of reviewers.

              https://chromium.googlesource.com/chromium/src/+/lkgr/docs/cl_respect.md#choose-your-reviewers

              I think you need two approvals from chromium committers though.

              File net/socket/udp_socket_posix.h
              Line 199, Patchset 23 (Latest): // |group_address| is the multicast group address to join.
              Kenichi Ishibashi . unresolved
              File net/socket/udp_socket_posix.cc
              Line 1084, Patchset 23 (Latest): // Validate that both addresses are the same IP version
              Kenichi Ishibashi . unresolved

              nit: Please add period.

              File net/socket/udp_socket_unittest.cc
              Line 2136, Patchset 23 (Latest):// Tests for Source-Specific Multicast (SSM)
              Kenichi Ishibashi . unresolved

              Can we also have tests to cover not connected cases?

              Open in Gerrit

              Related details

              Attention is currently required from:
              • Adam Rice
              • Harald Alvestrand
              • Nidhi Jaju
              • Omar Ramadan
              Submit Requirements:
                • requirement satisfiedCode-Coverage
                • requirement is not satisfiedCode-Owners
                • requirement is not satisfiedCode-Review
                • requirement is not satisfiedNo-Unresolved-Comments
                Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
                Gerrit-Attention: Omar Ramadan <om...@blockcast.net>
                Gerrit-Attention: Nidhi Jaju <nidh...@chromium.org>
                Gerrit-Attention: Adam Rice <ri...@chromium.org>
                Gerrit-Comment-Date: Tue, 25 Nov 2025 02:39:00 +0000
                Gerrit-HasComments: Yes
                Gerrit-Has-Labels: No
                satisfied_requirement
                unsatisfied_requirement
                open
                diffy

                Omar Ramadan (Gerrit)

                unread,
                Nov 26, 2025, 12:04:46 PM11/26/25
                to Adam Rice, Kenichi Ishibashi, Nidhi Jaju, Reilly Grant, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
                Attention needed from Adam Rice, Harald Alvestrand, Kenichi Ishibashi, Nidhi Jaju and Reilly Grant

                Omar Ramadan added 4 comments

                Patchset-level comments
                File-level comment, Patchset 23:
                Omar Ramadan . resolved

                thanks @ba...@chromium.org for the review and guidance. first time contributor so apologies for the large number of reviewers tagged

                File net/socket/udp_socket_posix.h
                Line 199, Patchset 23: // |group_address| is the multicast group address to join.
                Kenichi Ishibashi . resolved
                Omar Ramadan

                Done

                File net/socket/udp_socket_posix.cc
                Line 1084, Patchset 23: // Validate that both addresses are the same IP version
                Kenichi Ishibashi . resolved

                nit: Please add period.

                Omar Ramadan

                Done

                File net/socket/udp_socket_unittest.cc
                Line 2136, Patchset 23:// Tests for Source-Specific Multicast (SSM)
                Kenichi Ishibashi . resolved

                Can we also have tests to cover not connected cases?

                Omar Ramadan

                Done

                Open in Gerrit

                Related details

                Attention is currently required from:
                • Adam Rice
                • Harald Alvestrand
                • Kenichi Ishibashi
                • Nidhi Jaju
                • Reilly Grant
                Submit Requirements:
                  • requirement satisfiedCode-Coverage
                  • requirement is not 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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
                  Gerrit-Change-Number: 7160820
                  Gerrit-PatchSet: 24
                  Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
                  Gerrit-Reviewer: Adam Rice <ri...@chromium.org>
                  Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
                  Gerrit-Reviewer: Kenichi Ishibashi <ba...@chromium.org>
                  Gerrit-Reviewer: Nidhi Jaju <nidh...@chromium.org>
                  Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
                  Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
                  Gerrit-Reviewer: Vlad Krot <vk...@google.com>
                  Gerrit-CC: Andrew Rayskiy <green...@google.com>
                  Gerrit-CC: Kentaro Hara <har...@chromium.org>
                  Gerrit-CC: Mike Taylor <mike...@chromium.org>
                  Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
                  Gerrit-CC: Simon Hangl <sim...@google.com>
                  Gerrit-Attention: Kenichi Ishibashi <ba...@chromium.org>
                  Gerrit-Attention: Reilly Grant <rei...@chromium.org>
                  Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
                  Gerrit-Attention: Nidhi Jaju <nidh...@chromium.org>
                  Gerrit-Attention: Adam Rice <ri...@chromium.org>
                  Gerrit-Comment-Date: Wed, 26 Nov 2025 17:04:28 +0000
                  Gerrit-HasComments: Yes
                  Gerrit-Has-Labels: No
                  Comment-In-Reply-To: Kenichi Ishibashi <ba...@chromium.org>
                  satisfied_requirement
                  unsatisfied_requirement
                  open
                  diffy

                  Kenichi Ishibashi (Gerrit)

                  unread,
                  Nov 26, 2025, 7:22:12 PM11/26/25
                  to Omar Ramadan, Adam Rice, Nidhi Jaju, Reilly Grant, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
                  Attention needed from Adam Rice, Harald Alvestrand, Nidhi Jaju, Omar Ramadan and Reilly Grant

                  Kenichi Ishibashi added 1 comment

                  Patchset-level comments
                  File-level comment, Patchset 24 (Latest):
                  Kenichi Ishibashi . resolved

                  looking good, running trybots before approval.

                  Open in Gerrit

                  Related details

                  Attention is currently required from:
                  • Adam Rice
                  • Harald Alvestrand
                  • Nidhi Jaju
                  • Omar Ramadan
                  • Reilly Grant
                  Gerrit-Attention: Reilly Grant <rei...@chromium.org>
                  Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
                  Gerrit-Attention: Omar Ramadan <om...@blockcast.net>
                  Gerrit-Attention: Nidhi Jaju <nidh...@chromium.org>
                  Gerrit-Attention: Adam Rice <ri...@chromium.org>
                  Gerrit-Comment-Date: Thu, 27 Nov 2025 00:21:37 +0000
                  Gerrit-HasComments: Yes
                  Gerrit-Has-Labels: No
                  satisfied_requirement
                  unsatisfied_requirement
                  open
                  diffy

                  Omar Ramadan (Gerrit)

                  unread,
                  Nov 26, 2025, 7:32:40 PM11/26/25
                  to Kenichi Ishibashi, Adam Rice, Nidhi Jaju, Reilly Grant, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
                  Attention needed from Adam Rice, Harald Alvestrand, Kenichi Ishibashi, Nidhi Jaju and Reilly Grant

                  Omar Ramadan added 1 comment

                  Patchset-level comments
                  Omar Ramadan . resolved

                  thanks -- keeping an eye on the test run results

                  Open in Gerrit

                  Related details

                  Attention is currently required from:
                  • Adam Rice
                  • Harald Alvestrand
                  • Kenichi Ishibashi
                  • Nidhi Jaju
                  • Reilly Grant
                  Gerrit-Attention: Kenichi Ishibashi <ba...@chromium.org>
                  Gerrit-Attention: Reilly Grant <rei...@chromium.org>
                  Gerrit-Attention: Harald Alvestrand <h...@chromium.org>
                  Gerrit-Attention: Nidhi Jaju <nidh...@chromium.org>
                  Gerrit-Attention: Adam Rice <ri...@chromium.org>
                  Gerrit-Comment-Date: Thu, 27 Nov 2025 00:32:20 +0000
                  Gerrit-HasComments: Yes
                  Gerrit-Has-Labels: No
                  satisfied_requirement
                  unsatisfied_requirement
                  open
                  diffy

                  Kenichi Ishibashi (Gerrit)

                  unread,
                  Nov 26, 2025, 8:40:56 PM11/26/25
                  to Omar Ramadan, Adam Rice, Nidhi Jaju, Reilly Grant, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Andrew Rayskiy, Simon Hangl, Harald Alvestrand, Chromium LUCI CQ, AyeAye, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
                  Attention needed from Omar Ramadan

                  Kenichi Ishibashi added 1 comment

                  Patchset-level comments
                  File-level comment, Patchset 25 (Latest):
                  Kenichi Ishibashi . resolved

                  Can you fix the test failure? Thanks!

                  Open in Gerrit

                  Related details

                  Attention is currently required from:
                  • Omar Ramadan
                  Submit Requirements:
                  • requirement satisfiedCode-Coverage
                  • requirement is not 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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
                  Gerrit-Change-Number: 7160820
                  Gerrit-PatchSet: 25
                  Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
                  Gerrit-Reviewer: Adam Rice <ri...@chromium.org>
                  Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
                  Gerrit-Reviewer: Kenichi Ishibashi <ba...@chromium.org>
                  Gerrit-Reviewer: Nidhi Jaju <nidh...@chromium.org>
                  Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
                  Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
                  Gerrit-Reviewer: Vlad Krot <vk...@google.com>
                  Gerrit-CC: Andrew Rayskiy <green...@google.com>
                  Gerrit-CC: Kentaro Hara <har...@chromium.org>
                  Gerrit-CC: Mike Taylor <mike...@chromium.org>
                  Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
                  Gerrit-CC: Simon Hangl <sim...@google.com>
                  Gerrit-Attention: Omar Ramadan <om...@blockcast.net>
                  Gerrit-Comment-Date: Thu, 27 Nov 2025 01:40:21 +0000
                  Gerrit-HasComments: Yes
                  Gerrit-Has-Labels: No
                  satisfied_requirement
                  unsatisfied_requirement
                  open
                  diffy

                  Adam Rice (Gerrit)

                  unread,
                  Jan 8, 2026, 4:16:52 AM (8 days ago) Jan 8
                  to Omar Ramadan, Andrew Rayskiy, Kenichi Ishibashi, Nidhi Jaju, Reilly Grant, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Simon Hangl, Harald Alvestrand, Chromium LUCI CQ, AyeAye, fenced-fra...@chromium.org, asvitki...@chromium.org, bnc+...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
                  Attention needed from Omar Ramadan

                  Adam Rice added 4 comments

                  Patchset-level comments
                  File-level comment, Patchset 45 (Latest):
                  Adam Rice . resolved

                  Sorry for the slow review!

                  Commit Message
                  Line 32, Patchset 45 (Latest):Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7160820
                  Adam Rice . unresolved

                  You don't need to add the Reviewed-on field yourself, Gerrit does it automatically when the change is submitted.

                  File net/socket/udp_socket_unittest.cc
                  Line 2137, Patchset 45 (Latest):
                  Adam Rice . unresolved
                  I suggest you define a constant like
                  ```
                  #if defined(MCAST_LEAVE_SOURCE_GROUP) && !BUILDFLAG(IS_ANDROID) && \
                      !BUILDFLAG(IS_IOS)
                  constexpr bool kExpectSSMToWork = true;
                  #else
                  constexpr bool kExpectSSMToWork = false;
                  #endif
                  ```
                  and then use `kExpectSSMToWork` in the tests to ensure that ERR_NOT_IMPLEMENTED is only returned when we expect it to be.
                  Line 2291, Patchset 45 (Latest): // If either failed, platform doesn't support SSM
                  Adam Rice . unresolved

                  They may have failed for some other reason, this test is not reliable.

                  Open in Gerrit

                  Related details

                  Attention is currently required from:
                  • Omar Ramadan
                  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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
                    Gerrit-Change-Number: 7160820
                    Gerrit-PatchSet: 45
                    Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
                    Gerrit-Reviewer: Adam Rice <ri...@chromium.org>
                    Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
                    Gerrit-Reviewer: Kenichi Ishibashi <ba...@chromium.org>
                    Gerrit-Reviewer: Nidhi Jaju <nidh...@chromium.org>
                    Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
                    Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
                    Gerrit-Reviewer: Vlad Krot <vk...@google.com>
                    Gerrit-CC: Andrew Rayskiy <green...@google.com>
                    Gerrit-CC: Kentaro Hara <har...@chromium.org>
                    Gerrit-CC: Mike Taylor <mike...@chromium.org>
                    Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
                    Gerrit-CC: Simon Hangl <sim...@google.com>
                    Gerrit-Attention: Omar Ramadan <om...@blockcast.net>
                    Gerrit-Comment-Date: Thu, 08 Jan 2026 09:16:21 +0000
                    Gerrit-HasComments: Yes
                    Gerrit-Has-Labels: No
                    satisfied_requirement
                    unsatisfied_requirement
                    open
                    diffy

                    Omar Ramadan (Gerrit)

                    unread,
                    Jan 14, 2026, 3:18:39 AM (2 days ago) Jan 14
                    to Adam Rice, Andrew Rayskiy, Kenichi Ishibashi, Nidhi Jaju, Reilly Grant, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Simon Hangl, Harald Alvestrand, Chromium LUCI CQ, AyeAye, fenced-fra...@chromium.org, asvitki...@chromium.org, bnc+...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
                    Attention needed from Adam Rice

                    Omar Ramadan added 4 comments

                    Patchset-level comments
                    File-level comment, Patchset 48 (Latest):
                    Omar Ramadan . resolved

                    No worries at all, thank you for the thorough review!

                      Changes:
                      1. Removed Reviewed-on: field from commit message - Gerrit will add this automatically on submit.
                      2. Added kExpectSSMToWork constant for reliable test behavior:
                      #if defined(MCAST_JOIN_SOURCE_GROUP) && !BUILDFLAG(IS_ANDROID) && \

                          !BUILDFLAG(IS_IOS)
                      constexpr bool kExpectSSMToWork = true;
                      #else
                      constexpr bool kExpectSSMToWork = false;
                      #endif
                      3. Updated SSM tests to use kExpectSSMToWork:
                        - JoinSourceGroupIPv4/JoinSourceGroupIPv6: Assert IsOk() when SSM is expected to work, ERR_NOT_IMPLEMENTED otherwise
                        - JoinSourceGroupMultipleSources/JoinSourceGroupMultipleSourcesIPv6: Use GTEST_SKIP() when SSM not supported; assert all operations succeed when it is (removed unreliable "if failed, platform doesn't support SSM" pattern)
                        - SSMSourceFilteringMultiNIC: Added kExpectSSMToWork skip check
                      Tested locally on Linux/OSX/Windows.
                    Commit Message

                    You don't need to add the Reviewed-on field yourself, Gerrit does it automatically when the change is submitted.

                    Omar Ramadan

                    Done

                    File net/socket/udp_socket_unittest.cc
                    Line 2137, Patchset 45:
                    Adam Rice . resolved
                    I suggest you define a constant like
                    ```
                    #if defined(MCAST_LEAVE_SOURCE_GROUP) && !BUILDFLAG(IS_ANDROID) && \
                        !BUILDFLAG(IS_IOS)
                    constexpr bool kExpectSSMToWork = true;
                    #else
                    constexpr bool kExpectSSMToWork = false;
                    #endif
                    ```
                    and then use `kExpectSSMToWork` in the tests to ensure that ERR_NOT_IMPLEMENTED is only returned when we expect it to be.
                    Omar Ramadan

                    Done

                    Line 2291, Patchset 45: // If either failed, platform doesn't support SSM
                    Adam Rice . resolved

                    They may have failed for some other reason, this test is not reliable.

                    Omar Ramadan

                    Done

                    Open in Gerrit

                    Related details

                    Attention is currently required from:
                    • Adam Rice
                    Submit Requirements:
                      • requirement satisfiedCode-Coverage
                      • requirement is not 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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
                      Gerrit-Change-Number: 7160820
                      Gerrit-PatchSet: 48
                      Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
                      Gerrit-Reviewer: Adam Rice <ri...@chromium.org>
                      Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
                      Gerrit-Reviewer: Kenichi Ishibashi <ba...@chromium.org>
                      Gerrit-Reviewer: Nidhi Jaju <nidh...@chromium.org>
                      Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
                      Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
                      Gerrit-Reviewer: Vlad Krot <vk...@google.com>
                      Gerrit-CC: Andrew Rayskiy <green...@google.com>
                      Gerrit-CC: Kentaro Hara <har...@chromium.org>
                      Gerrit-CC: Mike Taylor <mike...@chromium.org>
                      Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
                      Gerrit-CC: Simon Hangl <sim...@google.com>
                      Gerrit-Attention: Adam Rice <ri...@chromium.org>
                      Gerrit-Comment-Date: Wed, 14 Jan 2026 08:18:28 +0000
                      Gerrit-HasComments: Yes
                      Gerrit-Has-Labels: No
                      Comment-In-Reply-To: Adam Rice <ri...@chromium.org>
                      satisfied_requirement
                      unsatisfied_requirement
                      open
                      diffy

                      Adam Rice (Gerrit)

                      unread,
                      7:17 AM (7 hours ago) 7:17 AM
                      to Omar Ramadan, Andrew Rayskiy, Kenichi Ishibashi, Nidhi Jaju, Reilly Grant, chromium...@chromium.org, Raphael Kubo da Costa, Kentaro Hara, Vlad Krot, Mike Taylor, Simon Hangl, Harald Alvestrand, Chromium LUCI CQ, AyeAye, fenced-fra...@chromium.org, asvitki...@chromium.org, bnc+...@chromium.org, ipc-securi...@chromium.org, blink-revie...@chromium.org, rmcelra...@chromium.org, jmedle...@chromium.org, blink-...@chromium.org, kinuko...@chromium.org, blink-re...@chromium.org, network-ser...@chromium.org, net-r...@chromium.org
                      Attention needed from Omar Ramadan

                      Adam Rice added 8 comments

                      File net/socket/udp_socket_posix.h
                      Line 198, Patchset 49 (Latest): // Joins a source-specific multicast group. Returns a net error code.
                      Adam Rice . unresolved

                      Why did you remove the extra information from the comment? The implementation details might have been unnecessary, but the description of parameters and reference to the RFC were useful.

                      File net/socket/udp_socket_posix.cc
                      Line 90, Patchset 49 (Latest):uint32_t GetInterfaceViaRoute(const IPAddress& source_address) {
                      Adam Rice . unresolved

                      "GetInterfaceForDestination()" might be a better name.

                      Line 92, Patchset 49 (Latest): int sock = socket(family, SOCK_DGRAM, 0);
                      Adam Rice . unresolved

                      You should locally create a `net::UDPSocketPosix` object instead of doing OS calls directly.

                      Line 115, Patchset 49 (Latest): if (getifaddrs(&ifap) != 0) {
                      Adam Rice . unresolved

                      Use `net::GetNetworkList(&networks, INCLUDE_HOST_SCOPE_VIRTUAL_INTERFACES)` instead of calling `getifaddrs()` directly.

                      Line 1100, Patchset 45: // Validate that both addresses are the same IP version.
                      Adam Rice . unresolved

                      Why did you remove this comment? I found it useful.

                      Line 1162, Patchset 49 (Latest):#if BUILDFLAG(IS_ANDROID) || BUILDFLAG(IS_IOS)
                      Adam Rice . unresolved

                      This whole chunk of code seems to be identical to a chunk in LeaveSourceGroup(), with the only difference being whether we pass `MCAST_JOIN_SOURCE_GROUP` or `MCAST_LEAVE_SOURCE_GROUP`. So you can factor it out into a method that takes the option as an argument, and call the new method from both places.

                      File net/socket/udp_socket_unittest.cc
                      Line 2359, Patchset 49 (Latest): if (!iface.address.IsLoopback()) {
                      Adam Rice . unresolved

                      Loopback is already skipped by GetNetworkList(), so this check is unnecessary. Which means you can avoid the loop and just `return interfaces.size() >= 2`.

                      File net/socket/udp_socket_win.cc
                      Line 1406, Patchset 49 (Latest): if (group_address.IsIPv4()) {
                      Adam Rice . unresolved

                      You can reduce code duplication here by defining a function
                      `CreateSourceGroupRequest()` which calls one of `CreateIPv4SourceGroupRequest()` or `CreateIPv6SourceGroupRequest()` depending on the value of `group_address.IsIPv4()`. Then the only difference between the IPv4 case and the IPv6 case is whether you pass `IPPROTO_IP` or `IPPROTO_IPV6` to `setsocketopt()`, which you can just write like `group_address.IsIPv4() ? IPPROTO_IP : IPPROTO_IPV6`.

                      You can also factor out the common code with LeaveSourceGroup() as I described for the POSIX version of the code.

                      Open in Gerrit

                      Related details

                      Attention is currently required from:
                      • Omar Ramadan
                      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: Id4f2cb41f7620ba589ff86bc5e6946486dd624c8
                        Gerrit-Change-Number: 7160820
                        Gerrit-PatchSet: 49
                        Gerrit-Owner: Omar Ramadan <om...@blockcast.net>
                        Gerrit-Reviewer: Adam Rice <ri...@chromium.org>
                        Gerrit-Reviewer: Harald Alvestrand <h...@chromium.org>
                        Gerrit-Reviewer: Kenichi Ishibashi <ba...@chromium.org>
                        Gerrit-Reviewer: Nidhi Jaju <nidh...@chromium.org>
                        Gerrit-Reviewer: Omar Ramadan <om...@blockcast.net>
                        Gerrit-Reviewer: Reilly Grant <rei...@chromium.org>
                        Gerrit-Reviewer: Vlad Krot <vk...@google.com>
                        Gerrit-CC: Andrew Rayskiy <green...@google.com>
                        Gerrit-CC: Kentaro Hara <har...@chromium.org>
                        Gerrit-CC: Mike Taylor <mike...@chromium.org>
                        Gerrit-CC: Raphael Kubo da Costa <ku...@igalia.com>
                        Gerrit-CC: Simon Hangl <sim...@google.com>
                        Gerrit-Attention: Omar Ramadan <om...@blockcast.net>
                        Gerrit-Comment-Date: Fri, 16 Jan 2026 12:17:18 +0000
                        Gerrit-HasComments: Yes
                        Gerrit-Has-Labels: No
                        satisfied_requirement
                        unsatisfied_requirement
                        open
                        diffy
                        Reply all
                        Reply to author
                        Forward
                        0 new messages