Skip to content

Commit

Permalink
Phone Hub: Allow ping manager timeout to be configured by server.
Browse files Browse the repository at this point in the history
Bug: b/229432201
Change-Id: I0853fd1e8100553b88cf0b606cbd17fe3642a825
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4310291
Reviewed-by: Pu Shi <pushi@google.com>
Reviewed-by: Jon Mann <jonmann@chromium.org>
Commit-Queue: Crisrael Lucero <crisrael@google.com>
Cr-Commit-Position: refs/heads/main@{#1118951}
  • Loading branch information
crisrael authored and Chromium LUCI CQ committed Mar 17, 2023
1 parent 6323970 commit c8ab130
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 6 deletions.
4 changes: 4 additions & 0 deletions ash/constants/ash_features.cc
Expand Up @@ -1611,6 +1611,10 @@ BASE_FEATURE(kPhoneHubPingOnBubbleOpen,
"PhoneHubPingOnBubbleOpen",
base::FEATURE_ENABLED_BY_DEFAULT);

// Maximum number of seconds to wait for ping response before disconnecting
const base::FeatureParam<base::TimeDelta> kPhoneHubPingTimeout{
&kPhoneHubPingOnBubbleOpen, "PhoneHubPingTimeout", base::Seconds(60)};

// Controls whether policy provided trust anchors are allowed at the lock
// screen.
BASE_FEATURE(kPolicyProvidedTrustAnchorsAllowedAtLockScreen,
Expand Down
2 changes: 2 additions & 0 deletions ash/constants/ash_features.h
Expand Up @@ -466,6 +466,8 @@ COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kPhoneHubNudge);
COMPONENT_EXPORT(ASH_CONSTANTS)
BASE_DECLARE_FEATURE(kPhoneHubPingOnBubbleOpen);
COMPONENT_EXPORT(ASH_CONSTANTS)
extern const base::FeatureParam<base::TimeDelta> kPhoneHubPingTimeout;
COMPONENT_EXPORT(ASH_CONSTANTS)
BASE_DECLARE_FEATURE(kPolicyProvidedTrustAnchorsAllowedAtLockScreen);
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kPreferConstantFrameRate);
COMPONENT_EXPORT(ASH_CONSTANTS) BASE_DECLARE_FEATURE(kPrivacyIndicators);
Expand Down
2 changes: 1 addition & 1 deletion chromeos/ash/components/phonehub/fake_ping_manager.cc
Expand Up @@ -23,7 +23,7 @@ void FakePingManager::OnPingResponseReceived() {
is_waiting_for_response_ = false;
}

int FakePingManager::GetNumPingRequest() const {
int FakePingManager::GetNumPingRequests() const {
return num_ping_requests_;
}

Expand Down
4 changes: 2 additions & 2 deletions chromeos/ash/components/phonehub/fake_ping_manager.h
Expand Up @@ -19,11 +19,11 @@ class FakePingManager : public PingManager {

void OnPingResponseReceived();

int GetNumPingRequest() const;
int GetNumPingRequests() const;
bool GetIsWaitingForResponse() const;

private:
int num_ping_requests_;
int num_ping_requests_ = 0;
bool is_waiting_for_response_;
};

Expand Down
4 changes: 2 additions & 2 deletions chromeos/ash/components/phonehub/ping_manager_impl.cc
Expand Up @@ -4,6 +4,7 @@

#include "chromeos/ash/components/phonehub/ping_manager_impl.h"

#include "ash/constants/ash_features.h"
#include "base/metrics/histogram_functions.h"
#include "chromeos/ash/components/multidevice/logging/logging.h"
#include "chromeos/ash/components/phonehub/message_receiver_impl.h"
Expand All @@ -15,7 +16,6 @@
namespace ash::phonehub {

const proto::PingRequest kDefaultPingRequest;
constexpr base::TimeDelta kPingTimeout = base::Seconds(2);

PingManagerImpl::PingManagerImpl(
secure_channel::ConnectionManager* connection_manager,
Expand Down Expand Up @@ -67,7 +67,7 @@ void PingManagerImpl::SendPingRequest() {
message_sender_->SendPingRequest(kDefaultPingRequest);

ping_sent_timestamp_ = base::TimeTicks::Now();
ping_timeout_timer_.Start(FROM_HERE, kPingTimeout,
ping_timeout_timer_.Start(FROM_HERE, features::kPhoneHubPingTimeout.Get(),
base::BindOnce(&PingManagerImpl::OnPingTimerFired,
base::Unretained(this)));
is_waiting_for_response_ = true;
Expand Down
Expand Up @@ -206,7 +206,7 @@ TEST_F(PingManagerImplTest, OnPingTimerFired) {
EXPECT_TRUE(ping_manager_->is_waiting_for_response());
EXPECT_TRUE(ping_manager_->IsPingTimeoutTimerRunning());

task_environment_.FastForwardBy(base::Seconds(3));
task_environment_.FastForwardBy(base::Minutes(2));

histogram_tester.ExpectBucketCount(kPingManagerPingResultHistogramName, false,
1);
Expand Down

0 comments on commit c8ab130

Please sign in to comment.