Skip to content

GitLab

T
tangled
Commit 005f82ed authored by Will Cassella's avatar Will Cassella Committed by Commit Bot
Browse files
Options

Refactoring changes to VRServiceImpl 

This CL performs some cleanup to the VRServiceImpl class.

1) Removed 'IsAnotherHostPresenting()' declaration, since that function
was never actually defined anywhere.

2) It inlines 'SetInFocusedFrame', since there was only a single call
to it, and it makes the code somewhat easier to follow.

3) Moved the check on 'initialization_complete_' inside of
'RequestSession' to the top of the function in order to be more
consistent with other functions in this class.

4) Removed null checks for 'render_frame_host_', because it's not
allowed to be null.

Change-Id: Ice0433923b747a3fda3e5e0178af2260b8b14d07
Bug: 846392
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1764549
Commit-Queue: Will Cassella <cassew@google.com>
Reviewed-by: default avatarAlexander Cooper <alcooper@chromium.org>
Reviewed-by: default avatarKlaus Weidner <klausw@chromium.org>
Cr-Commit-Position: refs/heads/master@{#691738}
parent 86faed3c
Hide whitespace changes
Inline Side-by-side
Showing with 58 additions and 108 deletions
chrome/browser/vr/service/vr_service_impl.cc
View file @ 005f82ed
......@@ -107,11 +107,7 @@ VRServiceImpl::VRServiceImpl(content::RenderFrameHost* render_frame_host)
: WebContentsObserver(
content::WebContents::FromRenderFrameHost(render_frame_host)),
render_frame_host_(render_frame_host),
// TODO(https://crbug.com/846392): render_frame_host can be null because
// of a test, not because a VRServiceImpl can be created without it.
in_focused_frame_(render_frame_host
? render_frame_host->GetView()->HasFocus()
: false) {
in_focused_frame_(render_frame_host->GetView()->HasFocus()) {
DCHECK(render_frame_host_);
runtime_manager_ = XRRuntimeManager::GetOrCreateInstance();
......@@ -124,7 +120,8 @@ VRServiceImpl::VRServiceImpl(content::RenderFrameHost* render_frame_host)
}
// Constructor for testing.
VRServiceImpl::VRServiceImpl() : render_frame_host_(nullptr) {
VRServiceImpl::VRServiceImpl(util::PassKey<XRRuntimeManagerTest>)
: render_frame_host_(nullptr) {
runtime_manager_ = XRRuntimeManager::GetOrCreateInstance();
runtime_manager_->AddService(this);
}
......@@ -212,7 +209,19 @@ void VRServiceImpl::OnWebContentsFocusChanged(content::RenderWidgetHost* host,
render_frame_host_->GetView()->GetRenderWidgetHost() != host) {
return;
}
SetInFocusedFrame(focused);
in_focused_frame_ = focused;
if (ListeningForActivate()) {
BrowserXRRuntime* immersive_runtime =
runtime_manager_->GetImmersiveRuntime();
if (immersive_runtime)
immersive_runtime->UpdateListeningForActivate(this);
}
magic_window_controllers_.ForAllPtrs(
[focused](device::mojom::XRSessionController* controller) {
controller->SetFrameDataRestricted(!focused);
});
}
// static
......@@ -302,6 +311,14 @@ void VRServiceImpl::RequestSession(
device::mojom::VRService::RequestSessionCallback callback) {
DCHECK(options);
// Queue the request to get to when initialization has completed.
if (!initialization_complete_) {
pending_requests_.push_back(
base::BindOnce(&VRServiceImpl::RequestSession, base::Unretained(this),
std::move(options), std::move(callback)));
return;
}
// Check that the request satisfies secure context requirements.
if (!IsSecureContextRequirementSatisfied()) {
std::move(callback).Run(
......@@ -327,14 +344,6 @@ void VRServiceImpl::RequestSession(
return;
}
// Queue the request to get to when initialization has completed.
if (!initialization_complete_) {
pending_requests_.push_back(
base::BindOnce(&VRServiceImpl::RequestSession, base::Unretained(this),
std::move(options), std::move(callback)));
return;
}
auto* runtime = runtime_manager_->GetRuntimeForOptions(options.get());
if (!runtime) {
std::move(callback).Run(
......@@ -371,6 +380,10 @@ void VRServiceImpl::ShowConsentPrompt(
DCHECK(options);
DCHECK(runtime);
#if defined(OS_WIN)
DCHECK(!options->environment_integration);
#endif
// WebVR did not require permissions.
// TODO(crbug.com/968221): Address privacy requirements for inline sessions
if (options->is_legacy_webvr) {
......@@ -385,7 +398,8 @@ void VRServiceImpl::ShowConsentPrompt(
// Skip the consent prompt if the user has already consented for this device,
// or if consent is not needed.
if (consent_level == XrConsentPromptLevel::kNone ||
IsConsentGrantedForDevice(runtime->GetId(), consent_level)) {
IsConsentGrantedForDevice(runtime->GetId(), consent_level) ||
IsXrDeviceConsentPromptDisabledForTesting()) {
DoRequestSession(std::move(options), std::move(callback),
std::move(requested_features));
return;
......@@ -393,75 +407,40 @@ void VRServiceImpl::ShowConsentPrompt(
// TODO(crbug.com/968233): Unify the below consent flow.
#if defined(OS_ANDROID)
if (options->environment_integration) {
#if BUILDFLAG(ENABLE_ARCORE)
if (!render_frame_host_) {
// Reject promise.
std::move(callback).Run(
device::mojom::RequestSessionResult::NewFailureReason(
device::mojom::RequestSessionError::INVALID_CLIENT));
} else {
if (IsXrDeviceConsentPromptDisabledForTesting()) {
DoRequestSession(std::move(options), std::move(callback),
std::move(requested_features));
} else {
ArCoreConsentPromptInterface::GetInstance()->ShowConsentPrompt(
render_frame_host_->GetProcess()->GetID(),
render_frame_host_->GetRoutingID(),
base::BindOnce(&VRServiceImpl::OnConsentResult,
weak_ptr_factory_.GetWeakPtr(), std::move(options),
std::move(callback), std::move(requested_features),
consent_level));
}
}
ArCoreConsentPromptInterface::GetInstance()->ShowConsentPrompt(
render_frame_host_->GetProcess()->GetID(),
render_frame_host_->GetRoutingID(),
base::BindOnce(&VRServiceImpl::OnConsentResult,
weak_ptr_factory_.GetWeakPtr(), std::move(options),
std::move(callback), std::move(requested_features),
consent_level));
return;
#else
std::move(callback).Run(nullptr);
#endif
return;
#endif
} else {
// GVR.
if (!render_frame_host_) {
// Reject promise.
std::move(callback).Run(
device::mojom::RequestSessionResult::NewFailureReason(
device::mojom::RequestSessionError::INVALID_CLIENT));
} else {
if (IsXrDeviceConsentPromptDisabledForTesting()) {
DoRequestSession(std::move(options), std::move(callback),
std::move(requested_features));
} else {
GvrConsentHelper::GetInstance()->PromptUserAndGetConsent(
render_frame_host_->GetProcess()->GetID(),
render_frame_host_->GetRoutingID(), consent_level,
base::BindOnce(&VRServiceImpl::OnConsentResult,
weak_ptr_factory_.GetWeakPtr(), std::move(options),
std::move(callback), std::move(requested_features)));
}
}
return;
}
#elif defined(OS_WIN)
DCHECK(!options->environment_integration);
if (IsXrDeviceConsentPromptDisabledForTesting()) {
DoRequestSession(std::move(options), std::move(callback),
std::move(requested_features));
} else {
XRSessionRequestConsentManager::Instance()->ShowDialogAndGetConsent(
GetWebContents(), consent_level,
GvrConsentHelper::GetInstance()->PromptUserAndGetConsent(
render_frame_host_->GetProcess()->GetID(),
render_frame_host_->GetRoutingID(), consent_level,
base::BindOnce(&VRServiceImpl::OnConsentResult,
weak_ptr_factory_.GetWeakPtr(), std::move(options),
std::move(callback), std::move(requested_features)));
return;
}
#elif defined(OS_WIN)
XRSessionRequestConsentManager::Instance()->ShowDialogAndGetConsent(
GetWebContents(), consent_level,
base::BindOnce(&VRServiceImpl::OnConsentResult,
weak_ptr_factory_.GetWeakPtr(), std::move(options),
std::move(callback), std::move(requested_features)));
return;
#else
#endif
NOTREACHED();
#endif
}
void VRServiceImpl::OnConsentResult(
......@@ -521,12 +500,6 @@ void VRServiceImpl::DoRequestSession(
#if defined(OS_ANDROID) && BUILDFLAG(ENABLE_ARCORE)
if (session_runtime_id == device::mojom::XRDeviceId::ARCORE_DEVICE_ID) {
if (!render_frame_host_) {
std::move(callback).Run(
device::mojom::RequestSessionResult::NewFailureReason(
device::mojom::RequestSessionError::INVALID_CLIENT));
return;
}
runtime_options->render_process_id =
render_frame_host_->GetProcess()->GetID();
runtime_options->render_frame_id = render_frame_host_->GetRoutingID();
......@@ -600,21 +573,6 @@ void VRServiceImpl::GetImmersiveVRDisplayInfo(
std::move(callback).Run(nullptr);
}
void VRServiceImpl::SetInFocusedFrame(bool in_focused_frame) {
in_focused_frame_ = in_focused_frame;
if (ListeningForActivate()) {
BrowserXRRuntime* immersive_runtime =
runtime_manager_->GetImmersiveRuntime();
if (immersive_runtime)
immersive_runtime->UpdateListeningForActivate(this);
}
magic_window_controllers_.ForAllPtrs(
[&in_focused_frame](device::mojom::XRSessionController* controller) {
controller->SetFrameDataRestricted(!in_focused_frame);
});
}
void VRServiceImpl::OnExitPresent() {
session_clients_.ForAllPtrs(
[](device::mojom::XRSessionClient* client) { client->OnExitPresent(); });
......@@ -646,14 +604,7 @@ void VRServiceImpl::OnDeactivate(device::mojom::VRDisplayEventReason reason) {
}
content::WebContents* VRServiceImpl::GetWebContents() {
if (render_frame_host_ != nullptr) {
return content::WebContents::FromRenderFrameHost(render_frame_host_);
}
// We should only have a null render_frame_host_ for some unittests, for which
// we don't actually expect to get here.
NOTREACHED();
return nullptr;
return content::WebContents::FromRenderFrameHost(render_frame_host_);
}
bool VRServiceImpl::IsSecureContextRequirementSatisfied() {
......
chrome/browser/vr/service/vr_service_impl.h
View file @ 005f82ed
......@@ -11,6 +11,7 @@
#include <vector>
#include "base/macros.h"
#include "base/util/type_safety/pass_key.h"
#include "chrome/browser/vr/metrics/session_metrics_helper.h"
#include "chrome/browser/vr/service/interface_set.h"
......@@ -42,10 +43,13 @@ class BrowserXRRuntime;
class VR_EXPORT VRServiceImpl : public device::mojom::VRService,
content::WebContentsObserver {
public:
friend XRRuntimeManagerTest;
static bool IsXrDeviceConsentPromptDisabledForTesting();
explicit VRServiceImpl(content::RenderFrameHost* render_frame_host);
// Constructor for tests.
explicit VRServiceImpl(util::PassKey<XRRuntimeManagerTest>);
~VRServiceImpl() override;
static void Create(content::RenderFrameHost* render_frame_host,
......@@ -93,12 +97,8 @@ class VR_EXPORT VRServiceImpl : public device::mojom::VRService,
void SetListeningForActivate(
device::mojom::VRDisplayClientPtr display_client) override;
void SetInFocusedFrame(bool in_focused_frame);
private:
// Constructor for tests.
VRServiceImpl();
// content::WebContentsObserver implementation
void OnWebContentsFocused(content::RenderWidgetHost* host) override;
void OnWebContentsLostFocus(content::RenderWidgetHost* host) override;
......@@ -109,8 +109,6 @@ class VR_EXPORT VRServiceImpl : public device::mojom::VRService,
void ResolvePendingRequests();
bool IsSecureContextRequirementSatisfied();
bool IsAnotherHostPresenting();
// Returns currently active instance of SessionMetricsHelper from WebContents.
// If the instance is not present on WebContents, it will be created with the
// assumption that we are not already in VR.
......
chrome/browser/vr/service/xr_runtime_manager_unittest.cc
View file @ 005f82ed
......@@ -44,7 +44,8 @@ class XRRuntimeManagerTest : public testing::Test {
std::unique_ptr<VRServiceImpl> BindService() {
mojo::PendingRemote<device::mojom::VRServiceClient> proxy;
device::FakeVRServiceClient client(proxy.InitWithNewPipeAndPassReceiver());
auto service = base::WrapUnique(new VRServiceImpl());
auto service =
std::make_unique<VRServiceImpl>(util::PassKey<XRRuntimeManagerTest>());
service->SetClient(std::move(proxy));
return service;
}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment