Update hover cards to support retained WebContents discard impl [chromium/src : main]

0 views
Skip to first unread message

Thomas Lukaszewicz (Gerrit)

unread,
Sep 9, 2024, 9:14:15 AM9/9/24
to chromium...@chromium.org, chrome-gr...@chromium.org

Thomas Lukaszewicz voted Commit-Queue+1

Commit-Queue+1
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • 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: I672f7abde6189ff04d6303b3d83c29a3658f5e11
Gerrit-Change-Number: 5840044
Gerrit-PatchSet: 7
Gerrit-Owner: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Reviewer: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Comment-Date: Mon, 09 Sep 2024 09:14:06 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Thomas Lukaszewicz (Gerrit)

unread,
Sep 9, 2024, 10:13:25 AM9/9/24
to Dana Fried, Francois Pierre Doray, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org
Attention needed from Dana Fried and Francois Pierre Doray

Thomas Lukaszewicz voted and added 1 comment

Votes added by Thomas Lukaszewicz

Commit-Queue+1

1 comment

Patchset-level comments
File-level comment, Patchset 8 (Latest):
Thomas Lukaszewicz . resolved

Dana ptal c/b/ui
Francois ptal c/b/resource_coordinator

Open in Gerrit

Related details

Attention is currently required from:
  • Dana Fried
  • Francois Pierre Doray
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • 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: I672f7abde6189ff04d6303b3d83c29a3658f5e11
Gerrit-Change-Number: 5840044
Gerrit-PatchSet: 8
Gerrit-Owner: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Reviewer: Dana Fried <dfr...@chromium.org>
Gerrit-Reviewer: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Reviewer: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Attention: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Attention: Dana Fried <dfr...@chromium.org>
Gerrit-Comment-Date: Mon, 09 Sep 2024 10:13:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

findit-for-me@appspot.gserviceaccount.com (Gerrit)

unread,
Sep 9, 2024, 11:17:06 AM9/9/24
to Thomas Lukaszewicz, Dana Fried, Francois Pierre Doray, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org
Attention needed from Dana Fried and Francois Pierre Doray

findit...@appspot.gserviceaccount.com voted Code-Coverage+1

This change meets the code coverage requirements.

Code-Coverage+1
Open in Gerrit

Related details

Attention is currently required from:
  • Dana Fried
  • Francois Pierre Doray
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • 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: I672f7abde6189ff04d6303b3d83c29a3658f5e11
Gerrit-Change-Number: 5840044
Gerrit-PatchSet: 8
Gerrit-Owner: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Reviewer: Dana Fried <dfr...@chromium.org>
Gerrit-Reviewer: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Reviewer: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Attention: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Attention: Dana Fried <dfr...@chromium.org>
Gerrit-Comment-Date: Mon, 09 Sep 2024 11:16:55 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
unsatisfied_requirement
open
diffy

Dana Fried (Gerrit)

unread,
Sep 9, 2024, 10:19:51 PM9/9/24
to Thomas Lukaszewicz, findit...@appspot.gserviceaccount.com, Francois Pierre Doray, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org
Attention needed from Francois Pierre Doray and Thomas Lukaszewicz

Dana Fried voted and added 1 comment

Votes added by Dana Fried

Code-Review+1

1 comment

Patchset-level comments
Dana Fried . resolved

Tentative LGTM. The controller system here is very complex so I'm hoping nothing breaks, but I'm sure we'll get bug reports if something does.

Certainly nothing looks too out of the ordinary.

Open in Gerrit

Related details

Attention is currently required from:
  • Francois Pierre Doray
  • Thomas Lukaszewicz
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • 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: I672f7abde6189ff04d6303b3d83c29a3658f5e11
Gerrit-Change-Number: 5840044
Gerrit-PatchSet: 8
Gerrit-Owner: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Reviewer: Dana Fried <dfr...@chromium.org>
Gerrit-Reviewer: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Reviewer: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Attention: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Attention: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Comment-Date: Mon, 09 Sep 2024 22:19:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Francois Pierre Doray (Gerrit)

unread,
Sep 10, 2024, 6:59:19 PM9/10/24
to Thomas Lukaszewicz, Dana Fried, findit...@appspot.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org
Attention needed from Thomas Lukaszewicz

Francois Pierre Doray voted Code-Review+1

Code-Review+1
Open in Gerrit

Related details

Attention is currently required from:
  • Thomas Lukaszewicz
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • 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: I672f7abde6189ff04d6303b3d83c29a3658f5e11
Gerrit-Change-Number: 5840044
Gerrit-PatchSet: 9
Gerrit-Owner: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Reviewer: Dana Fried <dfr...@chromium.org>
Gerrit-Reviewer: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Reviewer: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Attention: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Comment-Date: Tue, 10 Sep 2024 18:59:06 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Thomas Lukaszewicz (Gerrit)

unread,
Sep 10, 2024, 7:59:16 PM9/10/24
to Francois Pierre Doray, Dana Fried, findit...@appspot.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org

Thomas Lukaszewicz voted Commit-Queue+1

