Meta bug for PIP activation issue |
|||||||||
Issue descriptionI found a few case that PIP can still be activated. I'll send a fix soon.
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/14b73df744547f1a0443a92540d9304a567b7743 commit 14b73df744547f1a0443a92540d9304a567b7743 Author: Mitsuru Oshima <oshima@chromium.org> Date: Wed Dec 05 07:53:26 2018 PIP activation fix. * The initial state is also wrong because widget overwite the activatable state. * Deactivate when it exits PIP. This may not be an issue for ChromePIP, but nothing prevent it from swithing state so it's better to move that logic to windowstate. * Cnsolidates the PIP activation logic in Chrome PIP and Android PIP into WindowState. Bug: 911267 Test: covered by unittests Change-Id: Idf447771ccaa1d4ad95260c4c44f182946f64c0b Reviewed-on: https://chromium-review.googlesource.com/c/1357819 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Eliot Courtney <edcourtney@chromium.org> Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/heads/master@{#613901} [modify] https://crrev.com/14b73df744547f1a0443a92540d9304a567b7743/ash/BUILD.gn [add] https://crrev.com/14b73df744547f1a0443a92540d9304a567b7743/ash/wm/pip/pip_unittest.cc [modify] https://crrev.com/14b73df744547f1a0443a92540d9304a567b7743/ash/wm/window_state.cc [modify] https://crrev.com/14b73df744547f1a0443a92540d9304a567b7743/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc [modify] https://crrev.com/14b73df744547f1a0443a92540d9304a567b7743/chrome/browser/ui/views/overlay/overlay_window_views.cc [modify] https://crrev.com/14b73df744547f1a0443a92540d9304a567b7743/components/exo/client_controlled_shell_surface.cc [modify] https://crrev.com/14b73df744547f1a0443a92540d9304a567b7743/components/exo/shell_surface_base.cc
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1e388b0752dd47ca0f5c7168127d9824af9b6d0b commit 1e388b0752dd47ca0f5c7168127d9824af9b6d0b Author: Pavol Marko <pmarko@chromium.org> Date: Wed Dec 05 14:00:07 2018 Revert "PIP activation fix." This reverts commit 14b73df744547f1a0443a92540d9304a567b7743. Reason for revert: Suspected to cause build failures in network_service_browser_tests (PictureInPictureWindowControllerBrowserTest.CreationAndVisibilityAndActivation). Example failure: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/Mac10.12%20Tests/17330 [24104:4611:1205/045208.445953:WARNING:notification_platform_bridge_mac.mm(521)] AlertNotificationService: XPC connection invalidated. ../../chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc:203: Failure Value of: platform_util::IsWindowActive(native_window) Actual: false Expected: true Original change's description: > PIP activation fix. > > * The initial state is also wrong because widget overwite the > activatable state. > * Deactivate when it exits PIP. > This may not be an issue for ChromePIP, but nothing prevent it from > swithing state so it's better to move that logic to windowstate. > * Cnsolidates the PIP activation logic > in Chrome PIP and Android PIP into WindowState. > > Bug: 911267 > Test: covered by unittests > Change-Id: Idf447771ccaa1d4ad95260c4c44f182946f64c0b > Reviewed-on: https://chromium-review.googlesource.com/c/1357819 > Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> > Reviewed-by: Eliot Courtney <edcourtney@chromium.org> > Commit-Queue: Mitsuru Oshima <oshima@chromium.org> > Cr-Commit-Position: refs/heads/master@{#613901} TBR=oshima@chromium.org,mlamouri@chromium.org,edcourtney@chromium.org Change-Id: I5c52bbbb07e76f96571309bcdeb5839502a176b3 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 911267 Reviewed-on: https://chromium-review.googlesource.com/c/1363198 Reviewed-by: Pavol Marko <pmarko@chromium.org> Commit-Queue: Pavol Marko <pmarko@chromium.org> Cr-Commit-Position: refs/heads/master@{#613958} [modify] https://crrev.com/1e388b0752dd47ca0f5c7168127d9824af9b6d0b/ash/BUILD.gn [delete] https://crrev.com/4619e61d0c042ddfecc652c75ea797c39f4f457f/ash/wm/pip/pip_unittest.cc [modify] https://crrev.com/1e388b0752dd47ca0f5c7168127d9824af9b6d0b/ash/wm/window_state.cc [modify] https://crrev.com/1e388b0752dd47ca0f5c7168127d9824af9b6d0b/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc [modify] https://crrev.com/1e388b0752dd47ca0f5c7168127d9824af9b6d0b/chrome/browser/ui/views/overlay/overlay_window_views.cc [modify] https://crrev.com/1e388b0752dd47ca0f5c7168127d9824af9b6d0b/components/exo/client_controlled_shell_surface.cc [modify] https://crrev.com/1e388b0752dd47ca0f5c7168127d9824af9b6d0b/components/exo/shell_surface_base.cc
,
Dec 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ea9a040e5b7974b5317c15948f4bd72644aad94b commit ea9a040e5b7974b5317c15948f4bd72644aad94b Author: Mitsuru Oshima <oshima@chromium.org> Date: Wed Dec 05 22:29:41 2018 PIP activation fix. This is reland of crrev.com/c/1357819. I changed the expectation for MACOSX_10_12. * The initial state is also wrong because widget overwite the activatable state. * Deactivate when it exits PIP. This may not be an issue for ChromePIP, but nothing prevent it from swithing state so it's better to move that logic to windowstate. * Cnsolidates the PIP activation logic in Chrome PIP and Android PIP into WindowState. Bug: 911267 TBR: edcourtney@chromium.org Test: covered by unittests Change-Id: Ia6b9f3beb9035debfef64fbc85d1ce9482047422 Reviewed-on: https://chromium-review.googlesource.com/c/1357819 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Eliot Courtney <edcourtney@chromium.org> Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#613901} Reviewed-on: https://chromium-review.googlesource.com/c/1363204 Cr-Commit-Position: refs/heads/master@{#614136} [modify] https://crrev.com/ea9a040e5b7974b5317c15948f4bd72644aad94b/ash/BUILD.gn [add] https://crrev.com/ea9a040e5b7974b5317c15948f4bd72644aad94b/ash/wm/pip/pip_unittest.cc [modify] https://crrev.com/ea9a040e5b7974b5317c15948f4bd72644aad94b/ash/wm/window_state.cc [modify] https://crrev.com/ea9a040e5b7974b5317c15948f4bd72644aad94b/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc [modify] https://crrev.com/ea9a040e5b7974b5317c15948f4bd72644aad94b/chrome/browser/ui/views/overlay/overlay_window_views.cc [modify] https://crrev.com/ea9a040e5b7974b5317c15948f4bd72644aad94b/components/exo/client_controlled_shell_surface.cc [modify] https://crrev.com/ea9a040e5b7974b5317c15948f4bd72644aad94b/components/exo/shell_surface_base.cc
,
Dec 6
,
Dec 6
,
Dec 6
This bug requires manual review: Reverts referenced in bugdroid comments after merge request. Please contact the milestone owner if you have questions. Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 7
,
Dec 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e19f673801c7144464519868bb854c77e421acc commit 8e19f673801c7144464519868bb854c77e421acc Author: Mitsuru Oshima <oshima@chromium.org> Date: Fri Dec 07 17:50:49 2018 PIP activation fix. This is reland of crrev.com/c/1357819. I changed the expectation for MACOSX_10_12. * The initial state is also wrong because widget overwite the activatable state. * Deactivate when it exits PIP. This may not be an issue for ChromePIP, but nothing prevent it from swithing state so it's better to move that logic to windowstate. * Cnsolidates the PIP activation logic in Chrome PIP and Android PIP into WindowState. Bug: 911267 TBR: edcourtney@chromium.org Test: covered by unittests Change-Id: Ia6b9f3beb9035debfef64fbc85d1ce9482047422 Reviewed-on: https://chromium-review.googlesource.com/c/1357819 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Eliot Courtney <edcourtney@chromium.org> Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Cr-Original-Original-Commit-Position: refs/heads/master@{#613901} Reviewed-on: https://chromium-review.googlesource.com/c/1363204 Cr-Original-Commit-Position: refs/heads/master@{#614136}(cherry picked from commit ea9a040e5b7974b5317c15948f4bd72644aad94b) Reviewed-on: https://chromium-review.googlesource.com/c/1367891 Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#137} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} [modify] https://crrev.com/8e19f673801c7144464519868bb854c77e421acc/ash/BUILD.gn [add] https://crrev.com/8e19f673801c7144464519868bb854c77e421acc/ash/wm/pip/pip_unittest.cc [modify] https://crrev.com/8e19f673801c7144464519868bb854c77e421acc/ash/wm/window_state.cc [modify] https://crrev.com/8e19f673801c7144464519868bb854c77e421acc/chrome/browser/picture_in_picture/picture_in_picture_window_controller_browsertest.cc [modify] https://crrev.com/8e19f673801c7144464519868bb854c77e421acc/chrome/browser/ui/views/overlay/overlay_window_views.cc [modify] https://crrev.com/8e19f673801c7144464519868bb854c77e421acc/components/exo/client_controlled_shell_surface.cc [modify] https://crrev.com/8e19f673801c7144464519868bb854c77e421acc/components/exo/shell_surface_base.cc
,
Dec 7
,
Dec 19
Here's a summary of the rules that were executed: - OnlyMergeApprovedChange: Rule Failed -- Revision 8e19f673801c7144464519868bb854c77e421acc was merged to refs/branch-heads/3626 branch with no merge approval from a TPM! Please explain why this change was merged to the branch! - AcknowledgeMerge: Notification Required --
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8e19f673801c7144464519868bb854c77e421acc Commit: 8e19f673801c7144464519868bb854c77e421acc Author: oshima@chromium.org Commiter: oshima@chromium.org Date: 2018-12-07 17:50:49 +0000 UTC PIP activation fix. This is reland of crrev.com/c/1357819. I changed the expectation for MACOSX_10_12. * The initial state is also wrong because widget overwite the activatable state. * Deactivate when it exits PIP. This may not be an issue for ChromePIP, but nothing prevent it from swithing state so it's better to move that logic to windowstate. * Cnsolidates the PIP activation logic in Chrome PIP and Android PIP into WindowState. Bug: 911267 TBR: edcourtney@chromium.org Test: covered by unittests Change-Id: Ia6b9f3beb9035debfef64fbc85d1ce9482047422 Reviewed-on: https://chromium-review.googlesource.com/c/1357819 Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Eliot Courtney <edcourtney@chromium.org> Commit-Queue: Mitsuru Oshima <oshima@chromium.org> Cr-Original-Original-Commit-Position: refs/heads/master@{#613901} Reviewed-on: https://chromium-review.googlesource.com/c/1363204 Cr-Original-Commit-Position: refs/heads/master@{#614136}(cherry picked from commit ea9a040e5b7974b5317c15948f4bd72644aad94b) Reviewed-on: https://chromium-review.googlesource.com/c/1367891 Reviewed-by: Mitsuru Oshima <oshima@chromium.org> Cr-Commit-Position: refs/branch-heads/3626@{#137} Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437} |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by osh...@chromium.org
, Dec 4