[WebContentsState] Set version code at ctor [chromium/src : main]

0 views
Skip to first unread message

Calder Kitagawa (Gerrit)

unread,
Sep 25, 2025, 11:34:20 AM (19 hours ago) Sep 25
to Sky Malice, Chromium LUCI CQ, chromium...@chromium.org, jinsukk...@chromium.org
Attention needed from Sky Malice

Calder Kitagawa voted and added 1 comment

Votes added by Calder Kitagawa

Auto-Submit+1

1 comment

Patchset-level comments
File-level comment, Patchset 2 (Latest):
Calder Kitagawa . resolved

PTAL Thanks!

Open in Gerrit

Related details

Attention is currently required from:
  • Sky Malice
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
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: I04268bd4f2961af4cfbd6660cc541ea67a90730b
Gerrit-Change-Number: 6983896
Gerrit-PatchSet: 2
Gerrit-Owner: Calder Kitagawa <ckit...@chromium.org>
Gerrit-Reviewer: Calder Kitagawa <ckit...@chromium.org>
Gerrit-Reviewer: Sky Malice <sk...@chromium.org>
Gerrit-Attention: Sky Malice <sk...@chromium.org>
Gerrit-Comment-Date: Thu, 25 Sep 2025 15:34:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Sky Malice (Gerrit)

unread,
Sep 25, 2025, 11:46:48 AM (19 hours ago) Sep 25
to Calder Kitagawa, Chromium LUCI CQ, chromium...@chromium.org, jinsukk...@chromium.org
Attention needed from Calder Kitagawa

Sky Malice voted and added 6 comments

Votes added by Sky Malice

Code-Review+1

6 comments

Patchset-level comments
Sky Malice . resolved

What a great idea and change Calder.

File chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabUmaTest.java
Line 201, Patchset 2 (Latest): /* version= */ 2);
Sky Malice . unresolved

If there a deps problem with using WebContentsState.CONTENTS_STATE_CURRENT_VERSION here?

File chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreTest.java
Line 835, Patchset 2 (Latest): tabState.contentsState = new WebContentsState(buffer, /* version= */ 1);
Sky Malice . unresolved

Should we create a constant for this? I assume you're using old on purpose.

Actually, why is this not 2?

File chrome/browser/tab/java/src/org/chromium/chrome/browser/tab/WebContentsState.java
Line 25, Patchset 2 (Latest): * 1 - Chrome m25
Sky Malice . resolved

So brief!

Line 40, Patchset 2 (Latest): private final int mVersion;
Sky Malice . resolved

Nice!

File chrome/browser/tabpersistence/android/java/src/org/chromium/chrome/browser/tabpersistence/TabStateFileManager.java
Line 374, Patchset 2 (Parent): // On the stable channel, the first release is version 18. For all other channels,
// chrome 25 is the first release.
Sky Malice . resolved

Woah... I didn't know this.

Open in Gerrit

Related details

