0

Projector: Address comments from crrev/c/3016366

This CL is a follow-up to crrev/c/3016366 and addresses remaining
comments. This CL separates session active state from toolbar
visibility. It also adds TODOs about future migration plans.

Bug: 1213937
Change-Id: Iaffe53abd2e61050fbbc51da7721d9163bf9ef3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3025076
Reviewed-by: Yilkal Abe <yilkal@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: Li Lin <llin@chromium.org>
Commit-Queue: Toby Huang <tobyhuang@chromium.org>
Cr-Commit-Position: refs/heads/master@{#902735}
This commit is contained in:
Toby Huang
2021-07-16 23:37:00 +00:00
committed by Chromium LUCI CQ
parent 0144bf94ca
commit a735fb228a
12 changed files with 57 additions and 30 deletions

@ -19,11 +19,10 @@ class ASH_EXPORT ProjectorSessionImpl : public ProjectorSession {
ProjectorSessionImpl& operator=(const ProjectorSessionImpl&) = delete;
~ProjectorSessionImpl() override;
// Starts or stops the projector session.
void Start(SourceType preset_source_type = SourceType::kUnset);
void Stop();
// ProjectorSession:
void Start(SourceType preset_source_type) override;
void Stop() override;
void AddObserver(ProjectorSessionObserver* observer) override;
void RemoveObserver(ProjectorSessionObserver* observer) override;

@ -4,6 +4,7 @@
#include "ash/projector/model/projector_session_impl.h"
#include "ash/public/cpp/projector/projector_session.h"
#include "base/dcheck_is_on.h"
#include "testing/gtest/include/gtest/gtest.h"
@ -24,7 +25,7 @@ class ProjectorSessionImplTest : public testing::Test {
};
TEST_F(ProjectorSessionImplTest, Start) {
session_->Start();
session_->Start(SourceType::kUnset);
EXPECT_TRUE(session_->is_active());
ASSERT_EQ(SourceType::kUnset, session_->preset_source_type());
@ -45,9 +46,9 @@ TEST_F(ProjectorSessionImplTest, StartWithPresetSourceType) {
#if DCHECK_IS_ON()
TEST_F(ProjectorSessionImplTest, OnlyOneProjectorSessionAllowed) {
session_->Start();
EXPECT_DEATH_IF_SUPPORTED(session_->Start(), "");
session_->Start(SourceType::kUnset);
EXPECT_DEATH_IF_SUPPORTED(session_->Start(SourceType::kUnset), "");
}
#endif
} // namespace ash
} // namespace ash

@ -57,20 +57,20 @@ void ProjectorControllerImpl::SetProjectorToolsVisible(bool is_visible) {
// available.
if (is_visible) {
ui_controller_->ShowToolbar();
// TODO(crbug/1206720): Move elsewhere once screen capture integration
// finalized.
OnRecordingStarted();
return;
}
// TODO(crbug/1206720): Move elsewhere once screen capture integration
// finalized.
OnRecordingEnded();
if (client_->IsSelfieCamVisible())
client_->CloseSelfieCam();
ui_controller_->CloseToolbar();
}
bool ProjectorControllerImpl::AreProjectorToolsVisible() const {
return ui_controller_->IsToolbarVisible();
}
bool ProjectorControllerImpl::IsEligible() const {
return is_speech_recognition_available_;
}
@ -89,7 +89,6 @@ void ProjectorControllerImpl::MarkKeyIdea() {
}
void ProjectorControllerImpl::OnRecordingStarted() {
projector_session_->Start();
StartSpeechRecognition();
ui_controller_->OnRecordingStateChanged(true /* started */);
metadata_controller_->OnRecordingStarted();
@ -98,7 +97,6 @@ void ProjectorControllerImpl::OnRecordingStarted() {
void ProjectorControllerImpl::OnRecordingEnded() {
if (!projector_session_->is_active())
return;
projector_session_->Stop();
StopSpeechRecognition();
ui_controller_->OnRecordingStateChanged(false /* started */);

@ -37,7 +37,6 @@ class ASH_EXPORT ProjectorControllerImpl : public ProjectorController {
void OnTranscription(const media::SpeechRecognitionResult& result) override;
void OnTranscriptionError() override;
void SetProjectorToolsVisible(bool is_visible) override;
bool AreProjectorToolsVisible() const override;
bool IsEligible() const override;
// Sets Caption bubble state to become opened/closed.

@ -10,9 +10,11 @@
#include <vector>
#include "ash/constants/ash_features.h"
#include "ash/projector/model/projector_session_impl.h"
#include "ash/projector/test/mock_projector_client.h"
#include "ash/projector/test/mock_projector_metadata_controller.h"
#include "ash/projector/test/mock_projector_ui_controller.h"
#include "ash/public/cpp/projector/projector_session.h"
#include "ash/test/ash_test_base.h"
#include "base/files/file_path.h"
#include "base/json/json_writer.h"
@ -234,12 +236,11 @@ TEST_F(ProjectorControllerTest, RecordingStarted) {
}
TEST_F(ProjectorControllerTest, RecordingEnded) {
controller_->projector_session()->Start(SourceType::kUnset);
controller_->OnRecordingStarted();
mock_client_.SetSelfieCamVisible(/*visible=*/true);
EXPECT_CALL(mock_client_, StopSpeechRecognition());
EXPECT_CALL(*mock_ui_controller_, OnRecordingStateChanged(/*started=*/false));
EXPECT_CALL(mock_client_, CloseSelfieCam());
controller_->SetProjectorToolsVisible(/*is_visible=*/false);
controller_->OnRecordingEnded();
}
} // namespace ash

@ -4,8 +4,10 @@
#include "ash/projector/projector_feature_pod_controller.h"
#include "ash/projector/model/projector_session_impl.h"
#include "ash/projector/projector_controller_impl.h"
#include "ash/projector/projector_ui_controller.h"
#include "ash/public/cpp/projector/projector_session.h"
#include "ash/resources/vector_icons/vector_icons.h"
#include "ash/session/session_controller_impl.h"
#include "ash/shell.h"
@ -56,10 +58,17 @@ void ProjectorFeaturePodController::OnIconPressed() {
tray_controller_->CloseBubble();
auto* projector_controller = Shell::Get()->projector_controller();
auto* projector_session = projector_controller->projector_session();
DCHECK(projector_controller);
DCHECK(projector_session);
bool is_visible = projector_controller->AreProjectorToolsVisible();
projector_controller->SetProjectorToolsVisible(!is_visible);
if (projector_session->is_active()) {
projector_session->Stop();
projector_controller->SetProjectorToolsVisible(false);
} else {
projector_session->Start(SourceType::kUnset);
projector_controller->SetProjectorToolsVisible(true);
}
}
SystemTrayItemUmaType ProjectorFeaturePodController::GetUmaType() const {

@ -53,7 +53,6 @@ class ASH_PUBLIC_EXPORT ProjectorController {
// Sets Projector toolbar visibility.
virtual void SetProjectorToolsVisible(bool is_visible) = 0;
virtual bool AreProjectorToolsVisible() const = 0;
// Returns true if Projector is eligible to start a new session.
// TODO(yilkal): Rename to something more descriptive, like CanStart().

@ -11,6 +11,7 @@
namespace ash {
// The recording source type.
// TODO(crbug/1199163): Record metrics for most common source type.
enum class SourceType { kUnset = 0, kFullscreen = 1, kTab = 2, kWindow = 3 };
// A checked observer which receives notification of changes to the
@ -32,6 +33,10 @@ class ASH_PUBLIC_EXPORT ProjectorSession {
static ProjectorSession* Get();
// Starts or stops the projector session active state.
virtual void Start(SourceType preset_source_type) = 0;
virtual void Stop() = 0;
// Adds/removes the specified |observer|.
virtual void AddObserver(ProjectorSessionObserver* observer) = 0;
virtual void RemoveObserver(ProjectorSessionObserver* observer) = 0;

@ -15,9 +15,13 @@ found in the LICENSE file.
<h1>Projector Player</h1>
<!--TODO(crbug/1213937): Use Polymer and cr-button instead-->
<!--TODO(crbug/1213937): Add aria-label-->
<!--TODO(crbug/1213937): Migrate UI to Google3 and make visibility
conditional on receiving init POST message from trusted domain
-->
<button>
<!--TODO(crbug/1213937): Use i18n and localized strings-->
Show Projector Tools
</button>
</body>
<!--TODO(crbug/1213937): Embed an chrome-untrusted:// iframe here-->
</html>

@ -4,10 +4,10 @@
import {sendWithPromise} from 'chrome://resources/js/cr.m.js';
// TODO(crbug/1213937): Launch Projector toolbar and integrate with screen
// TODO(crbug/1206720): Launch Projector toolbar and integrate with screen
// capture.
function onLaunchClick() {
sendWithPromise('launchScreenCapture').then(function(isVisible) {
sendWithPromise('launchProjectorRecording').then(function(isVisible) {
var button = document.body.querySelector('button');
// TODO(crbug/1213937): Use $i18n{}.
if (isVisible) {
@ -19,6 +19,8 @@ function onLaunchClick() {
}
function initialize() {
// TODO(crbug/1213937): Migrate to Google3 and conditionally set button
// visibility.
document.body.querySelector('button').onclick = onLaunchClick;
}

@ -44,7 +44,6 @@ class ASH_PUBLIC_EXPORT MockProjectorController : public ProjectorController {
void(const media::SpeechRecognitionResult& result));
MOCK_METHOD0(OnTranscriptionError, void());
MOCK_METHOD1(SetProjectorToolsVisible, void(bool is_visible));
MOCK_CONST_METHOD0(AreProjectorToolsVisible, bool());
MOCK_CONST_METHOD0(IsEligible, bool());
};

@ -5,6 +5,7 @@
#include "chrome/browser/ui/webui/chromeos/projector/projector_ui.h"
#include "ash/public/cpp/projector/projector_controller.h"
#include "ash/public/cpp/projector/projector_session.h"
#include "base/values.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/browser/ui/webui/webui_util.h"
@ -38,7 +39,7 @@ class ProjectorMessageHandler : public content::WebUIMessageHandler {
// content::WebUIMessageHandler:
void RegisterMessages() override {
web_ui()->RegisterMessageCallback(
"launchScreenCapture",
"launchProjectorRecording",
base::BindRepeating(
&ProjectorMessageHandler::HandleShowHideProjectorToolbar,
base::Unretained(this)));
@ -50,15 +51,25 @@ class ProjectorMessageHandler : public content::WebUIMessageHandler {
CHECK_EQ(1u, args->GetSize());
bool is_visible = true;
// This code only shows and hides the Projector toolbar.
// TODO(crbug/1206720): Integrate with screen capture.
auto* projector_controller = ash::ProjectorController::Get();
if (projector_controller && projector_controller->IsEligible()) {
is_visible = projector_controller->AreProjectorToolsVisible();
projector_controller->SetProjectorToolsVisible(!is_visible);
if (!projector_controller || !projector_controller->IsEligible()) {
ResolveJavascriptCallback(args->GetList()[0],
/*is_visible=*/base::Value(false));
return;
}
ResolveJavascriptCallback(args->GetList()[0], base::Value(!is_visible));
auto* projector_session = ash::ProjectorSession::Get();
DCHECK(projector_session);
bool should_show = false;
if (projector_session->is_active()) {
projector_session->Stop();
} else {
projector_session->Start(ash::SourceType::kUnset);
should_show = true;
}
projector_controller->SetProjectorToolsVisible(should_show);
ResolveJavascriptCallback(args->GetList()[0], base::Value(should_show));
}
};