Commit 6e84c729 authored by Kyle Horimoto's avatar Kyle Horimoto Committed by Commit Bot

[CrOS PhoneHub] Add support for the notification access setup flow

This CL adds NotificationAccessManager::AttemptNotificationSetup(),
which starts the flow for enabling notification access. Note that the
real implementation for this flow has not yet been completed. However,
the new code added to FakeNotificationAccessManager will unblock UI
work which needs this code in place to write unit tests.

A follow-up CL will actually implement the flow itself.

Bug: 1106937
Change-Id: I2d0ccedbe758252857ec2238a294b8a2a1f31eeb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2354657
Commit-Queue: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: default avatarJosh Nohle <nohle@chromium.org>
Cr-Commit-Position: refs/heads/master@{#797949}
parent c678e84b
...@@ -18,6 +18,8 @@ static_library("phonehub") { ...@@ -18,6 +18,8 @@ static_library("phonehub") {
"notification_access_manager.h", "notification_access_manager.h",
"notification_access_manager_impl.cc", "notification_access_manager_impl.cc",
"notification_access_manager_impl.h", "notification_access_manager_impl.h",
"notification_access_setup_operation.cc",
"notification_access_setup_operation.h",
"phone_hub_manager.cc", "phone_hub_manager.cc",
"phone_hub_manager.h", "phone_hub_manager.h",
"pref_names.cc", "pref_names.cc",
......
...@@ -26,5 +26,15 @@ bool FakeNotificationAccessManager::HasAccessBeenGranted() const { ...@@ -26,5 +26,15 @@ bool FakeNotificationAccessManager::HasAccessBeenGranted() const {
return has_access_been_granted_; return has_access_been_granted_;
} }
void FakeNotificationAccessManager::SetNotificationSetupOperationStatus(
NotificationAccessSetupOperation::Status new_status) {
if (new_status ==
NotificationAccessSetupOperation::Status::kCompletedSuccessfully) {
SetHasAccessBeenGranted(true);
}
NotificationAccessManager::SetNotificationSetupOperationStatus(new_status);
}
} // namespace phonehub } // namespace phonehub
} // namespace chromeos } // namespace chromeos
...@@ -15,7 +15,11 @@ class FakeNotificationAccessManager : public NotificationAccessManager { ...@@ -15,7 +15,11 @@ class FakeNotificationAccessManager : public NotificationAccessManager {
explicit FakeNotificationAccessManager(bool has_access_been_granted = false); explicit FakeNotificationAccessManager(bool has_access_been_granted = false);
~FakeNotificationAccessManager() override; ~FakeNotificationAccessManager() override;
using NotificationAccessManager::IsSetupOperationInProgress;
void SetHasAccessBeenGranted(bool has_access_been_granted); void SetHasAccessBeenGranted(bool has_access_been_granted);
void SetNotificationSetupOperationStatus(
NotificationAccessSetupOperation::Status new_status);
// NotificationAccessManager: // NotificationAccessManager:
bool HasAccessBeenGranted() const override; bool HasAccessBeenGranted() const override;
......
...@@ -4,6 +4,10 @@ ...@@ -4,6 +4,10 @@
#include "chromeos/components/phonehub/notification_access_manager.h" #include "chromeos/components/phonehub/notification_access_manager.h"
#include "base/bind.h"
#include "base/memory/ptr_util.h"
#include "chromeos/components/multidevice/logging/logging.h"
namespace chromeos { namespace chromeos {
namespace phonehub { namespace phonehub {
...@@ -11,6 +15,25 @@ NotificationAccessManager::NotificationAccessManager() = default; ...@@ -11,6 +15,25 @@ NotificationAccessManager::NotificationAccessManager() = default;
NotificationAccessManager::~NotificationAccessManager() = default; NotificationAccessManager::~NotificationAccessManager() = default;
std::unique_ptr<NotificationAccessSetupOperation>
NotificationAccessManager::AttemptNotificationSetup(
NotificationAccessSetupOperation::Delegate* delegate) {
if (HasAccessBeenGranted())
return nullptr;
int operation_id = next_operation_id_;
++next_operation_id_;
auto operation = base::WrapUnique(new NotificationAccessSetupOperation(
delegate,
base::BindOnce(&NotificationAccessManager::OnSetupOperationDeleted,
weak_ptr_factory_.GetWeakPtr(), operation_id)));
id_to_operation_map_.emplace(operation_id, operation.get());
OnSetupAttemptStarted();
return operation;
}
void NotificationAccessManager::AddObserver(Observer* observer) { void NotificationAccessManager::AddObserver(Observer* observer) {
observer_list_.AddObserver(observer); observer_list_.AddObserver(observer);
} }
...@@ -24,5 +47,32 @@ void NotificationAccessManager::NotifyNotificationAccessChanged() { ...@@ -24,5 +47,32 @@ void NotificationAccessManager::NotifyNotificationAccessChanged() {
observer.OnNotificationAccessChanged(); observer.OnNotificationAccessChanged();
} }
void NotificationAccessManager::SetNotificationSetupOperationStatus(
NotificationAccessSetupOperation::Status new_status) {
DCHECK(IsSetupOperationInProgress());
PA_LOG(INFO) << "Notification access setup flow - new status: " << new_status;
for (auto& it : id_to_operation_map_)
it.second->NotifyStatusChanged(new_status);
if (NotificationAccessSetupOperation::IsFinalStatus(new_status))
id_to_operation_map_.clear();
}
bool NotificationAccessManager::IsSetupOperationInProgress() const {
return !id_to_operation_map_.empty();
}
void NotificationAccessManager::OnSetupOperationDeleted(int operation_id) {
auto it = id_to_operation_map_.find(operation_id);
if (it == id_to_operation_map_.end())
return;
id_to_operation_map_.erase(it);
if (id_to_operation_map_.empty())
OnSetupAttemptEnded();
}
} // namespace phonehub } // namespace phonehub
} // namespace chromeos } // namespace chromeos
...@@ -5,8 +5,11 @@ ...@@ -5,8 +5,11 @@
#ifndef CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_H_ #ifndef CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_H_
#define CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_H_ #define CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_MANAGER_H_
#include "base/containers/flat_map.h"
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h" #include "base/observer_list.h"
#include "base/observer_list_types.h" #include "base/observer_list_types.h"
#include "chromeos/components/phonehub/notification_access_setup_operation.h"
namespace chromeos { namespace chromeos {
namespace phonehub { namespace phonehub {
...@@ -17,6 +20,9 @@ namespace phonehub { ...@@ -17,6 +20,9 @@ namespace phonehub {
// Phone Hub connection to the phone has never succeeded, we assume that access // Phone Hub connection to the phone has never succeeded, we assume that access
// has not yet been granted. If there is no active Phone Hub connection, we // has not yet been granted. If there is no active Phone Hub connection, we
// assume that the last access value seen is the current value. // assume that the last access value seen is the current value.
//
// Additionally, this class provides an API for requesting the notification
// access setup flow via AttemptNotificationSetup().
class NotificationAccessManager { class NotificationAccessManager {
public: public:
class Observer : public base::CheckedObserver { class Observer : public base::CheckedObserver {
...@@ -35,6 +41,19 @@ class NotificationAccessManager { ...@@ -35,6 +41,19 @@ class NotificationAccessManager {
virtual bool HasAccessBeenGranted() const = 0; virtual bool HasAccessBeenGranted() const = 0;
// Starts an attempt to enable the notification access. |delegate| will be
// updated with the status of the flow as long as the operation object
// returned by this function remains instantiated.
//
// To cancel an ongoing setup attempt, delete the operation. If a setup
// attempt fails, clients can retry by calling AttemptNotificationSetup()
// again to start a new attempt.
//
// If notification access has already been granted, this function returns null
// since there is nothing to set up.
std::unique_ptr<NotificationAccessSetupOperation> AttemptNotificationSetup(
NotificationAccessSetupOperation::Delegate* delegate);
void AddObserver(Observer* observer); void AddObserver(Observer* observer);
void RemoveObserver(Observer* observer); void RemoveObserver(Observer* observer);
...@@ -42,9 +61,23 @@ class NotificationAccessManager { ...@@ -42,9 +61,23 @@ class NotificationAccessManager {
NotificationAccessManager(); NotificationAccessManager();
void NotifyNotificationAccessChanged(); void NotifyNotificationAccessChanged();
void SetNotificationSetupOperationStatus(
NotificationAccessSetupOperation::Status new_status);
bool IsSetupOperationInProgress() const;
virtual void OnSetupAttemptStarted() {}
virtual void OnSetupAttemptEnded() {}
private: private:
friend class NotificationAccessManagerImplTest;
void OnSetupOperationDeleted(int operation_id);
int next_operation_id_ = 0;
base::flat_map<int, NotificationAccessSetupOperation*> id_to_operation_map_;
base::ObserverList<Observer> observer_list_; base::ObserverList<Observer> observer_list_;
base::WeakPtrFactory<NotificationAccessManager> weak_ptr_factory_{this};
}; };
} // namespace phonehub } // namespace phonehub
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
#include "chromeos/components/phonehub/notification_access_manager_impl.h" #include "chromeos/components/phonehub/notification_access_manager_impl.h"
#include "chromeos/components/multidevice/logging/logging.h"
#include "chromeos/components/phonehub/pref_names.h" #include "chromeos/components/phonehub/pref_names.h"
#include "components/prefs/pref_registry_simple.h" #include "components/prefs/pref_registry_simple.h"
#include "components/prefs/pref_service.h" #include "components/prefs/pref_service.h"
...@@ -27,5 +28,15 @@ bool NotificationAccessManagerImpl::HasAccessBeenGranted() const { ...@@ -27,5 +28,15 @@ bool NotificationAccessManagerImpl::HasAccessBeenGranted() const {
return pref_service_->GetBoolean(prefs::kNotificationAccessGranted); return pref_service_->GetBoolean(prefs::kNotificationAccessGranted);
} }
void NotificationAccessManagerImpl::OnSetupAttemptStarted() {
PA_LOG(INFO) << "Notification access setup flow started.";
// TODO(khorimoto): Attempt notification setup flow.
}
void NotificationAccessManagerImpl::OnSetupAttemptEnded() {
PA_LOG(INFO) << "Notification access setup flow ended.";
// TODO(khorimoto): Stop ongoing notification setup flow.
}
} // namespace phonehub } // namespace phonehub
} // namespace chromeos } // namespace chromeos
...@@ -28,6 +28,8 @@ class NotificationAccessManagerImpl : public NotificationAccessManager { ...@@ -28,6 +28,8 @@ class NotificationAccessManagerImpl : public NotificationAccessManager {
private: private:
// NotificationAccessManager: // NotificationAccessManager:
bool HasAccessBeenGranted() const override; bool HasAccessBeenGranted() const override;
void OnSetupAttemptStarted() override;
void OnSetupAttemptEnded() override;
PrefService* pref_service_; PrefService* pref_service_;
}; };
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
#include <memory> #include <memory>
#include "chromeos/components/phonehub/notification_access_setup_operation.h"
#include "chromeos/components/phonehub/pref_names.h" #include "chromeos/components/phonehub/pref_names.h"
#include "components/prefs/testing_pref_service.h" #include "components/prefs/testing_pref_service.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
...@@ -28,6 +29,25 @@ class FakeObserver : public NotificationAccessManager::Observer { ...@@ -28,6 +29,25 @@ class FakeObserver : public NotificationAccessManager::Observer {
size_t num_calls_ = 0; size_t num_calls_ = 0;
}; };
class FakeOperationDelegate
: public NotificationAccessSetupOperation::Delegate {
public:
FakeOperationDelegate() = default;
~FakeOperationDelegate() override = default;
NotificationAccessSetupOperation::Status status() const { return status_; }
// NotificationAccessSetupOperation::Delegate:
void OnStatusChange(
NotificationAccessSetupOperation::Status new_status) override {
status_ = new_status;
}
private:
NotificationAccessSetupOperation::Status status_ =
NotificationAccessSetupOperation::Status::kConnecting;
};
} // namespace } // namespace
class NotificationAccessManagerImplTest : public testing::Test { class NotificationAccessManagerImplTest : public testing::Test {
...@@ -50,7 +70,14 @@ class NotificationAccessManagerImplTest : public testing::Test { ...@@ -50,7 +70,14 @@ class NotificationAccessManagerImplTest : public testing::Test {
manager_ = std::make_unique<NotificationAccessManagerImpl>(&pref_service_); manager_ = std::make_unique<NotificationAccessManagerImpl>(&pref_service_);
} }
std::unique_ptr<NotificationAccessSetupOperation> StartSetupOperation() {
return manager_->AttemptNotificationSetup(&fake_delegate_);
}
bool GetHasAccessBeenGranted() { return manager_->HasAccessBeenGranted(); } bool GetHasAccessBeenGranted() { return manager_->HasAccessBeenGranted(); }
bool IsSetupOperationInProgress() {
return manager_->IsSetupOperationInProgress();
}
size_t GetNumObserverCalls() const { return fake_observer_.num_calls(); } size_t GetNumObserverCalls() const { return fake_observer_.num_calls(); }
...@@ -58,17 +85,30 @@ class NotificationAccessManagerImplTest : public testing::Test { ...@@ -58,17 +85,30 @@ class NotificationAccessManagerImplTest : public testing::Test {
TestingPrefServiceSimple pref_service_; TestingPrefServiceSimple pref_service_;
FakeObserver fake_observer_; FakeObserver fake_observer_;
FakeOperationDelegate fake_delegate_;
std::unique_ptr<NotificationAccessManager> manager_; std::unique_ptr<NotificationAccessManager> manager_;
}; };
TEST_F(NotificationAccessManagerImplTest, InitiallyGranted) { TEST_F(NotificationAccessManagerImplTest, InitiallyGranted) {
Initialize(/*initial_has_access_been_granted=*/true); Initialize(/*initial_has_access_been_granted=*/true);
EXPECT_TRUE(GetHasAccessBeenGranted()); EXPECT_TRUE(GetHasAccessBeenGranted());
// Cannot start the notification access setup flow if access has already been
// granted.
auto operation = StartSetupOperation();
EXPECT_FALSE(operation);
} }
TEST_F(NotificationAccessManagerImplTest, InitiallyNotGranted) { TEST_F(NotificationAccessManagerImplTest, InitiallyNotGranted) {
Initialize(/*initial_has_access_been_granted=*/false); Initialize(/*initial_has_access_been_granted=*/false);
EXPECT_FALSE(GetHasAccessBeenGranted()); EXPECT_FALSE(GetHasAccessBeenGranted());
auto operation = StartSetupOperation();
EXPECT_TRUE(operation);
EXPECT_TRUE(IsSetupOperationInProgress());
operation.reset();
EXPECT_FALSE(IsSetupOperationInProgress());
} }
} // namespace phonehub } // namespace phonehub
......
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chromeos/components/phonehub/notification_access_setup_operation.h"
#include <array>
#include "base/check.h"
#include "base/stl_util.h"
namespace chromeos {
namespace phonehub {
namespace {
// Status values which are considered "final" - i.e., once the status of an
// operation changes to one of these values, the operation has completed. These
// status values indicate either a success or a fatal error.
constexpr std::array<NotificationAccessSetupOperation::Status, 3>
kOperationFinishedStatus{
NotificationAccessSetupOperation::Status::kTimedOutConnecting,
NotificationAccessSetupOperation::Status::kConnectionDisconnected,
NotificationAccessSetupOperation::Status::kCompletedSuccessfully,
};
} // namespace
// static
bool NotificationAccessSetupOperation::IsFinalStatus(Status status) {
return base::Contains(kOperationFinishedStatus, status);
}
NotificationAccessSetupOperation::NotificationAccessSetupOperation(
Delegate* delegate,
base::OnceClosure destructor_callback)
: delegate_(delegate),
destructor_callback_(std::move(destructor_callback)) {
DCHECK(delegate_);
DCHECK(destructor_callback_);
}
NotificationAccessSetupOperation::~NotificationAccessSetupOperation() {
std::move(destructor_callback_).Run();
}
void NotificationAccessSetupOperation::NotifyStatusChanged(Status new_status) {
delegate_->OnStatusChange(new_status);
}
std::ostream& operator<<(std::ostream& stream,
NotificationAccessSetupOperation::Status status) {
switch (status) {
case NotificationAccessSetupOperation::Status::kConnecting:
stream << "[Connecting]";
break;
case NotificationAccessSetupOperation::Status::kTimedOutConnecting:
stream << "[Timed out connecting]";
break;
case NotificationAccessSetupOperation::Status::kConnectionDisconnected:
stream << "[Connection disconnected]";
break;
case NotificationAccessSetupOperation::Status::
kSentMessageToPhoneAndWaitingForResponse:
stream << "[Sent message to phone; waiting for response]";
break;
case NotificationAccessSetupOperation::Status::kCompletedSuccessfully:
stream << "[Completed successfully]";
break;
}
return stream;
}
} // namespace phonehub
} // namespace chromeos
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_SETUP_OPERATION_H_
#define CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_SETUP_OPERATION_H_
#include <ostream>
#include "base/callback.h"
namespace chromeos {
namespace phonehub {
// Implements the notification access setup flow. This flow involves:
// (1) Creating a connection to the phone if one does not already exist.
// (2) Sending a message to the phone which asks it to begin the setup flow;
// upon receipt of the message, the phone displays a UI which asks the
// user to enable notification access for Phone Hub.
// (3) Waiting for the user to complete the flow; once the flow is complete, the
// phone sends a message back to this device which indicates that
// notification access has been granted.
//
// If an instance of this class exists, the flow continues until the status
// changes to a "final" status (i.e., a success or a fatal error). To cancel the
// ongoing setup operation, simply delete the instance of this class.
class NotificationAccessSetupOperation {
public:
// Note: Numerical values should not be changed because they must stay in
// sync with values declared in JS.
enum class Status {
// Connecting to the phone in order to set up notification access.
kConnecting = 0,
// No connection was able to be made to the phone within the expected time
// period.
kTimedOutConnecting = 1,
// A connection to the phone was successful, but it unexpectedly became
// disconnected before the setup flow could complete.
kConnectionDisconnected = 2,
// A connection to the phone has succeeded, and a message has been sent to
// the phone to start the notification access opt-in flow. However, the user
// has not yet completed the flow phone-side.
kSentMessageToPhoneAndWaitingForResponse = 3,
// The user has completed the phone-side opt-in flow.
kCompletedSuccessfully = 4,
};
// Returns true if the provided status is the final one for this operation,
// indicating either success or failure.
static bool IsFinalStatus(Status status);
class Delegate {
public:
virtual ~Delegate() = default;
// Called when status of the setup flow has changed.
virtual void OnStatusChange(Status new_status) = 0;
};
NotificationAccessSetupOperation(const NotificationAccessSetupOperation&) =
delete;
NotificationAccessSetupOperation& operator=(
const NotificationAccessSetupOperation&) = delete;
virtual ~NotificationAccessSetupOperation();
private:
friend class NotificationAccessManager;
NotificationAccessSetupOperation(Delegate* delegate,
base::OnceClosure destructor_callback);
void NotifyStatusChanged(Status new_status);
Delegate* const delegate_;
base::OnceClosure destructor_callback_;
};
std::ostream& operator<<(std::ostream& stream,
NotificationAccessSetupOperation::Status status);
} // namespace phonehub
} // namespace chromeos
#endif // CHROMEOS_COMPONENTS_PHONEHUB_NOTIFICATION_ACCESS_SETUP_OPERATION_H_
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment