Skip to content

Commit

Permalink
hps: Display inference results on internal page
Browse files Browse the repository at this point in the history
This patch adds real-time display of model inference results from the
presence sensor. It can be used to evaluate inference performance under
different conditions.

Depends on https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3614639

(cherry picked from commit 331074b)

Bug: b:193186731
Change-Id: I3ea9358e83889b5a912f290b8820972033e924d3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3615595
Commit-Queue: Sami Kyöstilä <skyostil@chromium.org>
Reviewed-by: Alex Newcomer <newcomer@chromium.org>
Reviewed-by: Guoxing Zhao <charleszhao@chromium.org>
Reviewed-by: Achuith Bhandarkar <achuith@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1003617}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3655188
Auto-Submit: Sami Kyöstilä <skyostil@chromium.org>
Owners-Override: Prudhvikumar Bommana <pbommana@google.com>
Commit-Queue: Prudhvikumar Bommana <pbommana@google.com>
Cr-Commit-Position: refs/branch-heads/5060@{#256}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
skyostil authored and Chromium LUCI CQ committed May 26, 2022
1 parent ef3a0ce commit 03ebc06
Show file tree
Hide file tree
Showing 15 changed files with 185 additions and 81 deletions.
2 changes: 1 addition & 1 deletion DEPS
Original file line number Diff line number Diff line change
Expand Up @@ -1128,7 +1128,7 @@ deps = {

# For Linux and Chromium OS.
'src/third_party/cros_system_api': {
'url': Var('chromium_git') + '/chromiumos/platform2/system_api.git' + '@' + '5ea193b9a3e6c3e9898f46527d49eb6c148551c6',
'url': Var('chromium_git') + '/chromiumos/platform2/system_api.git' + '@' + 'd79486f913c4d301047e2f6e8051538c94066ec9',
'condition': 'checkout_linux',
},

Expand Down
4 changes: 2 additions & 2 deletions ash/system/human_presence/lock_on_leave_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ void LockOnLeaveController::OnOrientationChanged(
ReconfigViaDbus();
}

void LockOnLeaveController::OnHpsSenseChanged(hps::HpsResult state) {}
void LockOnLeaveController::OnHpsSenseChanged(const hps::HpsResultProto&) {}

void LockOnLeaveController::OnHpsNotifyChanged(hps::HpsResult state) {}
void LockOnLeaveController::OnHpsNotifyChanged(const hps::HpsResultProto&) {}

void LockOnLeaveController::OnRestart() {
service_available_ = true;
Expand Down
4 changes: 2 additions & 2 deletions ash/system/human_presence/lock_on_leave_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ class ASH_EXPORT LockOnLeaveController
void OnOrientationChanged(bool suitable_for_human_presence) override;

// chromeos::HumanPresenceDBusClient::Observer:
void OnHpsSenseChanged(hps::HpsResult state) override;
void OnHpsNotifyChanged(hps::HpsResult state) override;
void OnHpsSenseChanged(const hps::HpsResultProto& result) override;
void OnHpsNotifyChanged(const hps::HpsResultProto& result) override;
// Re-enables LockOnLeave on human presence service restart if it was enabled
// before.
void OnRestart() override;
Expand Down
11 changes: 6 additions & 5 deletions ash/system/human_presence/snooping_protection_controller.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,12 @@ void SnoopingProtectionController::OnOrientationChanged(
UpdateSnooperStatus(new_state);
}

void SnoopingProtectionController::OnHpsSenseChanged(hps::HpsResult) {}
void SnoopingProtectionController::OnHpsSenseChanged(
const hps::HpsResultProto&) {}

void SnoopingProtectionController::OnHpsNotifyChanged(
hps::HpsResult detection_state) {
const bool present = detection_state == hps::HpsResult::POSITIVE;
const hps::HpsResultProto& result) {
const bool present = result.value() == hps::HpsResult::POSITIVE;

State new_state = state_;
new_state.present = present;
Expand Down Expand Up @@ -280,12 +281,12 @@ void SnoopingProtectionController::StartServiceObservation(
}

void SnoopingProtectionController::UpdateServiceState(
absl::optional<hps::HpsResult> response) {
absl::optional<hps::HpsResultProto> response) {
LOG_IF(WARNING, !response.has_value())
<< "Polling the presence daemon failed";

const bool present =
response.value_or(hps::HpsResult::NEGATIVE) == hps::HpsResult::POSITIVE;
response.has_value() && response->value() == hps::HpsResult::POSITIVE;

State new_state = state_;
new_state.present = present;
Expand Down
6 changes: 3 additions & 3 deletions ash/system/human_presence/snooping_protection_controller.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ class ASH_EXPORT SnoopingProtectionController
void OnOrientationChanged(bool suitable_for_human_presence) override;

// chromeos::HumanPresenceDBusClient::Observer:
void OnHpsSenseChanged(hps::HpsResult state) override;
void OnHpsNotifyChanged(hps::HpsResult state) override;
void OnHpsSenseChanged(const hps::HpsResultProto& result) override;
void OnHpsNotifyChanged(const hps::HpsResultProto& result) override;
void OnRestart() override;
void OnShutdown() override;

Expand Down Expand Up @@ -115,7 +115,7 @@ class ASH_EXPORT SnoopingProtectionController
void StartServiceObservation(bool service_is_available);

// Performs the state update from the daemon response.
void UpdateServiceState(absl::optional<hps::HpsResult> result);
void UpdateServiceState(absl::optional<hps::HpsResultProto> result);

// A callback to update visibility when the user enables or disables the
// feature.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,10 @@ class SnoopingProtectionControllerTestBase : public NoSessionAshTestBase {
chromeos::HumanPresenceDBusClient::InitializeFake();
dbus_client_ = chromeos::FakeHumanPresenceDBusClient::Get();
dbus_client_->set_hps_service_is_available(service_available_);
dbus_client_->set_hps_notify_result(
service_state_ ? hps::HpsResult::POSITIVE : hps::HpsResult::NEGATIVE);
hps::HpsResultProto state;
state.set_value(service_state_ ? hps::HpsResult::POSITIVE
: hps::HpsResult::NEGATIVE);
dbus_client_->set_hps_notify_result(state);

AshTestBase::SetUp();

Expand Down Expand Up @@ -144,12 +146,15 @@ TEST_F(SnoopingProtectionControllerTestAbsent, PresenceChange) {

EXPECT_FALSE(controller_->SnooperPresent());

controller_->OnHpsNotifyChanged(hps::HpsResult::POSITIVE);
hps::HpsResultProto state;
state.set_value(hps::HpsResult::POSITIVE);
controller_->OnHpsNotifyChanged(state);
task_environment()->FastForwardBy(kLongTime);

EXPECT_TRUE(controller_->SnooperPresent());

controller_->OnHpsNotifyChanged(hps::HpsResult::NEGATIVE);
state.set_value(hps::HpsResult::NEGATIVE);
controller_->OnHpsNotifyChanged(state);
task_environment()->FastForwardBy(kLongTime);

EXPECT_FALSE(controller_->SnooperPresent());
Expand Down Expand Up @@ -395,7 +400,9 @@ TEST_F(SnoopingProtectionControllerTestPresent, PositiveWindow) {
EXPECT_EQ(dbus_client_->hps_notify_count(), 1);

EXPECT_TRUE(controller_->SnooperPresent());
controller_->OnHpsNotifyChanged(hps::HpsResult::NEGATIVE);
hps::HpsResultProto state;
state.set_value(hps::HpsResult::NEGATIVE);
controller_->OnHpsNotifyChanged(state);

// The snooping status shouldn't immediately change, since we have a minimum
// length that it should remain positive.
Expand All @@ -407,7 +414,8 @@ TEST_F(SnoopingProtectionControllerTestPresent, PositiveWindow) {
EXPECT_FALSE(controller_->SnooperPresent());

// Snooping status should always immediately become true and stay true.
controller_->OnHpsNotifyChanged(hps::HpsResult::POSITIVE);
state.set_value(hps::HpsResult::POSITIVE);
controller_->OnHpsNotifyChanged(state);
EXPECT_TRUE(controller_->SnooperPresent());
task_environment()->FastForwardBy(kLongTime);
EXPECT_TRUE(controller_->SnooperPresent());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,9 @@ class SnoopingProtectionNotificationBlockerTest : public AshTestBase {
chromeos::HumanPresenceDBusClient::InitializeFake();
auto* dbus_client = chromeos::FakeHumanPresenceDBusClient::Get();
dbus_client->set_hps_service_is_available(true);
dbus_client->set_hps_notify_result(hps::HpsResult::NEGATIVE);
hps::HpsResultProto state;
state.set_value(hps::HpsResult::NEGATIVE);
dbus_client->set_hps_notify_result(state);

AshTestBase::SetUp();

Expand Down Expand Up @@ -256,7 +258,9 @@ TEST_F(SnoopingProtectionNotificationBlockerTest, Snooping) {
EXPECT_EQ(VisibleNotificationCount(), 1u);

// Simulate snooper presence.
controller_->OnHpsNotifyChanged(/*state=*/hps::HpsResult::POSITIVE);
hps::HpsResultProto state;
state.set_value(hps::HpsResult::POSITIVE);
controller_->OnHpsNotifyChanged(state);

// When snooping is detected, the popup notification should be hidden but
// remain in the notification queue. Note that, since the popup has been
Expand All @@ -272,7 +276,8 @@ TEST_F(SnoopingProtectionNotificationBlockerTest, Snooping) {

// Simulate snooper absence. We wait for a moment to bypass the controller's
// hysteresis logic.
controller_->OnHpsNotifyChanged(/*state=*/hps::HpsResult::NEGATIVE);
state.set_value(hps::HpsResult::NEGATIVE);
controller_->OnHpsNotifyChanged(state);
task_environment()->FastForwardBy(base::Seconds(10));

// The unshown popups should appear since snooper has left.
Expand All @@ -291,7 +296,9 @@ TEST_F(SnoopingProtectionNotificationBlockerTest, Pref) {
EXPECT_EQ(VisibleNotificationCount(), 1u);

// Simulate snooper presence.
controller_->OnHpsNotifyChanged(/*snooper=*/hps::HpsResult::POSITIVE);
hps::HpsResultProto state;
state.set_value(hps::HpsResult::POSITIVE);
controller_->OnHpsNotifyChanged(state);

// Notifications should be visible up until the user enables the feature.
EXPECT_EQ(VisiblePopupCount(), 1u);
Expand Down Expand Up @@ -339,7 +346,9 @@ TEST_F(SnoopingProtectionNotificationBlockerTest, SystemNotification) {
EXPECT_EQ(VisibleNotificationCount(), 3u);

// Simulate snooper presence.
controller_->OnHpsNotifyChanged(/*snooper=*/hps::HpsResult::POSITIVE);
hps::HpsResultProto state;
state.set_value(hps::HpsResult::POSITIVE);
controller_->OnHpsNotifyChanged(state);

// The safe notification shouldn't be suppressed, but the sensitive
// notification should be.
Expand All @@ -357,7 +366,9 @@ TEST_F(SnoopingProtectionNotificationBlockerTest, InfoPopup) {
SetBlockerPref(true);

// Simulate snooper presence.
controller_->OnHpsNotifyChanged(/*snooper=*/hps::HpsResult::POSITIVE);
hps::HpsResultProto state;
state.set_value(hps::HpsResult::POSITIVE);
controller_->OnHpsNotifyChanged(state);

// Two notifications we're blocking.
AddNotification("notification-1", u"notifier-1");
Expand Down Expand Up @@ -386,7 +397,9 @@ TEST_F(SnoopingProtectionNotificationBlockerTest, InfoPopupOtherBlocker) {
SetBlockerPref(true);

// Simulate snooper presence.
controller_->OnHpsNotifyChanged(/*snooper=*/hps::HpsResult::POSITIVE);
hps::HpsResultProto state;
state.set_value(hps::HpsResult::POSITIVE);
controller_->OnHpsNotifyChanged(state);

// One notification only we are blocking, and one notification that is also
// blocked by another blocker.
Expand Down Expand Up @@ -416,7 +429,9 @@ TEST_F(SnoopingProtectionNotificationBlockerTest,
SetBlockerPref(true);

// Simulate snooper presence.
controller_->OnHpsNotifyChanged(/*snooper=*/hps::HpsResult::POSITIVE);
hps::HpsResultProto state;
state.set_value(hps::HpsResult::POSITIVE);
controller_->OnHpsNotifyChanged(state);

// Newer notifiers should come before older ones.
AddNotification("notification-1", u"notifier-1");
Expand Down Expand Up @@ -462,7 +477,9 @@ TEST_F(SnoopingProtectionNotificationBlockerTest, ShowButtonClicked) {
SetBlockerPref(true);

// Simulate snooper presence.
controller_->OnHpsNotifyChanged(/*snooper=*/hps::HpsResult::POSITIVE);
hps::HpsResultProto state;
state.set_value(hps::HpsResult::POSITIVE);
controller_->OnHpsNotifyChanged(state);

AddNotification("notification-1", u"notifier-1");
AddNotification("notification-2", u"notifier-2");
Expand All @@ -479,7 +496,9 @@ TEST_F(SnoopingProtectionNotificationBlockerTest, SettingsButtonClicked) {
SetBlockerPref(true);

// Simulate snooper presence.
controller_->OnHpsNotifyChanged(/*snooper=*/hps::HpsResult::POSITIVE);
hps::HpsResultProto state;
state.set_value(hps::HpsResult::POSITIVE);
controller_->OnHpsNotifyChanged(state);

AddNotification("notification-1", u"notifier-1");
AddNotification("notification-2", u"notifier-2");
Expand Down
26 changes: 16 additions & 10 deletions chrome/browser/ui/webui/chromeos/human_presence_internals_ui.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@ class HumanPresenceInternalsUIMessageHandler
void OnJavascriptDisallowed() override;

// chromeos::HumanPresenceDBusClient::Observer implementation.
void OnHpsSenseChanged(hps::HpsResult state) override;
void OnHpsNotifyChanged(hps::HpsResult state) override;
void OnHpsSenseChanged(const hps::HpsResultProto&) override;
void OnHpsNotifyChanged(const hps::HpsResultProto&) override;
void OnRestart() override;
void OnShutdown() override;

Expand All @@ -60,8 +60,8 @@ class HumanPresenceInternalsUIMessageHandler
void QuerySnoopingProtection(const base::Value::List& args);

void OnConnected(bool connected);
void OnLockOnLeaveResult(absl::optional<hps::HpsResult>);
void OnSnoopingProtectionResult(absl::optional<hps::HpsResult>);
void OnLockOnLeaveResult(absl::optional<hps::HpsResultProto>);
void OnSnoopingProtectionResult(absl::optional<hps::HpsResultProto>);

base::ScopedObservation<chromeos::HumanPresenceDBusClient,
chromeos::HumanPresenceDBusClient::Observer>
Expand All @@ -79,31 +79,35 @@ HumanPresenceInternalsUIMessageHandler::
~HumanPresenceInternalsUIMessageHandler() = default;

void HumanPresenceInternalsUIMessageHandler::OnHpsSenseChanged(
hps::HpsResult state) {
const hps::HpsResultProto& state) {
OnLockOnLeaveResult(state);
}

void HumanPresenceInternalsUIMessageHandler::OnHpsNotifyChanged(
hps::HpsResult state) {
const hps::HpsResultProto& state) {
OnSnoopingProtectionResult(state);
}

void HumanPresenceInternalsUIMessageHandler::OnLockOnLeaveResult(
absl::optional<hps::HpsResult> state) {
absl::optional<hps::HpsResultProto> state) {
base::DictionaryValue value;
if (state.has_value()) {
value.SetInteger("state", *state);
value.SetInteger("state", state->value());
value.SetInteger("inference_result", state->inference_result());
value.SetInteger("inference_result_valid", state->inference_result_valid());
} else {
value.SetBoolean("disabled", true);
}
FireWebUIListener(hps::kHumanPresenceInternalsLockOnLeaveChangedEvent, value);
}

void HumanPresenceInternalsUIMessageHandler::OnSnoopingProtectionResult(
absl::optional<hps::HpsResult> state) {
absl::optional<hps::HpsResultProto> state) {
base::DictionaryValue value;
if (state.has_value()) {
value.SetInteger("state", *state);
value.SetInteger("state", state->value());
value.SetInteger("inference_result", state->inference_result());
value.SetInteger("inference_result_valid", state->inference_result_valid());
} else {
value.SetBoolean("disabled", true);
}
Expand Down Expand Up @@ -145,6 +149,7 @@ void HumanPresenceInternalsUIMessageHandler::EnableLockOnLeave(
return;
}
hps::FeatureConfig config(*hps::GetEnableLockOnLeaveConfig());
config.set_report_raw_results(true);
chromeos::HumanPresenceDBusClient::Get()->EnableHpsSense(config);
}

Expand All @@ -171,6 +176,7 @@ void HumanPresenceInternalsUIMessageHandler::EnableSnoopingProtection(
return;
}
hps::FeatureConfig config(*hps::GetEnableSnoopingProtectionConfig());
config.set_report_raw_results(true);
chromeos::HumanPresenceDBusClient::Get()->EnableHpsNotify(config);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,28 +47,30 @@ h2 {
height: 64px;
outline: solid 2px var(--cros-color-primary-inverted);
border-radius: 4px;
margin-right: 32px;
margin-bottom: 32px;
}

.feature-panel {
background-color: var(--cros-bg-color-elevation-5);
border-top: solid 4px var(--cros-color-prominent-inverted);
margin-right: 16px;
padding: 16px;
min-width: 544px;
}

.spacer {
flex-grow: 1;
}

.history {
min-width: 512px;
width: 512px;
min-height: 32px;
display: flex;
flex-direction: row;
justify-content: end;
outline: solid 2px var(--cros-color-primary-inverted);
border-radius: 4px;
margin-top: 32px;
}

.history span {
Expand Down Expand Up @@ -107,4 +109,25 @@ h2 {
);
background-attachment: fixed;
color: var(--cros-color-disabled) !important;
}
}

.inference-history {
height: 128px;
background: none;
align-items: end;
}

.inference-history span {
background: var(--cros-color-prominent-inverted);
}

.inference-history span.invalid {
background: var(--cros-color-primary-inverted);
}

#sense-inference-result, #notify-inference-result {
align-self: end;
margin-left: .5em;
margin-bottom: .5em;
font-variant-numeric: tabular-nums;
}

0 comments on commit 03ebc06

Please sign in to comment.