Skip to content

Commit

Permalink
Revert "[Eche] Create mojo interface and bindings for keyboard layout…
Browse files Browse the repository at this point in the history
… changes"

This reverts commit 8fe1a9e.

Reason for revert: Many tests are now failing during destruction of the EcheKeyboardLayoutHandler and this appears to be the culprit CL.

Original change's description:
> [Eche] Create mojo interface and bindings for keyboard layout changes
>
> This introduces a mojo interface that allows the EcheSWA to request the
> current keyboard layout configuration and creates an observer interface
> to route the information back. Additionally, EcheKeyboardLayoutHandler
> also observes ImeControllerImpl and sends the same information to the
> SWA whenever the keyboard layout changes.
>
> Test: Manually verified app streaming is WAI, unit tests added.
> Fixed: b/275816267
> Change-Id: I1522789ead32b2c8e009b88d668466c5fdb59fac
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4786991
> Reviewed-by: Jon Mann <jonmann@chromium.org>
> Reviewed-by: Pu Shi <pushi@google.com>
> Reviewed-by: Will Harris <wfh@chromium.org>
> Commit-Queue: Crisrael Lucero <crisrael@google.com>
> Reviewed-by: Abbas Nayebi <nayebi@google.com>
> Cr-Commit-Position: refs/heads/main@{#1187254}

Bug: 1475332
Change-Id: I8df645fdcb435804f52e54a6508a5a1044cad238
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4806963
Owners-Override: Darryl James <dljames@chromium.org>
Reviewed-by: Darryl James <dljames@chromium.org>
Commit-Queue: Joshua Hood <jdh@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1187378}
  • Loading branch information
