pip2: Don't discard tabs that own a picture-in-picture window [chromium/src : main]
Tommy Steimel (Gerrit)
Attention is currently required from: Chris Hamilton, Robert Kaplow.
Tommy Steimel would like Chris Hamilton and Robert Kaplow to review this change.
pip2: Don't discard tabs that own a picture-in-picture window
This CL prevents tabs from being discarded when they have a video or
document picture-in-picture window open. This prevents an issue where
picture-in-picture windows can close unexpectedly.
Bug: 1446327
Change-Id: I9d6c021a0f527fa86243ce2273d8d7b868c326bd
---
M chrome/browser/resource_coordinator/decision_details.cc
M chrome/browser/resource_coordinator/decision_details.h
M chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
M tools/metrics/ukm/ukm.xml
4 files changed, 17 insertions(+), 0 deletions(-)
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Tommy Steimel (Gerrit)
Attention is currently required from: Chris Hamilton, Robert Kaplow.
1 comment:
Patchset:
ptal. chrisha@ as owner of c/b/resource_coordinator/* and rkaplow@ as owner of tools/metrics/ukm/ukm.xml
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Chris Hamilton (Gerrit)
Attention is currently required from: Anthony Vallée-Dubois, Robert Kaplow, Tommy Steimel.
Chris Hamilton would like Anthony Vallée-Dubois to review this change authored by Tommy Steimel.
pip2: Don't discard tabs that own a picture-in-picture window
This CL prevents tabs from being discarded when they have a video or
document picture-in-picture window open. This prevents an issue where
picture-in-picture windows can close unexpectedly.
Bug: 1446327
Change-Id: I9d6c021a0f527fa86243ce2273d8d7b868c326bd
---
M chrome/browser/resource_coordinator/decision_details.cc
M chrome/browser/resource_coordinator/decision_details.h
M chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
M tools/metrics/ukm/ukm.xml
4 files changed, 17 insertions(+), 0 deletions(-)
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Chris Hamilton (Gerrit)
Attention is currently required from: Anthony Vallée-Dubois, Robert Kaplow, Tommy Steimel.
1 comment:
Patchset:
I'm no longer on Chrome team. Adding anthonyvd@ who now owns the discarding logic.
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Chris Hamilton (Gerrit)
Attention is currently required from: Anthony Vallée-Dubois, Robert Kaplow, Tommy Steimel.
1 comment:
Patchset:
Some additional info:
The resource_coordinator / tab_lifecycle_unit stuff is CrOS-only right now. For other platforms the logic is now in performance_manager. You might want to check out here:
(This is awkward and duplicate logic for the time being, but eventually the resource_coordinator mechanism will disappear.)
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Anthony Vallée-Dubois (Gerrit)
Attention is currently required from: Chris Hamilton, Robert Kaplow, Tommy Steimel.
2 comments:
Patchset:
Some additional info: […]
Yep, this logic needs to be in PerformanceManager (Chris' link) to work on all platforms.
I don't actually own PerformanceManager code, but you can add joenotcharles@ as a reviewer when this CL is changed to go through PM's `CanDiscard`
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Robert Kaplow (Gerrit)
Attention is currently required from: Chris Hamilton, Tommy Steimel.
Patch set 1:Code-Review +1
Robert Kaplow (Gerrit)
Attention is currently required from: Chris Hamilton, Tommy Steimel.
Patch set 1:-Code-Review
1 comment:
Patchset:
ukm side seems fine, but removing LGTM as seems like owners think it is in wrong place
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Tommy Steimel (Gerrit)
Attention is currently required from: Chris Hamilton, Joe Mason.
Tommy Steimel would like Joe Mason to review this change.
pip2: Don't discard tabs that own a picture-in-picture window
This CL prevents tabs from being discarded when they have a video or
document picture-in-picture window open. This prevents an issue where
picture-in-picture windows can close unexpectedly.
Bug: 1446327
Change-Id: I9d6c021a0f527fa86243ce2273d8d7b868c326bd
---
M chrome/browser/performance_manager/policies/background_tab_loading_policy_unittest.cc
M chrome/browser/performance_manager/policies/page_discarding_helper.cc
M chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc
M chrome/browser/resource_coordinator/decision_details.cc
M chrome/browser/resource_coordinator/decision_details.h
M chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
M components/performance_manager/graph/page_node_impl.cc
M components/performance_manager/graph/page_node_impl.h
M components/performance_manager/graph/page_node_impl_unittest.cc
M components/performance_manager/performance_manager_impl.cc
M components/performance_manager/performance_manager_impl.h
M components/performance_manager/performance_manager_impl_unittest.cc
M components/performance_manager/performance_manager_tab_helper.cc
M components/performance_manager/performance_manager_tab_helper.h
M components/performance_manager/public/graph/page_node.h
M components/performance_manager/test_support/graph_test_harness.h
M components/performance_manager/worker_watcher_unittest.cc
M tools/metrics/ukm/ukm.xml
18 files changed, 90 insertions(+), 22 deletions(-)
Tommy Steimel (Gerrit)
Attention is currently required from: Chris Hamilton, Joe Mason.
1 comment:
Patchset:
Thanks for the pointers. I got busy with some other work but I'm getting back to this now. I added the fixes in performance manager, though I wasn't sure if I should also keep the original changes (IIUC you said that it's used on ChromeOS?). Let me know if I should remove those. +joenotcharles@ as suggested.
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Joe Mason (Gerrit)
Attention is currently required from: Chris Hamilton, Tommy Steimel.
Patch set 2:Code-Review +1
1 comment:
File components/performance_manager/graph/page_node_impl.cc:
Patch Set #2, Line 57:
has_picture_in_pictureperformance_manager LGTM. I think it's time to change these bools to an EnumSet since they're getting unwieldy, but that's a separate change.
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Tommy Steimel (Gerrit)
Attention is currently required from: Chris Hamilton, Joe Mason, Tommy Steimel.
Tommy Steimel uploaded patch set #3 to this change.
The following approvals got outdated and were removed: Code-Review+1 by Joe Mason
pip2: Don't discard tabs that own a picture-in-picture window
This CL prevents tabs from being discarded when they have a video or
document picture-in-picture window open. This prevents an issue where
picture-in-picture windows can close unexpectedly.
Bug: 1446327
Change-Id: I9d6c021a0f527fa86243ce2273d8d7b868c326bd
---
M chrome/browser/performance_manager/policies/page_discarding_helper.cc
M chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc
M chrome/browser/resource_coordinator/decision_details.cc
M chrome/browser/resource_coordinator/decision_details.h
M chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
M components/performance_manager/graph/page_node_impl.cc
M components/performance_manager/graph/page_node_impl.h
M components/performance_manager/performance_manager_tab_helper.cc
M components/performance_manager/performance_manager_tab_helper.h
M components/performance_manager/public/graph/page_node.h
M tools/metrics/ukm/ukm.xml
11 files changed, 66 insertions(+), 2 deletions(-)
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Tommy Steimel (Gerrit)
Attention is currently required from: Joe Mason.
Tommy Steimel removed Chris Hamilton from this change.
pip2: Don't discard tabs that own a picture-in-picture window
This CL prevents tabs from being discarded when they have a video or
document picture-in-picture window open. This prevents an issue where
picture-in-picture windows can close unexpectedly.
Bug: 1446327
Change-Id: I9d6c021a0f527fa86243ce2273d8d7b868c326bd
---
M chrome/browser/performance_manager/policies/page_discarding_helper.cc
M chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc
M chrome/browser/resource_coordinator/decision_details.cc
M chrome/browser/resource_coordinator/decision_details.h
M chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
M components/performance_manager/graph/page_node_impl.cc
M components/performance_manager/graph/page_node_impl.h
M components/performance_manager/performance_manager_tab_helper.cc
M components/performance_manager/performance_manager_tab_helper.h
M components/performance_manager/public/graph/page_node.h
M tools/metrics/ukm/ukm.xml
11 files changed, 66 insertions(+), 2 deletions(-)
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Tommy Steimel (Gerrit)
Attention is currently required from: Joe Mason.
2 comments:
Patchset:
ptal
File components/performance_manager/graph/page_node_impl.cc:
Patch Set #2, Line 57:
has_picture_in_picture
performance_manager LGTM. […]
Rebased on your CL, thanks!
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Tommy Steimel (Gerrit)
Attention is currently required from: Francois Pierre Doray, Joe Mason.
Tommy Steimel would like Francois Pierre Doray to review this change.
pip2: Don't discard tabs that own a picture-in-picture window
This CL prevents tabs from being discarded when they have a video or
document picture-in-picture window open. This prevents an issue where
picture-in-picture windows can close unexpectedly.
Bug: 1446327
Change-Id: I9d6c021a0f527fa86243ce2273d8d7b868c326bd
---
M chrome/browser/performance_manager/policies/page_discarding_helper.cc
M chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc
M chrome/browser/resource_coordinator/decision_details.cc
M chrome/browser/resource_coordinator/decision_details.h
M chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
M components/performance_manager/graph/page_node_impl.cc
M components/performance_manager/graph/page_node_impl.h
M components/performance_manager/performance_manager_tab_helper.cc
M components/performance_manager/performance_manager_tab_helper.h
M components/performance_manager/public/graph/page_node.h
M tools/metrics/ukm/ukm.xml
11 files changed, 66 insertions(+), 2 deletions(-)
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Tommy Steimel (Gerrit)
Attention is currently required from: Francois Pierre Doray, Joe Mason.
1 comment:
Patchset:
-chrisa@ +fdoray@ for owners review of resource_coordinator
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Joe Mason (Gerrit)
-chrisa@ +fdoray@ for owners review of resource_coordinator
Huh, didn't realize I'm not a resource_coordinator owner. It should probably delegate to components/performance_manager/OWNERS at this point. fdoray@, want to make that change?
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Francois Pierre Doray (Gerrit)
Attention is currently required from: Tommy Steimel.
1 comment:
File components/performance_manager/graph/page_node_impl.h:
Patch Set #3, Line 362:
bool has_picture_in_picture_Make this an `ObservedProperty::NotifiesOnlyOnChanges<bool, ...> like other properties of this class. This will allow other policies to take into account the picture-in-picture state, for example components/performance_manager/graph/policies/process_priority_policy.h.
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Tommy Steimel (Gerrit)
Attention is currently required from: Tommy Steimel.
Tommy Steimel uploaded patch set #4 to this change.
pip2: Don't discard tabs that own a picture-in-picture window
This CL prevents tabs from being discarded when they have a video or
document picture-in-picture window open. This prevents an issue where
picture-in-picture windows can close unexpectedly.
Bug: 1446327
Change-Id: I9d6c021a0f527fa86243ce2273d8d7b868c326bd
---
M chrome/browser/performance_manager/policies/page_discarding_helper.cc
M chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc
M chrome/browser/resource_coordinator/decision_details.cc
M chrome/browser/resource_coordinator/decision_details.h
M chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
M chrome/browser/ui/webui/discards/graph_dump_impl.h
M components/performance_manager/graph/page_node_impl.cc
M components/performance_manager/graph/page_node_impl.h
M components/performance_manager/graph/page_node_impl_unittest.cc
M components/performance_manager/performance_manager_tab_helper.cc
M components/performance_manager/performance_manager_tab_helper.h
M components/performance_manager/public/graph/page_node.h
M tools/metrics/ukm/ukm.xml
13 files changed, 83 insertions(+), 2 deletions(-)
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Tommy Steimel (Gerrit)
Attention is currently required from: Francois Pierre Doray.
Patch set 4:Commit-Queue +1
1 comment:
File components/performance_manager/graph/page_node_impl.h:
Patch Set #3, Line 362:
bool has_picture_in_picture_
Make this an `ObservedProperty::NotifiesOnlyOnChanges<bool, ... […]
Done
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Francois Pierre Doray (Gerrit)
Tommy Steimel (Gerrit)
Attention is currently required from: Robert Kaplow.
1 comment:
Patchset:
=> rkaplow@ for owners review of tools/metrics/ukm/ukm.xml
To view, visit change 4582630. To unsubscribe, or for help writing mail filters, visit settings.
Robert Kaplow (Gerrit)
Tommy Steimel (Gerrit)
Patch set 4:Commit-Queue +2
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted this change.
pip2: Don't discard tabs that own a picture-in-picture window
This CL prevents tabs from being discarded when they have a video or
document picture-in-picture window open. This prevents an issue where
picture-in-picture windows can close unexpectedly.
Bug: 1446327
Change-Id: I9d6c021a0f527fa86243ce2273d8d7b868c326bd
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4582630
Reviewed-by: Francois Pierre Doray <fdo...@chromium.org>
Reviewed-by: Robert Kaplow <rka...@chromium.org>
Commit-Queue: Tommy Steimel <ste...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1210398}
---
M chrome/browser/performance_manager/policies/page_discarding_helper.cc
M chrome/browser/performance_manager/policies/page_discarding_helper_unittest.cc
M chrome/browser/resource_coordinator/decision_details.cc
M chrome/browser/resource_coordinator/decision_details.h
M chrome/browser/resource_coordinator/tab_lifecycle_unit.cc
M chrome/browser/ui/webui/discards/graph_dump_impl.h
M components/performance_manager/graph/page_node_impl.cc
M components/performance_manager/graph/page_node_impl.h
M components/performance_manager/graph/page_node_impl_unittest.cc
M components/performance_manager/performance_manager_tab_helper.cc
M components/performance_manager/performance_manager_tab_helper.h
M components/performance_manager/public/graph/page_node.h
M tools/metrics/ukm/ukm.xml
13 files changed, 83 insertions(+), 2 deletions(-)