[CodeHealth] Clean up WebContentsCaptureHiDPI feature
The feature was launched in M114, cleaning up the code. Note: the cleanup was attempted in the past in crrev.com/c/4977161, but was reverted due to MSAN failures. Some tests were reworked or removed in the past year, and now there are no failures when I run the same linux_chromium_chromeos_msan_rel_ng bot, so the cleanup can be landed again. Bug: 356624220 Change-Id: Ie8c22105eea8a8f6d5e87cee7daba14f3f324fc5 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6120408 Reviewed-by: Thomas Guilbert <tguilbert@chromium.org> Commit-Queue: Maria Kazinova <kazinova@google.com> Reviewed-by: Alex Moshchuk <alexmos@chromium.org> Reviewed-by: Jordan Bayles <jophba@chromium.org> Cr-Commit-Position: refs/heads/main@{#1401376}
This commit is contained in:

committed by
Chromium LUCI CQ

parent
1a3bcf8d9e
commit
3de24d549a
chrome/browser/media/webrtc
content/browser
media
capture
renderer_host
media/base
testing/variations
tools/metrics/histograms
@ -128,8 +128,7 @@ struct TestConfigForFakeUI {
|
||||
const char* display_surface;
|
||||
};
|
||||
|
||||
struct TestConfigForHiDpi {
|
||||
bool enable_hidpi;
|
||||
struct TestConfigForMediaResolution {
|
||||
int constraint_width;
|
||||
int constraint_height;
|
||||
};
|
||||
@ -923,29 +922,21 @@ IN_PROC_BROWSER_TEST_P(GetDisplayMediaVideoTrackBrowserTest, RunCombinedTest) {
|
||||
!(defined(MEMORY_SANITIZER) || defined(ADDRESS_SANITIZER))
|
||||
class GetDisplayMediaHiDpiBrowserTest
|
||||
: public WebRtcTestBase,
|
||||
public testing::WithParamInterface<TestConfigForHiDpi> {
|
||||
public testing::WithParamInterface<TestConfigForMediaResolution> {
|
||||
public:
|
||||
GetDisplayMediaHiDpiBrowserTest() : test_config_(GetParam()) {}
|
||||
|
||||
// The browser window size must be consistent with the
|
||||
// INSTANTIATE_TEST_SUITE_P TestConfigForHiDpi configurations below. See the
|
||||
// comments there for more details.
|
||||
// INSTANTIATE_TEST_SUITE_P TestConfigForMediaResolution configurations below.
|
||||
// See the comments there for more details.
|
||||
static constexpr int kBrowserWindowWidth = 800;
|
||||
static constexpr int kBrowserWindowHeight = 600;
|
||||
|
||||
bool enable_hidpi() const { return test_config_.enable_hidpi; }
|
||||
int constraint_width() const { return test_config_.constraint_width; }
|
||||
int constraint_height() const { return test_config_.constraint_height; }
|
||||
|
||||
void SetUpInProcessBrowserTestFixture() override {
|
||||
if (enable_hidpi()) {
|
||||
feature_list_.InitAndEnableFeature(media::kWebContentsCaptureHiDpi);
|
||||
} else {
|
||||
feature_list_.InitAndDisableFeature(media::kWebContentsCaptureHiDpi);
|
||||
}
|
||||
|
||||
WebRtcTestBase::SetUpInProcessBrowserTestFixture();
|
||||
|
||||
DetectErrorsInJavaScript();
|
||||
}
|
||||
|
||||
@ -1012,8 +1003,7 @@ class GetDisplayMediaHiDpiBrowserTest
|
||||
.ExtractString();
|
||||
}
|
||||
|
||||
base::test::ScopedFeatureList feature_list_;
|
||||
const TestConfigForHiDpi test_config_;
|
||||
const TestConfigForMediaResolution test_config_;
|
||||
raw_ptr<content::WebContents, AcrossTasksDanglingUntriaged> tab_ = nullptr;
|
||||
};
|
||||
|
||||
@ -1048,10 +1038,8 @@ IN_PROC_BROWSER_TEST_P(GetDisplayMediaHiDpiBrowserTest, Capture) {
|
||||
WaitForVideoToPlay(Tab());
|
||||
|
||||
// If the video size is higher resolution than the browser window
|
||||
// size, expect that HiDPI mode should be active. This requires
|
||||
// the feature to be enabled.
|
||||
bool expect_hidpi = enable_hidpi() &&
|
||||
constraint_width() > kBrowserWindowWidth &&
|
||||
// size, expect that HiDPI mode should be active.
|
||||
bool expect_hidpi = constraint_width() > kBrowserWindowWidth &&
|
||||
constraint_height() > kBrowserWindowHeight;
|
||||
|
||||
double device_pixel_ratio = GetDevicePixelRatio();
|
||||
@ -1071,15 +1059,10 @@ INSTANTIATE_TEST_SUITE_P(
|
||||
// (cf. kBrowserWindowWidth and kBrowserWindowHeight in
|
||||
// GetDisplayMediaHiDpiBrowserTest above), and the large sizes must be
|
||||
// significantly larger than the browser window size.
|
||||
Values(TestConfigForHiDpi{/*enable_hidpi=*/false,
|
||||
/*constraint_width=*/3840,
|
||||
/*constraint_height=*/2160},
|
||||
TestConfigForHiDpi{/*enable_hidpi=*/true,
|
||||
/*constraint_width=*/640,
|
||||
/*constraint_height=*/480},
|
||||
TestConfigForHiDpi{/*enable_hidpi=*/true,
|
||||
/*constraint_width=*/3840,
|
||||
/*constraint_height=*/2160}));
|
||||
Values(TestConfigForMediaResolution{/*constraint_width=*/640,
|
||||
/*constraint_height=*/480},
|
||||
TestConfigForMediaResolution{/*constraint_width=*/3840,
|
||||
/*constraint_height=*/2160}));
|
||||
#endif
|
||||
|
||||
class GetDisplayMediaChangeSourceBrowserTest
|
||||
|
@ -38,10 +38,6 @@ class WebContentsAutoScalerTest : public ::testing::Test {
|
||||
WebContentsAutoScalerTest() = default;
|
||||
~WebContentsAutoScalerTest() override = default;
|
||||
|
||||
void SetUp() override {
|
||||
feature_list_.InitAndEnableFeature(media::kWebContentsCaptureHiDpi);
|
||||
}
|
||||
|
||||
void CreateAutoScaler(const gfx::Size& capture_size) {
|
||||
scaler_ = std::make_unique<WebContentsAutoScaler>(delegate_, capture_size);
|
||||
}
|
||||
|
@ -29,7 +29,6 @@
|
||||
#include "content/public/browser/web_contents.h"
|
||||
#include "content/public/browser/web_contents_media_capture_id.h"
|
||||
#include "content/public/browser/web_contents_observer.h"
|
||||
#include "media/base/media_switches.h"
|
||||
#include "media/base/video_util.h"
|
||||
#include "media/capture/mojom/video_capture_types.mojom.h"
|
||||
#include "media/capture/video_capture_types.h"
|
||||
@ -164,7 +163,6 @@ void WebContentsFrameTracker::WillStartCapturingWebContents(
|
||||
|
||||
capture_size_ = capture_size;
|
||||
if (is_high_dpi_enabled &&
|
||||
base::FeatureList::IsEnabled(media::kWebContentsCaptureHiDpi) &&
|
||||
!GpuDataManagerImpl::GetInstance()->IsGpuCompositingDisabled()) {
|
||||
auto_scaler_ =
|
||||
std::make_unique<WebContentsAutoScaler>(*context_, capture_size);
|
||||
|
@ -26,7 +26,6 @@
|
||||
#include "content/public/browser/web_contents.h"
|
||||
#include "content/public/test/browser_test.h"
|
||||
#include "content/shell/browser/shell.h"
|
||||
#include "media/base/media_switches.h"
|
||||
#include "media/base/video_frame.h"
|
||||
#include "media/base/video_types.h"
|
||||
#include "media/base/video_util.h"
|
||||
@ -52,10 +51,7 @@ class WebContentsVideoCaptureDeviceBrowserTest
|
||||
: public ContentCaptureDeviceBrowserTestBase,
|
||||
public FrameTestUtil {
|
||||
public:
|
||||
WebContentsVideoCaptureDeviceBrowserTest() {
|
||||
// TODO(https://crbug.com/1324757): tests should work with HiDPI enabled.
|
||||
scoped_feature_list_.InitAndDisableFeature(media::kWebContentsCaptureHiDpi);
|
||||
}
|
||||
WebContentsVideoCaptureDeviceBrowserTest() = default;
|
||||
|
||||
WebContentsVideoCaptureDeviceBrowserTest(
|
||||
const WebContentsVideoCaptureDeviceBrowserTest&) = delete;
|
||||
|
@ -565,6 +565,13 @@ class CONTENT_EXPORT RenderWidgetHostViewMac
|
||||
|
||||
void UpdateWindowsNow();
|
||||
|
||||
// For HiDPI capture mode, adjust the device scale factor to render the
|
||||
// contents at a higher pixel density when scale_override_for_capture_ > 1.0.
|
||||
// The first boolean returns true if any of the ScreenInfo elements in
|
||||
// `screen_infos_` was changed. The second boolean returns true if the current
|
||||
// ScreenInfo element was changed.
|
||||
std::pair<bool, bool> MaybeUpdateScreenInfosForHiDPI();
|
||||
|
||||
// Interface through which the NSView is to be manipulated. This points either
|
||||
// to |in_process_ns_view_bridge_| or to |remote_ns_view_|.
|
||||
raw_ptr<remote_cocoa::mojom::RenderWidgetHostNSView> ns_view_ = nullptr;
|
||||
|
@ -876,29 +876,9 @@ void RenderWidgetHostViewMac::UpdateScreenInfo() {
|
||||
new_screen_infos_from_shim_.reset();
|
||||
}
|
||||
|
||||
if (base::FeatureList::IsEnabled(media::kWebContentsCaptureHiDpi)) {
|
||||
// If HiDPI capture mode is active, adjust the device scale factor to
|
||||
// increase the rendered pixel count. |new_screen_infos| always contains
|
||||
// the unmodified original values for the display, and a copy of it is
|
||||
// saved in |screen_infos_|, with a modification applied if applicable.
|
||||
// When HiDPI mode is turned off (the scale override is 1.0), the original
|
||||
// |new_screen_infos| value gets copied unchanged to |screen_infos_|.
|
||||
display::ScreenInfos new_screen_infos = original_screen_infos_;
|
||||
const float old_device_scale_factor =
|
||||
new_screen_infos.current().device_scale_factor;
|
||||
new_screen_infos.mutable_current().device_scale_factor =
|
||||
old_device_scale_factor * scale_override_for_capture_;
|
||||
if (screen_infos_ != new_screen_infos) {
|
||||
DVLOG(1) << __func__ << ": Overriding device_scale_factor from "
|
||||
<< old_device_scale_factor << " to "
|
||||
<< new_screen_infos.current().device_scale_factor
|
||||
<< " for capture.";
|
||||
any_display_changed = true;
|
||||
current_display_changed |=
|
||||
new_screen_infos.current() != screen_infos_.current();
|
||||
screen_infos_ = new_screen_infos;
|
||||
}
|
||||
}
|
||||
std::pair<bool, bool> was_updated = MaybeUpdateScreenInfosForHiDPI();
|
||||
any_display_changed |= was_updated.first;
|
||||
current_display_changed |= was_updated.second;
|
||||
|
||||
bool dip_size_changed = view_bounds_in_window_dip_.size() !=
|
||||
browser_compositor_->GetRendererSize();
|
||||
@ -2428,6 +2408,32 @@ void RenderWidgetHostViewMac::UpdateWindowsNow() {
|
||||
[NSApp updateWindows];
|
||||
}
|
||||
|
||||
std::pair<bool, bool>
|
||||
RenderWidgetHostViewMac::MaybeUpdateScreenInfosForHiDPI() {
|
||||
// For HiDPI capture mode, adjust the device scale factor to
|
||||
// increase the rendered pixel count. |new_screen_infos| always contains
|
||||
// the unmodified original values for the display, and a copy of it is
|
||||
// saved in |screen_infos_|, with a modification applied if applicable.
|
||||
// When HiDPI mode is turned off (the scale override is 1.0), the original
|
||||
// |new_screen_infos| value gets copied unchanged to |screen_infos_|.
|
||||
display::ScreenInfos new_screen_infos = original_screen_infos_;
|
||||
const float old_device_scale_factor =
|
||||
new_screen_infos.current().device_scale_factor;
|
||||
new_screen_infos.mutable_current().device_scale_factor =
|
||||
old_device_scale_factor * scale_override_for_capture_;
|
||||
if (screen_infos_ != new_screen_infos) {
|
||||
DVLOG(1) << __func__ << ": Overriding device_scale_factor from "
|
||||
<< old_device_scale_factor << " to "
|
||||
<< new_screen_infos.current().device_scale_factor
|
||||
<< " for capture.";
|
||||
const bool current_display_changed =
|
||||
new_screen_infos.current() != screen_infos_.current();
|
||||
screen_infos_ = new_screen_infos;
|
||||
return {true, current_display_changed};
|
||||
}
|
||||
return {false, false};
|
||||
}
|
||||
|
||||
Class GetRenderWidgetHostViewCocoaClassForTesting() {
|
||||
return [RenderWidgetHostViewCocoa class];
|
||||
}
|
||||
|
@ -1006,12 +1006,6 @@ const base::FeatureParam<bool>
|
||||
kHardwareSecureDecryptionFallbackOnHardwareContextReset{
|
||||
&kHardwareSecureDecryptionFallback, "on_hardware_context_reset", true};
|
||||
|
||||
// If active, enable HiDPI mode that increases the display scale factor
|
||||
// while capturing a low-resolution tab.
|
||||
BASE_FEATURE(kWebContentsCaptureHiDpi,
|
||||
"WebContentsCaptureHiDPI",
|
||||
base::FEATURE_ENABLED_BY_DEFAULT);
|
||||
|
||||
// Enables handling of hardware media keys for controlling media.
|
||||
BASE_FEATURE(kHardwareMediaKeyHandling,
|
||||
"HardwareMediaKeyHandling",
|
||||
|
@ -377,7 +377,6 @@ MEDIA_EXPORT BASE_DECLARE_FEATURE(kCastVideoEncoderFrameDrop);
|
||||
MEDIA_EXPORT BASE_DECLARE_FEATURE(kWebCodecsVideoEncoderFrameDrop);
|
||||
MEDIA_EXPORT BASE_DECLARE_FEATURE(kWebRTCHardwareVideoEncoderFrameDrop);
|
||||
MEDIA_EXPORT BASE_DECLARE_FEATURE(kWebRTCColorAccuracy);
|
||||
MEDIA_EXPORT BASE_DECLARE_FEATURE(kWebContentsCaptureHiDpi);
|
||||
MEDIA_EXPORT BASE_DECLARE_FEATURE(kWebrtcMediaCapabilitiesParameters);
|
||||
MEDIA_EXPORT BASE_DECLARE_FEATURE(kResolutionBasedDecoderPriority);
|
||||
|
||||
|
@ -24650,24 +24650,6 @@
|
||||
]
|
||||
}
|
||||
],
|
||||
"WebContentsCaptureHiDPI": [
|
||||
{
|
||||
"platforms": [
|
||||
"chromeos",
|
||||
"linux",
|
||||
"mac",
|
||||
"windows"
|
||||
],
|
||||
"experiments": [
|
||||
{
|
||||
"name": "Enabled",
|
||||
"enable_features": [
|
||||
"WebContentsCaptureHiDPI"
|
||||
]
|
||||
}
|
||||
]
|
||||
}
|
||||
],
|
||||
"WebContentsDiscard": [
|
||||
{
|
||||
"platforms": [
|
||||
|
@ -15218,7 +15218,6 @@ from previous Chrome versions.
|
||||
<int value="-2072877761" label="WaylandUiScale:disabled"/>
|
||||
<int value="-2072754800" label="HelpAppAppsList:disabled"/>
|
||||
<int value="-2072471036" label="OobeHidDetectionRevamp:disabled"/>
|
||||
<int value="-2072181147" label="WebContentsCaptureHiDPI:enabled"/>
|
||||
<int value="-2071515296" label="allow-sync-xhr-in-page-dismissal"/>
|
||||
<int value="-2071202821" label="CopyLinkToText:enabled"/>
|
||||
<int value="-2069855464"
|
||||
@ -22872,7 +22871,6 @@ from previous Chrome versions.
|
||||
<int value="982032277" label="NTPOfflineBadge:disabled"/>
|
||||
<int value="982511393" label="NTPArticleSuggestions:disabled"/>
|
||||
<int value="982805190" label="PageInfoAboutThisSiteMoreLangs:enabled"/>
|
||||
<int value="982932009" label="WebContentsCaptureHiDPI:disabled"/>
|
||||
<int value="982983280" label="LauncherSearchControl:enabled"/>
|
||||
<int value="983084316" label="SyncUSSNigori:disabled"/>
|
||||
<int value="983236654" label="TrackingProtection3pcd:enabled"/>
|
||||
|
Reference in New Issue
Block a user