Remove kCapturedSurfaceControlTemporaryZoom
1. This flag has been default-on for long enough that it may now be safely removed from the codebase. 2. Existing tests are reanchored on a different fixture. 3. A minor expansion of test coverage is added, ensuring the side-effect is not experienced by newly opened (and navigated) tabs after the original one is closed. Bug: 328589994 Change-Id: Ie295a473515ac3a20ee47e5596144615c7aa0ca0 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6316730 Reviewed-by: Avi Drissman <avi@chromium.org> Commit-Queue: Elad Alon <eladalon@chromium.org> Auto-Submit: Elad Alon <eladalon@chromium.org> Reviewed-by: Tove Petersson <tovep@chromium.org> Cr-Commit-Position: refs/heads/main@{#1427678}
This commit is contained in:
Elad Alon
committed by
Chromium LUCI CQ
Chromium LUCI CQ
parent
3bd0bdf320
commit
3c74eb4662
content
@ -6,14 +6,12 @@
|
||||
|
||||
#include <cmath>
|
||||
|
||||
#include "base/feature_list.h"
|
||||
#include "base/task/bind_post_task.h"
|
||||
#include "content/browser/media/captured_surface_control_permission_manager.h"
|
||||
#include "content/browser/media/media_stream_web_contents_observer.h"
|
||||
#include "content/browser/renderer_host/render_frame_host_impl.h"
|
||||
#include "content/browser/renderer_host/render_widget_host_impl.h"
|
||||
#include "content/browser/web_contents/web_contents_impl.h"
|
||||
#include "content/common/features.h"
|
||||
#include "content/public/browser/browser_thread.h"
|
||||
#include "content/public/browser/global_routing_id.h"
|
||||
#include "content/public/browser/host_zoom_map.h"
|
||||
@ -205,15 +203,6 @@ CapturedSurfaceControlResult DoSetZoomLevel(
|
||||
return CapturedSurfaceControlResult::kDisallowedForSelfCaptureError;
|
||||
}
|
||||
|
||||
// TODO(crbug.com/328589994): Hard-code kCapturedSurfaceControlTemporaryZoom.
|
||||
if (!base::FeatureList::IsEnabled(
|
||||
features::kCapturedSurfaceControlTemporaryZoom)) {
|
||||
HostZoomMap::SetZoomLevel(
|
||||
captured_wc.get(),
|
||||
blink::ZoomFactorToZoomLevel(static_cast<double>(zoom_level) / 100));
|
||||
return CapturedSurfaceControlResult::kSuccess;
|
||||
}
|
||||
|
||||
HostZoomMap* const zoom_map =
|
||||
HostZoomMap::GetForWebContents(captured_wc.get());
|
||||
if (!zoom_map) {
|
||||
|
||||
@ -11,10 +11,8 @@
|
||||
#include "base/functional/bind.h"
|
||||
#include "base/notreached.h"
|
||||
#include "base/run_loop.h"
|
||||
#include "base/test/scoped_feature_list.h"
|
||||
#include "content/browser/host_zoom_map_impl.h"
|
||||
#include "content/browser/media/captured_surface_control_permission_manager.h"
|
||||
#include "content/common/features.h"
|
||||
#include "content/public/browser/global_routing_id.h"
|
||||
#include "content/public/test/browser_task_environment.h"
|
||||
#include "content/test/test_web_contents.h"
|
||||
@ -541,28 +539,16 @@ TEST_P(CapturedSurfaceControllerSetZoomLevelTest, SetZoomLevelSuccess) {
|
||||
EXPECT_EQ(zoom_level_, capturee_->GetZoomLevel());
|
||||
}
|
||||
|
||||
class CapturedSurfaceControllerSetZoomTemporarinessTest
|
||||
: public CapturedSurfaceControllerTestBase,
|
||||
public ::testing::WithParamInterface<bool> {
|
||||
class CapturedSurfaceControllerSetZoomLevelImpermanenceTest
|
||||
: public CapturedSurfaceControllerTestBase {
|
||||
public:
|
||||
CapturedSurfaceControllerSetZoomTemporarinessTest()
|
||||
: is_temporary_(GetParam()) {
|
||||
features_.InitWithFeatureStates(
|
||||
{{features::kCapturedSurfaceControlTemporaryZoom, is_temporary_}});
|
||||
}
|
||||
~CapturedSurfaceControllerSetZoomTemporarinessTest() override = default;
|
||||
|
||||
protected:
|
||||
const bool is_temporary_;
|
||||
base::test::ScopedFeatureList features_;
|
||||
~CapturedSurfaceControllerSetZoomLevelImpermanenceTest() override = default;
|
||||
};
|
||||
|
||||
INSTANTIATE_TEST_SUITE_P(,
|
||||
CapturedSurfaceControllerSetZoomTemporarinessTest,
|
||||
::testing::Bool());
|
||||
|
||||
TEST_P(CapturedSurfaceControllerSetZoomTemporarinessTest,
|
||||
TemporarinessDependsOnConfiguration) {
|
||||
// Ensure the effect does not extend to other tabs, even if they are dialed
|
||||
// to the same origin.
|
||||
TEST_F(CapturedSurfaceControllerSetZoomLevelImpermanenceTest,
|
||||
SetZoomLevelOnlyAffectsCapturedTab) {
|
||||
ASSERT_EQ(capturee_->GetZoomLevel(), 100);
|
||||
|
||||
// Create another tab and navigate it to the same URL as the captured tab.
|
||||
@ -579,12 +565,36 @@ TEST_P(CapturedSurfaceControllerSetZoomTemporarinessTest,
|
||||
run_loop.Run();
|
||||
ASSERT_EQ(capturee_->GetZoomLevel(), 200);
|
||||
|
||||
// In temporary zoom mode, setting the zoom level only affects the
|
||||
// current tab.
|
||||
// In persistent zoom mode, setting the zoom is equivalent to
|
||||
// the user changing it through their interaction with the
|
||||
// browser's UX.
|
||||
EXPECT_EQ(duplicate_tab->GetZoomLevel(), is_temporary_ ? 100 : 200);
|
||||
// Setting the zoom level only affected the captured tab, not the
|
||||
// Chrome-level settings for the origin.
|
||||
EXPECT_EQ(duplicate_tab->GetZoomLevel(), 100);
|
||||
}
|
||||
|
||||
// Ensure the effect does not get persisted and does not affect newly
|
||||
// opened tabs later, even if they are navigated to the same URL.
|
||||
TEST_F(CapturedSurfaceControllerSetZoomLevelImpermanenceTest,
|
||||
SetZoomLevelEffectsDoNotPersistAfterClosed) {
|
||||
ASSERT_EQ(capturee_->GetZoomLevel(), 100);
|
||||
|
||||
// Change the zoom-level on the captured tab.
|
||||
permission_manager_->SetPermissionResult(CSCPermissionResult::kGranted);
|
||||
base::RunLoop run_loop;
|
||||
controller_->SetZoomLevel(
|
||||
200, MakeCallbackExpectingResult(&run_loop, CSCResult::kSuccess,
|
||||
mock_widget_input_handler_.get()));
|
||||
run_loop.Run();
|
||||
ASSERT_EQ(capturee_->GetZoomLevel(), 200);
|
||||
|
||||
// Close the tab.
|
||||
capturee_.reset();
|
||||
|
||||
// Create another tab and navigate it to the same URL as the captured tab.
|
||||
auto new_tab =
|
||||
std::make_unique<TestTab>(GetBrowserContext(), GURL(kUrlString));
|
||||
|
||||
// Setting the zoom level only affected the captured tab, not the
|
||||
// Chrome-level settings for the origin.
|
||||
EXPECT_EQ(new_tab->GetZoomLevel(), 100);
|
||||
}
|
||||
|
||||
enum class CapturedSurfaceControlAPI {
|
||||
|
||||
@ -65,13 +65,6 @@ BASE_FEATURE(kHidePastePopupOnGSB,
|
||||
base::FEATURE_ENABLED_BY_DEFAULT);
|
||||
#endif
|
||||
|
||||
// If enabled, changes to the zoom level are temporary and are forgotten when
|
||||
// the tab is closed. If disabled, changes to the zoom level persist, as though
|
||||
// the user affected them through the browser's UX.
|
||||
BASE_FEATURE(kCapturedSurfaceControlTemporaryZoom,
|
||||
"CapturedSurfaceControlTemporaryZoom",
|
||||
base::FEATURE_ENABLED_BY_DEFAULT);
|
||||
|
||||
// If Canvas2D Image Chromium is allowed, this feature controls whether it is
|
||||
// enabled.
|
||||
BASE_FEATURE(kCanvas2DImageChromium,
|
||||
|
||||
@ -20,7 +20,6 @@ CONTENT_EXPORT BASE_DECLARE_FEATURE(kBackForwardCacheTimeToLiveControl);
|
||||
BASE_DECLARE_FEATURE(kBeforeUnloadBrowserResponseQueue);
|
||||
CONTENT_EXPORT BASE_DECLARE_FEATURE(
|
||||
kBlockInsecurePrivateNetworkRequestsFromUnknown);
|
||||
CONTENT_EXPORT BASE_DECLARE_FEATURE(kCapturedSurfaceControlTemporaryZoom);
|
||||
CONTENT_EXPORT BASE_DECLARE_FEATURE(kCanvas2DImageChromium);
|
||||
CONTENT_EXPORT BASE_DECLARE_FEATURE(kCompositeClipPathAnimation);
|
||||
CONTENT_EXPORT BASE_DECLARE_FEATURE(kCodeCacheDeletionWithoutFilter);
|
||||
|
||||
Reference in New Issue
Block a user