[ImmersivePlayback] Refactor immersive PiP structures to C++ and JNI [chromium/src : main]

0 views
Skip to first unread message

Frank Liberato (Gerrit)

unread,
May 26, 2026, 11:57:07 AM (7 days ago) May 26
to Oleh Desiatyrikov (xWF), Gurmeet Kalra, chromium...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org
Attention needed from Frank Liberato, Gurmeet Kalra and Oleh Desiatyrikov (xWF)

Frank Liberato added 1 comment

Patchset-level comments
File-level comment, Patchset 1 (Latest):
Frank Liberato . resolved

i'm still taking a closer look at this plus the next CL, but the overall structure looks really nice.

-fl

Open in Gerrit

Related details

Attention is currently required from:
  • Frank Liberato
  • Gurmeet Kalra
  • Oleh Desiatyrikov (xWF)
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: Idb1f4f8e95d3cc45c5d322da8fdc120323a2a7cc
Gerrit-Change-Number: 7876303
Gerrit-PatchSet: 1
Gerrit-Owner: Oleh Desiatyrikov (xWF) <desiat...@google.com>
Gerrit-Reviewer: Frank Liberato <libe...@chromium.org>
Gerrit-Reviewer: Gurmeet Kalra <gurm...@google.com>
Gerrit-CC: Frank Liberato <libe...@google.com>
Gerrit-Attention: Gurmeet Kalra <gurm...@google.com>
Gerrit-Attention: Oleh Desiatyrikov (xWF) <desiat...@google.com>
Gerrit-Attention: Frank Liberato <libe...@chromium.org>
Gerrit-Comment-Date: Tue, 26 May 2026 15:56:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Gurmeet Kalra (Gerrit)

unread,
May 26, 2026, 9:51:46 PM (7 days ago) May 26
to Oleh Desiatyrikov (xWF), Frank Liberato, chromium...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org
Attention needed from Frank Liberato and Oleh Desiatyrikov (xWF)

Gurmeet Kalra voted and added 1 comment

Votes added by Gurmeet Kalra

Code-Review+1

1 comment

Patchset-level comments
Gurmeet Kalra . resolved

LGTM for `*/media/immersive_playback/*` code.

Open in Gerrit

Related details

