Skip to content

Commit

Permalink
Cleanup SplitUserMediaQueues feature flag
Browse files Browse the repository at this point in the history
Bug: 1373032
Change-Id: I79c91fe83fe00e6a817388e4016a9cf126e9da26
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4973841
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: Tove Petersson <tovep@chromium.org>
Reviewed-by: Tony Herre <toprice@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1216854}
  • Loading branch information
Tove Petersson authored and Chromium LUCI CQ committed Oct 30, 2023
1 parent 3f4956c commit fcd6adf
Show file tree
Hide file tree
Showing 8 changed files with 80 additions and 119 deletions.
4 changes: 0 additions & 4 deletions third_party/blink/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1836,10 +1836,6 @@ const base::FeatureParam<bool> kSpeculativeServiceWorkerWarmUpOnPointerover{
const base::FeatureParam<bool> kSpeculativeServiceWorkerWarmUpOnPointerdown{
&kSpeculativeServiceWorkerWarmUp, "sw_warm_up_on_pointerdown", true};

BASE_FEATURE(kSplitUserMediaQueues,
"SplitUserMediaQueues",
base::FEATURE_ENABLED_BY_DEFAULT);

BASE_FEATURE(kStartMediaStreamCaptureIndicatorInBrowser,
"StartMediaStreamCaptureIndicatorInBrowser",
base::FEATURE_ENABLED_BY_DEFAULT);
Expand Down
3 changes: 0 additions & 3 deletions third_party/blink/public/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -1197,9 +1197,6 @@ BLINK_COMMON_EXPORT extern const base::FeatureParam<bool>
BLINK_COMMON_EXPORT extern const base::FeatureParam<bool>
kSpeculativeServiceWorkerWarmUpOnPointerdown;

// Process device and display capture requests on different queues.
BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(kSplitUserMediaQueues);

// Make the browser decide when to turn on the capture indicator (red button)
BLINK_COMMON_EXPORT BASE_DECLARE_FEATURE(
kStartMediaStreamCaptureIndicatorInBrowser);
Expand Down
54 changes: 27 additions & 27 deletions third_party/blink/renderer/modules/mediastream/user_media_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -282,10 +282,11 @@ UserMediaClient::UserMediaClient(
ExecutionContextLifecycleObserver(frame->DomWindow()),
frame_(frame),
media_devices_dispatcher_(frame->DomWindow()),
pending_requests_(MakeGarbageCollected<RequestQueue>(frame,
user_media_processor,
this,
task_runner)),
pending_device_requests_(
MakeGarbageCollected<RequestQueue>(frame,
user_media_processor,
this,
task_runner)),
pending_display_requests_(
display_user_media_processor
? MakeGarbageCollected<RequestQueue>(frame,
Expand Down Expand Up @@ -317,20 +318,18 @@ UserMediaClient::UserMediaClient(
},
WrapWeakPersistent(this)),
frame->GetTaskRunner(blink::TaskType::kInternalMedia)),
base::FeatureList::IsEnabled(features::kSplitUserMediaQueues)
? MakeGarbageCollected<UserMediaProcessor>(
frame,
WTF::BindRepeating(
[](UserMediaClient* client)
-> mojom::blink::MediaDevicesDispatcherHost* {
// |client| is guaranteed to be not null because
// |client| transitively owns this UserMediaProcessor.
DCHECK(client);
return client->GetMediaDevicesDispatcher();
},
WrapWeakPersistent(this)),
frame->GetTaskRunner(blink::TaskType::kInternalMedia))
: nullptr,
MakeGarbageCollected<UserMediaProcessor>(
frame,
WTF::BindRepeating(
[](UserMediaClient* client)
-> mojom::blink::MediaDevicesDispatcherHost* {
// |client| is guaranteed to be not null because
// |client| transitively owns this UserMediaProcessor.
DCHECK(client);
return client->GetMediaDevicesDispatcher();
},
WrapWeakPersistent(this)),
frame->GetTaskRunner(blink::TaskType::kInternalMedia)),
std::move(task_runner)) {}

void UserMediaClient::RequestUserMedia(UserMediaRequest* user_media_request) {
Expand Down Expand Up @@ -415,7 +414,7 @@ void UserMediaClient::StopTrack(MediaStreamComponent* track) {
}

bool UserMediaClient::IsCapturing() {
return pending_requests_->IsCapturing() ||
return pending_device_requests_->IsCapturing() ||
(pending_display_requests_ &&
pending_display_requests_->IsCapturing());
}
Expand All @@ -424,15 +423,15 @@ bool UserMediaClient::IsCapturing() {
void UserMediaClient::FocusCapturedSurface(const String& label, bool focus) {
// Get queue with display capture requests. Only display capturer can be
// focused.
RequestQueue* queue =
pending_display_requests_ ? pending_display_requests_ : pending_requests_;
RequestQueue* queue = pending_display_requests_ ? pending_display_requests_
: pending_device_requests_;
queue->FocusCapturedSurface(label, focus);
}
#endif

void UserMediaClient::CancelUserMediaRequest(
UserMediaRequest* user_media_request) {
pending_requests_->CancelUserMediaRequest(user_media_request);
pending_device_requests_->CancelUserMediaRequest(user_media_request);
if (pending_display_requests_)
pending_display_requests_->CancelUserMediaRequest(user_media_request);
}
Expand All @@ -441,7 +440,7 @@ void UserMediaClient::DeleteAllUserMediaRequests() {
DCHECK_CALLED_ON_VALID_THREAD(thread_checker_);
if (frame_)
frame_->SetIsCapturingMediaCallback(LocalFrame::IsCapturingMediaCallback());
pending_requests_->DeleteAllUserMediaRequests();
pending_device_requests_->DeleteAllUserMediaRequests();
if (pending_display_requests_)
pending_display_requests_->DeleteAllUserMediaRequests();
}
Expand All @@ -457,7 +456,7 @@ void UserMediaClient::Trace(Visitor* visitor) const {
ExecutionContextLifecycleObserver::Trace(visitor);
visitor->Trace(frame_);
visitor->Trace(media_devices_dispatcher_);
visitor->Trace(pending_requests_);
visitor->Trace(pending_device_requests_);
visitor->Trace(pending_display_requests_);
}

Expand Down Expand Up @@ -504,8 +503,9 @@ void UserMediaClient::KeepDeviceAliveForTransfer(
UserMediaProcessor::KeepDeviceAliveForTransferCallback keep_alive_cb) {
// Get queue with display capture requests. Only display capture requests are
// supported for transfer.
UserMediaClient::RequestQueue* queue =
pending_display_requests_ ? pending_display_requests_ : pending_requests_;
UserMediaClient::RequestQueue* queue = pending_display_requests_
? pending_display_requests_
: pending_device_requests_;
queue->KeepDeviceAliveForTransfer(session_id, transfer_id,
std::move(keep_alive_cb));
}
Expand All @@ -518,7 +518,7 @@ UserMediaClient::RequestQueue* UserMediaClient::GetRequestQueue(
mojom::blink::MediaStreamType::DISPLAY_AUDIO_CAPTURE)) {
return pending_display_requests_.Get();
} else {
return pending_requests_.Get();
return pending_device_requests_.Get();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,14 +141,7 @@ class MODULES_EXPORT UserMediaClient
HeapMojoRemote<blink::mojom::blink::MediaDevicesDispatcherHost>
media_devices_dispatcher_;

// TODO(crbug.com/1373032): Rename pending_requests_ to
// pending_device_requests_ when the kSplitUserMediaQueues feature flag is
// removed and update the comments below accordingly.
// Default queue for all requests if kSplitUserMediaQueues is disabled, or
// device requests only if kSplitUserMediaQueues is enabled.
Member<RequestQueue> pending_requests_;
// Queue for display requests if kSplitUserMediaQueues is enabled, or set to
// nullptr otherwise.
Member<RequestQueue> pending_device_requests_;
Member<RequestQueue> pending_display_requests_;

THREAD_CHECKER(thread_checker_);
Expand Down

0 comments on commit fcd6adf

Please sign in to comment.