Commit-Queue+1
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • 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: I672f7abde6189ff04d6303b3d83c29a3658f5e11
Gerrit-Change-Number: 5840044
Gerrit-PatchSet: 9
Gerrit-Owner: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Reviewer: Dana Fried <dfr...@chromium.org>
Gerrit-Reviewer: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Reviewer: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Comment-Date: Tue, 10 Sep 2024 19:59:06 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Thomas Lukaszewicz (Gerrit)

unread,
Sep 10, 2024, 7:59:34 PM9/10/24
to Francois Pierre Doray, Dana Fried, findit...@appspot.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org

Thomas Lukaszewicz voted Commit-Queue+2

Commit-Queue+2
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • 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: I672f7abde6189ff04d6303b3d83c29a3658f5e11
Gerrit-Change-Number: 5840044
Gerrit-PatchSet: 9
Gerrit-Owner: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Reviewer: Dana Fried <dfr...@chromium.org>
Gerrit-Reviewer: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Reviewer: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Comment-Date: Tue, 10 Sep 2024 19:59:20 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Thomas Lukaszewicz (Gerrit)

unread,
Sep 10, 2024, 10:28:47 PM9/10/24
to Francois Pierre Doray, Dana Fried, findit...@appspot.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org

Thomas Lukaszewicz voted Commit-Queue+2

Commit-Queue+2
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • 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: I672f7abde6189ff04d6303b3d83c29a3658f5e11
Gerrit-Change-Number: 5840044
Gerrit-PatchSet: 10
Gerrit-Owner: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Reviewer: Dana Fried <dfr...@chromium.org>
Gerrit-Reviewer: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Reviewer: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Comment-Date: Tue, 10 Sep 2024 22:28:38 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

findit-for-me@appspot.gserviceaccount.com (Gerrit)

unread,
Sep 10, 2024, 10:55:19 PM9/10/24
to Thomas Lukaszewicz, Francois Pierre Doray, Dana Fried, Chromium LUCI CQ, chromium...@chromium.org, chrome-gr...@chromium.org

This change meets the code coverage requirements.

Code-Coverage+1
Open in Gerrit

Related details

Attention set is empty
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • 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: I672f7abde6189ff04d6303b3d83c29a3658f5e11
Gerrit-Change-Number: 5840044
Gerrit-PatchSet: 10
Gerrit-Owner: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Reviewer: Dana Fried <dfr...@chromium.org>
Gerrit-Reviewer: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Reviewer: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Comment-Date: Tue, 10 Sep 2024 22:55:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
satisfied_requirement
open
diffy

Chromium LUCI CQ (Gerrit)

unread,
Sep 10, 2024, 10:59:16 PM9/10/24
to Thomas Lukaszewicz, Francois Pierre Doray, Dana Fried, findit...@appspot.gserviceaccount.com, chromium...@chromium.org, chrome-gr...@chromium.org

Chromium LUCI CQ submitted the change with unreviewed changes

Unreviewed changes

9 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.

Change information

Commit message:
Update hover cards to support retained WebContents discard impl

This CL updates the ThumbnailReadinessTracker to reset its status to
kNotReady after a discard.

In the current impl this is a no-op as the tracker is re-created with
kNotReady as the default when the null WebContents is created. This
change ensures the tracker is reset to a similar state in the new
impl which does not re-create the tab helper after discard.


The ThumbnailTabHelper has also been updated to avoid clearing data
for discarded tabs and attempting a reload when starting a new
thumbnail observation (a new observation is started when the user
hovers over a tab and cleared when unhovered).

Both changes are logically appropriate and there is no impact on the
current implementation. This allows side-by-side support of the
current and new discard impls.


TabLifecycleUnit is updated to explicitly notify TabStripModel::
UpdateWebContentsStateAt(), ensuring downstream code responds
appropriately to discard status changes and that this is reflected
in updated TabRenderData.
Bug: 347770670
Change-Id: I672f7abde6189ff04d6303b3d83c29a3658f5e11
Commit-Queue: Thomas Lukaszewicz <tl...@chromium.org>
Reviewed-by: Dana Fried <dfr...@chromium.org>
Reviewed-by: Francois Pierre Doray <fdo...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1353646}
Files:
  • M chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
  • M chrome/browser/ui/thumbnails/thumbnail_readiness_tracker.cc
  • M chrome/browser/ui/thumbnails/thumbnail_readiness_tracker.h
  • M chrome/browser/ui/thumbnails/thumbnail_tab_helper.cc
  • M chrome/browser/ui/views/tabs/tab_hover_card_controller_interactive_uitest.cc
Change size: M
Delta: 5 files changed, 49 insertions(+), 5 deletions(-)
Branch: refs/heads/main
Submit Requirements:
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: I672f7abde6189ff04d6303b3d83c29a3658f5e11
Gerrit-Change-Number: 5840044
Gerrit-PatchSet: 11
Gerrit-Owner: Thomas Lukaszewicz <tl...@chromium.org>
Gerrit-Reviewer: Chromium LUCI CQ <chromiu...@luci-project-accounts.iam.gserviceaccount.com>
Gerrit-Reviewer: Dana Fried <dfr...@chromium.org>
Gerrit-Reviewer: Francois Pierre Doray <fdo...@chromium.org>
Gerrit-Reviewer: Thomas Lukaszewicz <tl...@chromium.org>
open
diffy
satisfied_requirement
Reply all
Reply to author
Forward
0 new messages