Attention is currently required from:
  • Frank Liberato
  • Oleh Desiatyrikov (xWF)
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement is not satisfiedCode-Owners
    • requirement is not 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: Idb1f4f8e95d3cc45c5d322da8fdc120323a2a7cc
    Gerrit-Change-Number: 7876303
    Gerrit-PatchSet: 1
    Gerrit-Owner: Oleh Desiatyrikov (xWF) <desiat...@google.com>
    Gerrit-Reviewer: Frank Liberato <libe...@chromium.org>
    Gerrit-Reviewer: Gurmeet Kalra <gurm...@google.com>
    Gerrit-CC: Frank Liberato <libe...@google.com>
    Gerrit-Attention: Oleh Desiatyrikov (xWF) <desiat...@google.com>
    Gerrit-Attention: Frank Liberato <libe...@chromium.org>
    Gerrit-Comment-Date: Wed, 27 May 2026 01:51:28 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Frank Liberato (Gerrit)

    unread,
    May 27, 2026, 1:34:26 PM (6 days ago) May 27
    to Oleh Desiatyrikov (xWF), Gurmeet Kalra, Frank Liberato, chromium...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org
    Attention needed from Oleh Desiatyrikov (xWF)

    Frank Liberato voted and added 2 comments

    Votes added by Frank Liberato

    Code-Review+1

    2 comments

    Patchset-level comments
    Frank Liberato . resolved

    lgtm % a future CL to remove the bit operations.

    -fl

    File chrome/browser/android/tab_web_contents_delegate_android.cc
    Line 878, Patchset 1 (Latest): options.stereo_mode = static_cast<content::ImmersiveStereoMode>(
    (packed_result >> 4) & 0xF);
    Frank Liberato . unresolved

    i missed this earlier, but in a future CL please switch this not to require bit operations. this is technicalyl undefined if it happens that the packed result happens to have a value that doesn't correspond to an enum.

    Open in Gerrit

    Related details

    Attention is currently required from:
    • Oleh Desiatyrikov (xWF)
    Submit Requirements:
      • requirement satisfiedCode-Coverage
      • requirement is not 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: Idb1f4f8e95d3cc45c5d322da8fdc120323a2a7cc
      Gerrit-Change-Number: 7876303
      Gerrit-PatchSet: 1
      Gerrit-Owner: Oleh Desiatyrikov (xWF) <desiat...@google.com>
      Gerrit-Reviewer: Frank Liberato <libe...@chromium.org>
      Gerrit-Reviewer: Gurmeet Kalra <gurm...@google.com>
      Gerrit-CC: Frank Liberato <libe...@google.com>
      Gerrit-Attention: Oleh Desiatyrikov (xWF) <desiat...@google.com>
      Gerrit-Comment-Date: Wed, 27 May 2026 17:34:18 +0000
      Gerrit-HasComments: Yes
      Gerrit-Has-Labels: Yes
      satisfied_requirement
      unsatisfied_requirement
      open
      diffy

      Oleh Desiatyrikov (xWF) (Gerrit)

      unread,
      May 27, 2026, 1:59:28 PM (6 days ago) May 27
      to Gurmeet Kalra, Frank Liberato, chromium...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org

      Oleh Desiatyrikov (xWF) added 1 comment

      File chrome/browser/android/tab_web_contents_delegate_android.cc
      Line 878, Patchset 1 (Latest): options.stereo_mode = static_cast<content::ImmersiveStereoMode>(
      (packed_result >> 4) & 0xF);
      Frank Liberato . resolved

      i missed this earlier, but in a future CL please switch this not to require bit operations. this is technicalyl undefined if it happens that the packed result happens to have a value that doesn't correspond to an enum.

      Oleh Desiatyrikov (xWF)

      I created a separated bug to track this change: crbug.com/517174599

      Open in Gerrit

      Related details

      Attention set is empty
      Submit Requirements:
        • requirement satisfiedCode-Coverage
        • requirement is not 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: Idb1f4f8e95d3cc45c5d322da8fdc120323a2a7cc
        Gerrit-Change-Number: 7876303
        Gerrit-PatchSet: 1
        Gerrit-Owner: Oleh Desiatyrikov (xWF) <desiat...@google.com>
        Gerrit-Reviewer: Frank Liberato <libe...@chromium.org>
        Gerrit-Reviewer: Gurmeet Kalra <gurm...@google.com>
        Gerrit-CC: Frank Liberato <libe...@google.com>
        Gerrit-Comment-Date: Wed, 27 May 2026 17:59:19 +0000
        Gerrit-HasComments: Yes
        Gerrit-Has-Labels: No
        Comment-In-Reply-To: Frank Liberato <libe...@chromium.org>
        satisfied_requirement
        unsatisfied_requirement
        open
        diffy

        Gurmeet Kalra (Gerrit)

        unread,
        May 28, 2026, 7:16:39 PM (5 days ago) May 28
        to Oleh Desiatyrikov (xWF), Frank Liberato, chromium...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org
        Attention needed from Frank Liberato and Oleh Desiatyrikov (xWF)

        Gurmeet Kalra voted Code-Review+1

        Code-Review+1
        Open in Gerrit

        Related details

        Attention is currently required from:
        • Frank Liberato
        • Oleh Desiatyrikov (xWF)
        Submit Requirements:
          • requirement satisfiedCode-Coverage
          • requirement is not satisfiedCode-Owners
          • requirement is not 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: Idb1f4f8e95d3cc45c5d322da8fdc120323a2a7cc
          Gerrit-Change-Number: 7876303
          Gerrit-PatchSet: 2
          Gerrit-Owner: Oleh Desiatyrikov (xWF) <desiat...@google.com>
          Gerrit-Reviewer: Frank Liberato <libe...@chromium.org>
          Gerrit-Reviewer: Gurmeet Kalra <gurm...@google.com>
          Gerrit-CC: Frank Liberato <libe...@google.com>
          Gerrit-Attention: Oleh Desiatyrikov (xWF) <desiat...@google.com>
          Gerrit-Attention: Frank Liberato <libe...@chromium.org>
          Gerrit-Comment-Date: Thu, 28 May 2026 23:16:27 +0000
          Gerrit-HasComments: No
          Gerrit-Has-Labels: Yes
          satisfied_requirement
          unsatisfied_requirement
          open
          diffy

          Oleh Desiatyrikov (xWF) (Gerrit)

          unread,
          May 29, 2026, 5:51:25 PM (4 days ago) May 29
          to Chromium LUCI CQ, Wang, Wei4, Kenneth R Christiansen, Mandy, Arnaud, android-bu...@system.gserviceaccount.com, Gurmeet Kalra, Frank Liberato, chromium...@chromium.org, headless...@chromium.org, blink-re...@chromium.org, blink-...@chromium.org, feature-me...@chromium.org, feature-v...@chromium.org, ipc-securi...@chromium.org, kinuko...@chromium.org
          Attention needed from Frank Liberato, Gurmeet Kalra and Oleh Desiatyrikov (xWF)

          Message from Oleh Desiatyrikov (xWF)

          Set Ready For Review

          Open in Gerrit

          Related details

          Attention is currently required from:
          • Frank Liberato
          • Gurmeet Kalra
          • Oleh Desiatyrikov (xWF)
          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: Idb1f4f8e95d3cc45c5d322da8fdc120323a2a7cc
            Gerrit-Change-Number: 7876303
            Gerrit-PatchSet: 6
            Gerrit-Owner: Oleh Desiatyrikov (xWF) <desiat...@google.com>
            Gerrit-Reviewer: Frank Liberato <libe...@chromium.org>
            Gerrit-Reviewer: Gurmeet Kalra <gurm...@google.com>
            Gerrit-Reviewer: Oleh Desiatyrikov (xWF) <desiat...@google.com>
            Gerrit-CC: Frank Liberato <libe...@google.com>
            Gerrit-CC: Kenneth R Christiansen <kenneth.r.c...@intel.com>
            Gerrit-CC: Mandy, Arnaud <arnaud...@intel.com>
            Gerrit-CC: Wang, Wei4 <wei4...@intel.com>
            Gerrit-Attention: Gurmeet Kalra <gurm...@google.com>
            Gerrit-Attention: Oleh Desiatyrikov (xWF) <desiat...@google.com>
            Gerrit-Attention: Frank Liberato <libe...@chromium.org>
            Gerrit-Comment-Date: Fri, 29 May 2026 21:51:11 +0000
            Gerrit-HasComments: No
            Gerrit-Has-Labels: No
            satisfied_requirement
            unsatisfied_requirement
            open
            diffy
            Reply all
            Reply to author
            Forward
            0 new messages