Skip to content

Commit

Permalink
Cast UI: Remove the "cast file" option from the sources dropdown
Browse files Browse the repository at this point in the history 
The underlying MediaRouterUI/MediaRouterFileDialog code is deleted in a
subsequent CL.

Bug: 1284717, 1242962, 1274077
Change-Id: I04c7414795b28bf1e2658ec51e4198efa9c0f016
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3355496
Reviewed-by: Muyao Xu <muyaoxu@google.com>
Commit-Queue: Takumi Fujimoto <takumif@chromium.org>
Cr-Commit-Position: refs/heads/main@{#956805}
  • Loading branch information
@takumif
takumif authored and Chromium LUCI CQ committed Jan 8, 2022
1 parent deffdda commit 823301b
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 128 deletions.
48 changes: 1 addition & 47 deletions chrome/browser/ui/views/media_router/cast_dialog_view.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,6 @@ std::u16string CastDialogView::GetWindowTitle() const {
case SourceType::kDesktop:
return l10n_util::GetStringUTF16(
IDS_MEDIA_ROUTER_DESKTOP_MIRROR_CAST_MODE);
case SourceType::kLocalFile:
return l10n_util::GetStringFUTF16(IDS_MEDIA_ROUTER_CAST_LOCAL_MEDIA_TITLE,
local_file_name_.value());
default:
NOTREACHED();
return std::u16string();
Expand Down Expand Up @@ -174,18 +171,7 @@ bool CastDialogView::IsCommandIdEnabled(int command_id) const {
}

void CastDialogView::ExecuteCommand(int command_id, int event_flags) {
// This method is called when the user selects a source in the source picker.
if (command_id == SourceType::kLocalFile) {
// When the file picker dialog opens, the Cast dialog loses focus. So we
// must temporarily prevent it from closing when losing focus.
set_close_on_deactivate(false);
controller_->ChooseLocalFile(base::BindOnce(
&CastDialogView::OnFilePickerClosed, weak_factory_.GetWeakPtr()));
} else {
if (local_file_name_)
local_file_name_.reset();
SelectSource(static_cast<SourceType>(command_id));
}
SelectSource(static_cast<SourceType>(command_id));
}

void CastDialogView::AddObserver(Observer* observer) {
Expand Down Expand Up @@ -283,11 +269,7 @@ void CastDialogView::ShowAccessCodeCastDialog() {
case SourceType::kDesktop:
preferred_cast_mode = MediaCastMode::DESKTOP_MIRROR;
break;
case SourceType::kLocalFile:
preferred_cast_mode = MediaCastMode::LOCAL_FILE;
break;
}

AccessCodeCastDialog::Show(preferred_cast_mode);
}

Expand Down Expand Up @@ -381,8 +363,6 @@ void CastDialogView::ShowSourcesMenu() {
SourceType::kTab, IDS_MEDIA_ROUTER_TAB_MIRROR_CAST_MODE);
sources_menu_model_->AddCheckItemWithStringId(
SourceType::kDesktop, IDS_MEDIA_ROUTER_DESKTOP_MIRROR_CAST_MODE);
sources_menu_model_->AddCheckItemWithStringId(
SourceType::kLocalFile, IDS_MEDIA_ROUTER_LOCAL_FILE_CAST_MODE);
}

sources_menu_runner_ = std::make_unique<views::MenuRunner>(
Expand Down Expand Up @@ -417,16 +397,7 @@ void CastDialogView::SinkPressed(size_t index) {
} else {
absl::optional<MediaCastMode> cast_mode = GetCastModeToUse(sink);
if (cast_mode) {
// Starting local file casting may open a new tab synchronously on the UI
// thread, which deactivates the dialog. So we must prevent it from
// closing and getting destroyed.
if (cast_mode.value() == LOCAL_FILE)
set_close_on_deactivate(false);
controller_->StartCasting(sink.id, cast_mode.value());
// Re-enable close on deactivate so the user can click elsewhere to close
// the dialog.
if (cast_mode.value() == LOCAL_FILE)
set_close_on_deactivate(!keep_shown_for_testing_);
metrics_.OnStartCasting(base::Time::Now(), index, cast_mode.value(),
sink.icon_type, HasCastAndDialSinks());
}
Expand Down Expand Up @@ -454,10 +425,6 @@ absl::optional<MediaCastMode> CastDialogView::GetCastModeToUse(
if (base::Contains(sink.cast_modes, DESKTOP_MIRROR))
return absl::make_optional<MediaCastMode>(DESKTOP_MIRROR);
break;
case SourceType::kLocalFile:
if (base::Contains(sink.cast_modes, LOCAL_FILE))
return absl::make_optional<MediaCastMode>(LOCAL_FILE);
break;
}
return absl::nullopt;
}
Expand Down Expand Up @@ -487,19 +454,6 @@ void CastDialogView::RecordSinkCount() {
metrics_.OnRecordSinkCount(sink_buttons_);
}

void CastDialogView::OnFilePickerClosed(const ui::SelectedFileInfo* file_info) {
// Re-enable the setting to close the dialog when it loses focus.
set_close_on_deactivate(!keep_shown_for_testing_);
if (file_info) {
#if defined(OS_WIN)
local_file_name_ = base::WideToUTF16(file_info->display_name);
#else
local_file_name_ = base::UTF8ToUTF16(file_info->display_name);
#endif // defined(OS_WIN)
SelectSource(SourceType::kLocalFile);
}
}

bool CastDialogView::HasCastAndDialSinks() const {
bool has_cast = false;
bool has_dial = false;
Expand Down
11 changes: 1 addition & 10 deletions chrome/browser/ui/views/media_router/cast_dialog_view.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include "chrome/browser/ui/views/media_router/cast_dialog_metrics.h"
#include "ui/base/metadata/metadata_header_macros.h"
#include "ui/base/models/simple_menu_model.h"
#include "ui/shell_dialogs/selected_file_info.h"
#include "ui/views/bubble/bubble_border.h"
#include "ui/views/bubble/bubble_dialog_delegate_view.h"
#include "ui/views/controls/menu/menu_runner.h"
Expand Down Expand Up @@ -51,7 +50,7 @@ class CastDialogView : public views::BubbleDialogDelegateView,
virtual void OnDialogWillClose(CastDialogView* dialog_view) = 0;
};

enum SourceType { kTab, kDesktop, kLocalFile };
enum SourceType { kTab, kDesktop };

CastDialogView(const CastDialogView&) = delete;
CastDialogView& operator=(const CastDialogView&) = delete;
Expand Down Expand Up @@ -134,8 +133,6 @@ class CastDialogView : public views::BubbleDialogDelegateView,
private:
friend class CastDialogViewTest;
friend class MediaRouterCastUiForTest;
FRIEND_TEST_ALL_PREFIXES(CastDialogViewTest, CancelLocalFileSelection);
FRIEND_TEST_ALL_PREFIXES(CastDialogViewTest, CastLocalFile);
FRIEND_TEST_ALL_PREFIXES(CastDialogViewTest, DisableUnsupportedSinks);
FRIEND_TEST_ALL_PREFIXES(CastDialogViewTest, ShowAndHideDialog);
FRIEND_TEST_ALL_PREFIXES(CastDialogViewTest, ShowSourcesMenu);
Expand Down Expand Up @@ -199,9 +196,6 @@ class CastDialogView : public views::BubbleDialogDelegateView,
// Records the number of sinks shown with the metrics recorder.
void RecordSinkCount();

// Sets local file as the selected source if |file_info| is not null.
void OnFilePickerClosed(const ui::SelectedFileInfo* file_info);

// Returns true if there are active Cast and DIAL sinks.
bool HasCastAndDialSinks() const;

Expand Down Expand Up @@ -251,9 +245,6 @@ class CastDialogView : public views::BubbleDialogDelegateView,
// multiple sinks at the same time, the last activated sink is used.
absl::optional<size_t> selected_sink_index_;

// This value is set if the user has chosen a local file to cast.
absl::optional<std::u16string> local_file_name_;

base::ObserverList<Observer> observers_;

// When this is set to true, the dialog does not close on blur.
Expand Down
73 changes: 5 additions & 68 deletions chrome/browser/ui/views/media_router/cast_dialog_view_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,11 @@ TEST_F(CastDialogViewTest, ShowSourcesMenu) {
InitializeDialogWithModel(model);
// Press the button to show the sources menu.
views::test::ButtonTestApi(sources_button()).NotifyClick(CreateMouseEvent());
// The items should be "tab" (includes tab mirroring and presentation),
// "desktop", and "local file".
EXPECT_EQ(3, sources_menu_model()->GetItemCount());
// The items should be "tab" (includes tab mirroring and presentation) and
// "desktop".
EXPECT_EQ(2, sources_menu_model()->GetItemCount());
EXPECT_EQ(CastDialogView::kTab, sources_menu_model()->GetCommandIdAt(0));
EXPECT_EQ(CastDialogView::kDesktop, sources_menu_model()->GetCommandIdAt(1));
EXPECT_EQ(CastDialogView::kLocalFile,
sources_menu_model()->GetCommandIdAt(2));

// When there are no sinks, the sources button should be disabled.
model.set_media_sinks({});
Expand All @@ -249,8 +247,8 @@ TEST_F(CastDialogViewTest, CastAlternativeSources) {
InitializeDialogWithModel(model);
// Press the button to show the sources menu.
views::test::ButtonTestApi(sources_button()).NotifyClick(CreateMouseEvent());
// There should be three sources: tab, desktop, and local file.
ASSERT_EQ(3, sources_menu_model()->GetItemCount());
// There should be two sources: tab and desktop.
ASSERT_EQ(2, sources_menu_model()->GetItemCount());

EXPECT_CALL(controller_, StartCasting(model.media_sinks()[0].id, TAB_MIRROR));
sources_menu_model()->ActivatedAt(0);
Expand All @@ -263,67 +261,6 @@ TEST_F(CastDialogViewTest, CastAlternativeSources) {
SinkPressedAtIndex(0);
}

TEST_F(CastDialogViewTest, CastLocalFile) {
const std::string file_name = "example.mp4";
const std::string file_path = "path/to/" + file_name;
std::vector<UIMediaSink> media_sinks = {CreateAvailableSink()};
media_sinks[0].cast_modes = {TAB_MIRROR, LOCAL_FILE};
CastDialogModel model = CreateModelWithSinks(std::move(media_sinks));
InitializeDialogWithModel(model);
views::test::ButtonTestApi(sources_button()).NotifyClick(CreateMouseEvent());

#if defined(OS_WIN)
ui::SelectedFileInfo file_info{base::FilePath(base::UTF8ToWide(file_name)),
base::FilePath(base::UTF8ToWide(file_path))};
#else
ui::SelectedFileInfo file_info{base::FilePath(file_name),
base::FilePath(file_path)};
#endif // defined(OS_WIN)
EXPECT_CALL(controller_, ChooseLocalFile(_))
.WillOnce(
[file_info](base::OnceCallback<void(const ui::SelectedFileInfo*)>
file_callback) {
std::move(file_callback).Run(&file_info);
});
ASSERT_EQ(CastDialogView::kLocalFile,
sources_menu_model()->GetCommandIdAt(2));
sources_menu_model()->ActivatedAt(2);
EXPECT_EQ(dialog_->GetWindowTitle(),
l10n_util::GetStringFUTF16(IDS_MEDIA_ROUTER_CAST_LOCAL_MEDIA_TITLE,
base::UTF8ToUTF16(file_name)));

EXPECT_CALL(controller_, StartCasting(model.media_sinks()[0].id, LOCAL_FILE));
SinkPressedAtIndex(0);
}

TEST_F(CastDialogViewTest, CancelLocalFileSelection) {
std::vector<UIMediaSink> media_sinks = {CreateAvailableSink()};
media_sinks[0].cast_modes = {TAB_MIRROR, LOCAL_FILE};
CastDialogModel model = CreateModelWithSinks(std::move(media_sinks));
InitializeDialogWithModel(model);
views::test::ButtonTestApi(sources_button()).NotifyClick(CreateMouseEvent());

// The tab source should be selected by default.
ASSERT_EQ(CastDialogView::kTab, sources_menu_model()->GetCommandIdAt(0));
ASSERT_TRUE(sources_menu_model()->IsItemCheckedAt(0));

// Select the local file source, then cancel file selection by passing a
// nullptr into the callback.
EXPECT_CALL(controller_, ChooseLocalFile(_))
.WillOnce(
[](base::OnceCallback<void(const ui::SelectedFileInfo*)>
file_callback) { std::move(file_callback).Run(nullptr); });
ASSERT_EQ(CastDialogView::kLocalFile,
sources_menu_model()->GetCommandIdAt(2));
sources_menu_model()->ActivatedAt(2);

// Since we cancelled file selection, "tab" should still be the selected
// source.
EXPECT_TRUE(sources_menu_model()->IsItemCheckedAt(0));
EXPECT_CALL(controller_, StartCasting(model.media_sinks()[0].id, TAB_MIRROR));
SinkPressedAtIndex(0);
}

TEST_F(CastDialogViewTest, DisableUnsupportedSinks) {
std::vector<UIMediaSink> media_sinks = {CreateAvailableSink(),
CreateAvailableSink()};
Expand Down
3 changes: 0 additions & 3 deletions chrome/test/media_router/media_router_cast_ui_for_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ void MediaRouterCastUiForTest::ChooseSourceType(
case CastDialogView::kDesktop:
source_index = 1;
break;
case CastDialogView::kLocalFile:
source_index = 2;
break;
}
dialog_view->sources_menu_model_for_test()->ActivatedAt(source_index);
}
Expand Down

0 comments on commit 823301b

Please sign in to comment.