Skip to content

Commit

Permalink
[notification scheduler]: GetClientOverview public API.
Browse files Browse the repository at this point in the history
- Change public service API getImpressionDetail to
getClientOverview.

- New API returns a struct ClientOverview including info about
notifications in the scheduler also ImpressionDetails.

- The glue layer wrapped result queried from ImpressionTracker
and result queried from ScheduledNotificationManager to
ClientOverview struct.

Bug: 1025387
Change-Id: Iab6bad5be5e914730645975af86293c2b6395810
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1918651
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Xing Liu <xingliu@chromium.org>
Commit-Queue: Hesen Zhang <hesen@chromium.org>
Auto-Submit: Hesen Zhang <hesen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#716862}
  • Loading branch information
Hesen Zhang authored and Commit Bot committed Nov 20, 2019
1 parent ea86225 commit 0b109bb
Show file tree
Hide file tree
Showing 14 changed files with 115 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,15 @@ void InitAwareNotificationScheduler::DeleteAllNotifications(
weak_ptr_factory_.GetWeakPtr(), type));
}

void InitAwareNotificationScheduler::GetImpressionDetail(
void InitAwareNotificationScheduler::GetClientOverview(
SchedulerClientType type,
ImpressionDetail::ImpressionDetailCallback callback) {
ClientOverview::ClientOverviewCallback callback) {
if (IsReady()) {
impl_->GetImpressionDetail(type, std::move(callback));
impl_->GetClientOverview(type, std::move(callback));
return;
}
MaybeCacheClosure(base::BindOnce(
&InitAwareNotificationScheduler::GetImpressionDetail,
&InitAwareNotificationScheduler::GetClientOverview,
weak_ptr_factory_.GetWeakPtr(), type, std::move(callback)));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ class InitAwareNotificationScheduler : public NotificationScheduler {
void Schedule(
std::unique_ptr<NotificationParams> notification_params) override;
void DeleteAllNotifications(SchedulerClientType type) override;
void GetImpressionDetail(
void GetClientOverview(
SchedulerClientType type,
ImpressionDetail::ImpressionDetailCallback callback) override;
ClientOverview::ClientOverviewCallback callback) override;
void OnStartTask(TaskFinishedCallback callback) override;
void OnStopTask() override;
void OnUserAction(const UserActionData& action_data) override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ class MockNotificationScheduler : public NotificationScheduler {
MOCK_METHOD1(Init, void(InitCallback));
MOCK_METHOD1(Schedule, void(std::unique_ptr<NotificationParams>));
MOCK_METHOD1(DeleteAllNotifications, void(SchedulerClientType type));
MOCK_METHOD2(GetImpressionDetail,
MOCK_METHOD2(GetClientOverview,
void(SchedulerClientType type,
ImpressionDetail::ImpressionDetailCallback callback));
ClientOverview::ClientOverviewCallback callback));
MOCK_METHOD1(OnStartTask, void(TaskFinishedCallback));
MOCK_METHOD0(OnStopTask, void());
MOCK_METHOD1(OnUserAction, void(const UserActionData&));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ void NoopNotificationScheduleService::Schedule(
void NoopNotificationScheduleService::DeleteNotifications(
SchedulerClientType type) {}

void NoopNotificationScheduleService::GetImpressionDetail(
void NoopNotificationScheduleService::GetClientOverview(
SchedulerClientType,
ImpressionDetail::ImpressionDetailCallback callback) {}
ClientOverview::ClientOverviewCallback callback) {}

NotificationBackgroundTaskScheduler::Handler*
NoopNotificationScheduleService::GetBackgroundTaskSchedulerHandler() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ class NoopNotificationScheduleService
void Schedule(
std::unique_ptr<NotificationParams> notification_params) override;
void DeleteNotifications(SchedulerClientType type) override;
void GetImpressionDetail(
void GetClientOverview(
SchedulerClientType,
ImpressionDetail::ImpressionDetailCallback callback) override;
ClientOverview::ClientOverviewCallback callback) override;
NotificationBackgroundTaskScheduler::Handler*
GetBackgroundTaskSchedulerHandler() override;
UserActionHandler* GetUserActionHandler() override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,10 @@ void NotificationScheduleServiceImpl::DeleteNotifications(
scheduler_->DeleteAllNotifications(type);
}

void NotificationScheduleServiceImpl::GetImpressionDetail(
void NotificationScheduleServiceImpl::GetClientOverview(
SchedulerClientType type,
ImpressionDetail::ImpressionDetailCallback callback) {
scheduler_->GetImpressionDetail(type, std::move(callback));
ClientOverview::ClientOverviewCallback callback) {
scheduler_->GetClientOverview(type, std::move(callback));
}

NotificationBackgroundTaskScheduler::Handler*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ class NotificationScheduleServiceImpl
void Schedule(
std::unique_ptr<NotificationParams> notification_params) override;
void DeleteNotifications(SchedulerClientType type) override;
void GetImpressionDetail(
void GetClientOverview(
SchedulerClientType,
ImpressionDetail::ImpressionDetailCallback callback) override;
ClientOverview::ClientOverviewCallback callback) override;
NotificationBackgroundTaskScheduler::Handler*
GetBackgroundTaskSchedulerHandler() override;
UserActionHandler* GetUserActionHandler() override;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -227,11 +227,23 @@ class NotificationSchedulerImpl : public NotificationScheduler {
context_->notification_manager()->DeleteNotifications(type);
}

void GetImpressionDetail(
void GetClientOverview(
SchedulerClientType type,
ImpressionDetail::ImpressionDetailCallback callback) override {
context_->impression_tracker()->GetImpressionDetail(type,
std::move(callback));
ClientOverview::ClientOverviewCallback callback) override {
context_->impression_tracker()->GetImpressionDetail(
type, base::BindOnce(
&NotificationSchedulerImpl::OnImpressionDetailQueryCompleted,
weak_ptr_factory_.GetWeakPtr(), type, std::move(callback)));
}

void OnImpressionDetailQueryCompleted(
SchedulerClientType type,
ClientOverview::ClientOverviewCallback callback,
ImpressionDetail impression_detail) {
std::vector<const NotificationEntry*> notifications;
context_->notification_manager()->GetNotifications(type, &notifications);
ClientOverview result(std::move(impression_detail), notifications.size());
std::move(callback).Run(std::move(result));
}

void OnInitialized(InitCallback init_callback, bool success) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

#include "base/callback.h"
#include "base/macros.h"
#include "chrome/browser/notifications/scheduler/public/impression_detail.h"
#include "chrome/browser/notifications/scheduler/public/client_overview.h"
#include "chrome/browser/notifications/scheduler/public/notification_background_task_scheduler.h"
#include "chrome/browser/notifications/scheduler/public/user_action_handler.h"

Expand Down Expand Up @@ -39,10 +39,11 @@ class NotificationScheduler
virtual void Schedule(
std::unique_ptr<NotificationParams> notification_params) = 0;

// Queries impression detail for a given |SchedulerClientType|.
virtual void GetImpressionDetail(
// Queries an overview of client information for a given
// |SchedulerClientType| including impression details.
virtual void GetClientOverview(
SchedulerClientType type,
ImpressionDetail::ImpressionDetailCallback callback) = 0;
ClientOverview::ClientOverviewCallback callback) = 0;

// Deletes all notifications of a given |SchedulerClientType|.
virtual void DeleteAllNotifications(SchedulerClientType type) = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,14 +241,18 @@ TEST_F(NotificationSchedulerTest, DeleteAllNotifications) {
scheduler()->DeleteAllNotifications(SchedulerClientType::kTest1);
}

// Test to get impression details.
TEST_F(NotificationSchedulerTest, GetImpressionDetail) {
// Test to get client overview.
TEST_F(NotificationSchedulerTest, GetClientOverview) {
Init();

EXPECT_CALL(*impression_tracker(),
GetImpressionDetail(SchedulerClientType::kTest1, _));
scheduler()->GetImpressionDetail(SchedulerClientType::kTest1,
base::DoNothing());
GetImpressionDetail(SchedulerClientType::kTest1, _))
.WillOnce(Invoke([](SchedulerClientType type,
ImpressionDetail::ImpressionDetailCallback callback) {
std::move(callback).Run(ImpressionDetail());
}));
EXPECT_CALL(*notification_manager(), GetNotifications(_, _));
scheduler()->GetClientOverview(SchedulerClientType::kTest1,
base::DoNothing());
}

// Test to verify user actions are propagated through correctly.
Expand Down
2 changes: 2 additions & 0 deletions chrome/browser/notifications/scheduler/public/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ if (is_android) {

source_set("public") {
sources = [
"client_overview.cc",
"client_overview.h",
"display_agent.cc",
"display_agent.h",
"features.cc",
Expand Down
27 changes: 27 additions & 0 deletions chrome/browser/notifications/scheduler/public/client_overview.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2019 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 "chrome/browser/notifications/scheduler/public/client_overview.h"

#include <utility>

namespace notifications {

ClientOverview::ClientOverview() : num_scheduled_notifications(0) {}

ClientOverview::ClientOverview(ImpressionDetail impression_detail,
int num_scheduled_notifications)
: impression_detail(std::move(impression_detail)),
num_scheduled_notifications(num_scheduled_notifications) {}

ClientOverview::ClientOverview(const ClientOverview& other) = default;

ClientOverview::~ClientOverview() = default;

bool ClientOverview::operator==(const ClientOverview& other) const {
return num_scheduled_notifications == other.num_scheduled_notifications &&
impression_detail == other.impression_detail;
}

} // namespace notifications
33 changes: 33 additions & 0 deletions chrome/browser/notifications/scheduler/public/client_overview.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
// Copyright 2019 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 CHROME_BROWSER_NOTIFICATIONS_SCHEDULER_PUBLIC_CLIENT_OVERVIEW_H_
#define CHROME_BROWSER_NOTIFICATIONS_SCHEDULER_PUBLIC_CLIENT_OVERVIEW_H_

#include "base/callback.h"

#include "chrome/browser/notifications/scheduler/public/impression_detail.h"

namespace notifications {

struct ClientOverview {
using ClientOverviewCallback = base::OnceCallback<void(ClientOverview)>;

ClientOverview();
ClientOverview(ImpressionDetail impression_detail,
int num_scheduled_notifications);
ClientOverview(const ClientOverview& other);
~ClientOverview();
bool operator==(const ClientOverview& other) const;

// Details of impression.
ImpressionDetail impression_detail;

// The number of notifications cached in scheduler but not displayed yet.
int num_scheduled_notifications;
};

} // namespace notifications

#endif // CHROME_BROWSER_NOTIFICATIONS_SCHEDULER_PUBLIC_CLIENT_OVERVIEW_H_
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#include <memory>

#include "base/macros.h"
#include "chrome/browser/notifications/scheduler/public/client_overview.h"
#include "chrome/browser/notifications/scheduler/public/impression_detail.h"
#include "chrome/browser/notifications/scheduler/public/notification_background_task_scheduler.h"
#include "components/keyed_service/core/keyed_service.h"
Expand All @@ -30,10 +31,11 @@ class NotificationScheduleService : public KeyedService {
// Deletes notifications of a given |SchedulerClientType|.
virtual void DeleteNotifications(SchedulerClientType type) = 0;

// Queries impression details for a given |SchedulerClientType|.
virtual void GetImpressionDetail(
// Queries an overview of notifications for a given
// |SchedulerClientType| including impression details.
virtual void GetClientOverview(
SchedulerClientType type,
ImpressionDetail::ImpressionDetailCallback callback) = 0;
ClientOverview::ClientOverviewCallback callback) = 0;

// Returns NotificationBackgroundTaskScheduler Handler.
virtual NotificationBackgroundTaskScheduler::Handler*
Expand Down

0 comments on commit 0b109bb

Please sign in to comment.