Attention is currently required from:
  • Calder Kitagawa
Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Owners
    • requirement satisfiedCode-Review
    • requirement is not satisfiedNo-Unresolved-Comments
    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: I04268bd4f2961af4cfbd6660cc541ea67a90730b
    Gerrit-Change-Number: 6983896
    Gerrit-PatchSet: 2
    Gerrit-Owner: Calder Kitagawa <ckit...@chromium.org>
    Gerrit-Reviewer: Calder Kitagawa <ckit...@chromium.org>
    Gerrit-Reviewer: Sky Malice <sk...@chromium.org>
    Gerrit-Attention: Calder Kitagawa <ckit...@chromium.org>
    Gerrit-Comment-Date: Thu, 25 Sep 2025 15:46:36 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    satisfied_requirement
    unsatisfied_requirement
    open
    diffy

    Calder Kitagawa (Gerrit)

    unread,
    Sep 25, 2025, 12:37:42 PM (18 hours ago) Sep 25
    to Sky Malice, Chromium LUCI CQ, chromium...@chromium.org, jinsukk...@chromium.org

    Calder Kitagawa voted and added 2 comments

    Votes added by Calder Kitagawa

    Auto-Submit+1
    Commit-Queue+2

    2 comments

    File chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabUmaTest.java
    Line 201, Patchset 2: /* version= */ 2);
    Sky Malice . resolved

    If there a deps problem with using WebContentsState.CONTENTS_STATE_CURRENT_VERSION here?

    Calder Kitagawa

    Done

    File chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreTest.java
    Line 835, Patchset 2: tabState.contentsState = new WebContentsState(buffer, /* version= */ 1);
    Sky Malice . resolved

    Should we create a constant for this? I assume you're using old on purpose.

    Actually, why is this not 2?

    Calder Kitagawa

    Done

    Open in Gerrit

    Related details

    Attention set is empty
    Submit Requirements:
    • requirement satisfiedCode-Coverage
    • requirement satisfiedCode-Owners
    • requirement satisfiedCode-Review
    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: I04268bd4f2961af4cfbd6660cc541ea67a90730b
    Gerrit-Change-Number: 6983896
    Gerrit-PatchSet: 4
    Gerrit-Owner: Calder Kitagawa <ckit...@chromium.org>
    Gerrit-Reviewer: Calder Kitagawa <ckit...@chromium.org>
    Gerrit-Reviewer: Sky Malice <sk...@chromium.org>
    Gerrit-Comment-Date: Thu, 25 Sep 2025 16:37:35 +0000
    Gerrit-HasComments: Yes
    Gerrit-Has-Labels: Yes
    Comment-In-Reply-To: Sky Malice <sk...@chromium.org>
    satisfied_requirement
    open
    diffy

    Chromium LUCI CQ (Gerrit)

    unread,
    Sep 25, 2025, 1:38:04 PM (17 hours ago) Sep 25
    to Calder Kitagawa, Sky Malice, chromium...@chromium.org, jinsukk...@chromium.org

    Chromium LUCI CQ submitted the change with unreviewed changes

    Unreviewed changes

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

    ```
    The name of the file: chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreTest.java
    Insertions: 3, Deletions: 1.

    The diff is too large to show. Please review the diff.
    ```
    ```
    The name of the file: chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabUmaTest.java
    Insertions: 1, Deletions: 1.

    The diff is too large to show. Please review the diff.
    ```

    Change information

    Commit message:
    [WebContentsState] Set version code at ctor

    Attaching the version code after the fact is puzzling as we should know
    both the version and buffer state at all times. Change the ctor to take
    the version code and update all calls to set this value.
    Bug: 447345580
    Change-Id: I04268bd4f2961af4cfbd6660cc541ea67a90730b
    Commit-Queue: Calder Kitagawa <ckit...@chromium.org>
    Auto-Submit: Calder Kitagawa <ckit...@chromium.org>
    Reviewed-by: Sky Malice <sk...@chromium.org>
    Cr-Commit-Position: refs/heads/main@{#1520726}
    Files:
    • M chrome/android/java/src/org/chromium/chrome/browser/app/tabmodel/TabStateStore.java
    • M chrome/android/java/src/org/chromium/chrome/browser/tab/TabStateExtractor.java
    • M chrome/android/javatests/src/org/chromium/chrome/browser/tab/TabUmaTest.java
    • M chrome/android/javatests/src/org/chromium/chrome/browser/tab/state/TabStateFlatBufferTest.java
    • M chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabPersistentStoreTest.java
    • M chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/TabbedModeTabPersistencePolicyTest.java
    • M chrome/android/junit/src/org/chromium/chrome/browser/app/tabmodel/TabPersistentStoreIntegrationTest.java
    • M chrome/android/junit/src/org/chromium/chrome/browser/tab/tab_restore/HistoricalTabSaverImplUnitTest.java
    • M chrome/browser/tab/java/src/org/chromium/chrome/browser/tab/TabStateStorageService.java
    • M chrome/browser/tab/java/src/org/chromium/chrome/browser/tab/WebContentsState.java
    • M chrome/browser/tab/java/src/org/chromium/chrome/browser/tab/WebContentsStateBridge.java
    • M chrome/browser/tabpersistence/android/java/src/org/chromium/chrome/browser/tabpersistence/FlatBufferTabStateSerializer.java
    • M chrome/browser/tabpersistence/android/java/src/org/chromium/chrome/browser/tabpersistence/TabStateFileManager.java
    • M chrome/browser/tabpersistence/android/java/src/org/chromium/chrome/browser/tabpersistence/TabStateFileManagerUnitTest.java
    Change size: M
    Delta: 14 files changed, 68 insertions(+), 52 deletions(-)
    Branch: refs/heads/main
    Submit Requirements:
    • requirement satisfiedCode-Review: +1 by Sky Malice
    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: I04268bd4f2961af4cfbd6660cc541ea67a90730b
    Gerrit-Change-Number: 6983896
    Gerrit-PatchSet: 5
    Gerrit-Owner: Calder Kitagawa <ckit...@chromium.org>
    Gerrit-Reviewer: Calder Kitagawa <ckit...@chromium.org>
    Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
    Gerrit-Reviewer: Sky Malice <sk...@chromium.org>
    open
    diffy
    satisfied_requirement
    Reply all
    Reply to author
    Forward
    0 new messages