hoodjoshua authored and Chromium LUCI CQ committed Aug 23, 2023
1 parent 5365fd5 commit ebf0f80
Show file tree
Hide file tree
Showing 14 changed files with 6 additions and 415 deletions.
3 changes: 0 additions & 3 deletions ash/webui/eche_app_ui/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ static_library("eche_app_ui") {
"eche_connector_impl.h",
"eche_feature_status_provider.cc",
"eche_feature_status_provider.h",
"eche_keyboard_layout_handler.cc",
"eche_keyboard_layout_handler.h",
"eche_message_receiver.cc",
"eche_message_receiver.h",
"eche_message_receiver_impl.cc",
Expand Down Expand Up @@ -196,7 +194,6 @@ source_set("unit_tests") {
"eche_connection_status_handler_unittest.cc",
"eche_connector_impl_unittest.cc",
"eche_feature_status_provider_unittest.cc",
"eche_keyboard_layout_handler_unittest.cc",
"eche_message_receiver_impl_unittest.cc",
"eche_notification_click_handler_unittest.cc",
"eche_presence_manager_unittest.cc",
Expand Down
11 changes: 1 addition & 10 deletions ash/webui/eche_app_ui/eche_app_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "ash/webui/eche_app_ui/eche_connection_scheduler_impl.h"
#include "ash/webui/eche_app_ui/eche_connection_status_handler.h"
#include "ash/webui/eche_app_ui/eche_connector_impl.h"
#include "ash/webui/eche_app_ui/eche_keyboard_layout_handler.h"
#include "ash/webui/eche_app_ui/eche_message_receiver_impl.h"
#include "ash/webui/eche_app_ui/eche_presence_manager.h"
#include "ash/webui/eche_app_ui/eche_signaler.h"
Expand Down Expand Up @@ -129,9 +128,7 @@ EcheAppManager::EcheAppManager(
stream_status_change_handler_.get(),
feature_status_provider_.get())),
eche_stream_orientation_observer_(
std::make_unique<EcheStreamOrientationObserver>()),
eche_keyboard_layout_handler_(
std::make_unique<EcheKeyboardLayoutHandler>()) {
std::make_unique<EcheStreamOrientationObserver>()) {
ash::GetNetworkConfigService(
remote_cros_network_config_.BindNewPipeAndPassReceiver());
system_info_provider_ = std::make_unique<SystemInfoProvider>(
Expand Down Expand Up @@ -191,11 +188,6 @@ void EcheAppManager::BindConnectionStatusObserverInterface(
eche_connection_status_handler_->Bind(std::move(receiver));
}

void EcheAppManager::BindKeyboardLayoutHandlerInterface(
mojo::PendingReceiver<mojom::KeyboardLayoutHandler> receiver) {
eche_keyboard_layout_handler_->Bind(std::move(receiver));
}

AppsAccessManager* EcheAppManager::GetAppsAccessManager() {
return apps_access_manager_.get();
}
Expand All @@ -220,7 +212,6 @@ void EcheAppManager::Shutdown() {
phone_hub_manager_->SetSystemInfoProvider(nullptr);
}

eche_keyboard_layout_handler_.reset();
eche_stream_orientation_observer_.reset();
system_info_provider_.reset();
eche_tray_stream_status_observer_.reset();
Expand Down
5 changes: 0 additions & 5 deletions ash/webui/eche_app_ui/eche_app_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@ class EcheTrayStreamStatusObserver;
class EcheConnectionScheduler;
class EcheStreamOrientationObserver;
class EcheConnectionStatusHandler;
class EcheKeyboardLayoutHandler;

// Implements the core logic of the EcheApp and exposes interfaces via its
// public API. Implemented as a KeyedService since it depends on other
Expand Down Expand Up @@ -107,9 +106,6 @@ class EcheAppManager : public KeyedService {
void BindConnectionStatusObserverInterface(
mojo::PendingReceiver<mojom::ConnectionStatusObserver> receiver);

void BindKeyboardLayoutHandlerInterface(
mojo::PendingReceiver<mojom::KeyboardLayoutHandler> receiver);

AppsAccessManager* GetAppsAccessManager();

EcheConnectionStatusHandler* GetEcheConnectionStatusHandler();
Expand Down Expand Up @@ -150,7 +146,6 @@ class EcheAppManager : public KeyedService {
eche_tray_stream_status_observer_;
std::unique_ptr<EcheStreamOrientationObserver>
eche_stream_orientation_observer_;
std::unique_ptr<EcheKeyboardLayoutHandler> eche_keyboard_layout_handler_;
};

} // namespace eche_app
Expand Down
37 changes: 5 additions & 32 deletions ash/webui/eche_app_ui/eche_app_manager_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@

#include "ash/webui/eche_app_ui/eche_app_manager.h"

#include "ash/test/ash_test_base.h"
#include "ash/test/test_ash_web_view_factory.h"
#include <vector>

#include "ash/webui/eche_app_ui/eche_connection_status_handler.h"
#include "ash/webui/eche_app_ui/eche_keyboard_layout_handler.h"
#include "ash/webui/eche_app_ui/eche_stream_orientation_observer.h"
#include "ash/webui/eche_app_ui/eche_stream_status_change_handler.h"
#include "ash/webui/eche_app_ui/launch_app_helper.h"
Expand All @@ -20,7 +19,6 @@
#include "chromeos/ash/components/multidevice/remote_device_test_util.h"
#include "chromeos/ash/components/phonehub/fake_phone_hub_manager.h"
#include "chromeos/ash/components/phonehub/phone_hub_manager.h"
#include "chromeos/ash/components/test/ash_test_suite.h"
#include "chromeos/ash/services/device_sync/public/cpp/fake_device_sync_client.h"
#include "chromeos/ash/services/multidevice_setup/public/cpp/fake_multidevice_setup_client.h"
#include "chromeos/ash/services/secure_channel/public/cpp/client/fake_secure_channel_client.h"
Expand All @@ -31,7 +29,6 @@
#include "device/bluetooth/dbus/fake_bluetooth_debug_manager_client.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "ui/base/resource/resource_bundle.h"
#include "ui/gfx/image/image.h"

namespace ash::eche_app {
Expand Down Expand Up @@ -79,7 +76,7 @@ const char kFakeGaiaId[] = "123";
const size_t kNumTestDevices = 3;
const char kFakeDeviceType[] = "Chromebook";

class EcheAppManagerTest : public AshTestBase {
class EcheAppManagerTest : public testing::Test {
public:
EcheAppManagerTest(const EcheAppManagerTest&) = delete;
EcheAppManagerTest& operator=(const EcheAppManagerTest&) = delete;
Expand All @@ -90,7 +87,7 @@ class EcheAppManagerTest : public AshTestBase {
multidevice::CreateRemoteDeviceRefListForTest(kNumTestDevices)) {}
~EcheAppManagerTest() override = default;

// AshTestBase:
// testing::Test:
void SetUp() override {
std::unique_ptr<bluez::BluezDBusManagerSetter> dbus_setter =
bluez::BluezDBusManager::GetSetterForTesting();
Expand All @@ -104,11 +101,6 @@ class EcheAppManagerTest : public AshTestBase {
std::unique_ptr<bluez::BluetoothDebugManagerClient>(
std::move(fake_bluetooth_debug_manager_client)));

DCHECK(test_web_view_factory_.get());
ui::ResourceBundle::CleanupSharedInstance();
AshTestSuite::LoadTestResources();
AshTestBase::SetUp();

fake_phone_hub_manager_ = std::make_unique<phonehub::FakePhoneHubManager>();
fake_device_sync_client_ =
std::make_unique<device_sync::FakeDeviceSyncClient>();
Expand Down Expand Up @@ -138,15 +130,6 @@ class EcheAppManagerTest : public AshTestBase {
base::BindRepeating(&CloseNotificationFunction));
}

void TearDown() override {
manager_.reset();
fake_secure_channel_client_.reset();
fake_multidevice_setup_client_.reset();
fake_device_sync_client_.reset();
fake_phone_hub_manager_.reset();
AshTestBase::TearDown();
}

mojo::Remote<mojom::SignalingMessageExchanger>&
signaling_message_exchanger_remote() {
return signaling_message_exchanger_remote_;
Expand Down Expand Up @@ -178,10 +161,6 @@ class EcheAppManagerTest : public AshTestBase {
return connection_status_observer_remote_;
}

mojo::Remote<mojom::KeyboardLayoutHandler>& keyboard_layout_handler_remote() {
return keyboard_layout_handler_remote_;
}

void Bind() {
manager_->BindSignalingMessageExchangerInterface(
signaling_message_exchanger_remote_.BindNewPipeAndPassReceiver());
Expand All @@ -197,11 +176,10 @@ class EcheAppManagerTest : public AshTestBase {
stream_orientation_observer_remote_.BindNewPipeAndPassReceiver());
manager_->BindConnectionStatusObserverInterface(
connection_status_observer_remote_.BindNewPipeAndPassReceiver());
manager_->BindKeyboardLayoutHandlerInterface(
keyboard_layout_handler_remote_.BindNewPipeAndPassReceiver());
}

private:
base::test::TaskEnvironment task_environment_;
const multidevice::RemoteDeviceRefList test_devices_;

TestingPrefServiceSimple test_pref_service_;
Expand All @@ -212,8 +190,6 @@ class EcheAppManagerTest : public AshTestBase {
std::unique_ptr<secure_channel::FakeSecureChannelClient>
fake_secure_channel_client_;
std::unique_ptr<EcheAppManager> manager_;
std::unique_ptr<TestAshWebViewFactory> test_web_view_factory_ =
std::make_unique<TestAshWebViewFactory>();

mojo::Remote<mojom::SignalingMessageExchanger>
signaling_message_exchanger_remote_;
Expand All @@ -225,7 +201,6 @@ class EcheAppManagerTest : public AshTestBase {
stream_orientation_observer_remote_;
mojo::Remote<mojom::ConnectionStatusObserver>
connection_status_observer_remote_;
mojo::Remote<mojom::KeyboardLayoutHandler> keyboard_layout_handler_remote_;
};

TEST_F(EcheAppManagerTest, BindCheck) {
Expand All @@ -236,7 +211,6 @@ TEST_F(EcheAppManagerTest, BindCheck) {
EXPECT_FALSE(display_stream_handler_remote());
EXPECT_FALSE(stream_orientation_observer_remote());
EXPECT_FALSE(connection_status_observer_remote());
EXPECT_FALSE(keyboard_layout_handler_remote());

Bind();

Expand All @@ -247,7 +221,6 @@ TEST_F(EcheAppManagerTest, BindCheck) {
EXPECT_TRUE(display_stream_handler_remote());
EXPECT_TRUE(stream_orientation_observer_remote());
EXPECT_TRUE(connection_status_observer_remote());
EXPECT_TRUE(keyboard_layout_handler_remote());
}

} // namespace ash::eche_app
7 changes: 0 additions & 7 deletions ash/webui/eche_app_ui/eche_app_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,6 @@ void EcheAppUI::BindInterface(
}
}

void EcheAppUI::BindInterface(
mojo::PendingReceiver<mojom::KeyboardLayoutHandler> receiver) {
if (manager_) {
manager_->BindKeyboardLayoutHandlerInterface(std::move(receiver));
}
}

WEB_UI_CONTROLLER_TYPE_IMPL(EcheAppUI)

} // namespace ash::eche_app
3 changes: 0 additions & 3 deletions ash/webui/eche_app_ui/eche_app_ui.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ class EcheAppUI : public ui::MojoWebUIController {
void BindInterface(
mojo::PendingReceiver<mojom::ConnectionStatusObserver> receiver);

void BindInterface(
mojo::PendingReceiver<mojom::KeyboardLayoutHandler> receiver);

private:
raw_ptr<EcheAppManager> manager_;

Expand Down
62 changes: 0 additions & 62 deletions ash/webui/eche_app_ui/eche_keyboard_layout_handler.cc

This file was deleted.

51 changes: 0 additions & 51 deletions ash/webui/eche_app_ui/eche_keyboard_layout_handler.h

This file was deleted.

0 comments on commit ebf0f80

Please sign in to comment.