Skip to content

Commit

Permalink
Notification scheduler: Add impression mapping proto.
Browse files Browse the repository at this point in the history
Any user action can map to custom defined impression result. For example,
a notification dismiss can be negative user impression where by default
it's neutral.

This CL adds the proto buffer field for the impression mapping. A
following CL will hook the proto to application logic layer.

Bug: 965133
Change-Id: I36a2b592be9f192975d574f6d7080b082df64eff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1697370
Commit-Queue: Xing Liu <xingliu@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Cr-Commit-Position: refs/heads/master@{#676658}
  • Loading branch information
Xing Liu authored and Commit Bot committed Jul 11, 2019
1 parent 15cd1a8 commit df3602c
Show file tree
Hide file tree
Showing 9 changed files with 97 additions and 3 deletions.
11 changes: 10 additions & 1 deletion chrome/browser/notifications/proto/impression.proto
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ package notifications.proto;
// Contains data to determine when a notification should be shown to the user
// and the user impression towards this notification. Should match Impression in
// impression_types.h.
// Next tag: 7
// Next tag: 8
message Impression {
// The type of user feedback from a displayed notification. Should match
// UserFeedback in notification_scheduler_types.h.
Expand Down Expand Up @@ -41,6 +41,13 @@ message Impression {
EVENING = 2;
}

// Used to override default user action and impression mapping.
// Next tag: 3
message ImpressionMapping {
optional UserFeedback user_feedback = 1;
optional ImpressionResult impression_result = 2;
}

// Creation time stamp in milliseconds since epoch.
optional int64 create_time = 1;

Expand All @@ -59,4 +66,6 @@ message Impression {

// The unique identifier of the notification.
optional string guid = 6;

repeated ImpressionMapping impression_mapping = 7;
}
4 changes: 4 additions & 0 deletions chrome/browser/notifications/proto/notification_entry.proto
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,20 @@ option optimize_for = LITE_RUNTIME;
package notifications.proto;

import "client_state.proto";
import "impression.proto";
import "notification_data.proto";

// Defines scheduling and throttling details.
// Next tag: 3
message ScheduleParams {
enum Priority {
LOW = 0;
HIGH = 1;
NO_THROTTLE = 2;
}

optional Priority priority = 1;
repeated Impression.ImpressionMapping impression_mapping = 2;
}

// The notification entry that contains all data for a scheduled notification.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,15 @@ Impression::Impression(SchedulerClientType type,
const base::Time& create_time)
: create_time(create_time), guid(guid), type(type) {}

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

Impression::~Impression() = default;

bool Impression::operator==(const Impression& other) const {
return create_time == other.create_time && feedback == other.feedback &&
impression == other.impression && integrated == other.integrated &&
task_start_time == other.task_start_time && guid == other.guid &&
type == other.type;
type == other.type && impression_mapping == other.impression_mapping;
}

SuppressionInfo::SuppressionInfo(const base::Time& last_trigger,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ struct Impression {
Impression(SchedulerClientType type,
const std::string& guid,
const base::Time& create_time);
Impression(const Impression& other);
~Impression();

bool operator==(const Impression& other) const;

Expand Down Expand Up @@ -58,6 +60,9 @@ struct Impression {
// initialized.
// TODO(xingliu): Consider to persist this as well.
SchedulerClientType type = SchedulerClientType::kUnknown;

// Used to override default impression result.
std::map<UserFeedback, ImpressionResult> impression_mapping;
};

// Contains details about supression and recovery after suppression expired.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,12 +267,28 @@ ScheduleParams::Priority ScheduleParamsPriorityFromProto(
void ScheduleParamsToProto(ScheduleParams* params,
proto::ScheduleParams* proto) {
proto->set_priority(ScheduleParamsPriorityToProto(params->priority));

for (const auto& mapping : params->impression_mapping) {
auto* proto_impression_mapping = proto->add_impression_mapping();
proto_impression_mapping->set_user_feedback(ToUserFeedback(mapping.first));
proto_impression_mapping->set_impression_result(
ToImpressionResult(mapping.second));
}
}

// Converts ScheduleParams from proto buffer type.
void ScheduleParamsFromProto(proto::ScheduleParams* proto,
ScheduleParams* params) {
params->priority = ScheduleParamsPriorityFromProto(proto->priority());

for (int i = 0; i < proto->impression_mapping_size(); ++i) {
const auto& proto_impression_mapping = proto->impression_mapping(i);
auto user_feedback =
FromUserFeedback(proto_impression_mapping.user_feedback());
auto impression_result =
FromImpressionResult(proto_impression_mapping.impression_result());
params->impression_mapping[user_feedback] = impression_result;
}
}

} // namespace
Expand Down Expand Up @@ -303,6 +319,14 @@ void ClientStateToProto(ClientState* client_state,
impression_ptr->set_task_start_time(
ToSchedulerTaskTime(impression.task_start_time));
impression_ptr->set_guid(impression.guid);

for (const auto& mapping : impression.impression_mapping) {
auto* proto_impression_mapping = impression_ptr->add_impression_mapping();
proto_impression_mapping->set_user_feedback(
ToUserFeedback(mapping.first));
proto_impression_mapping->set_impression_result(
ToImpressionResult(mapping.second));
}
}

if (client_state->suppression_info.has_value()) {
Expand Down Expand Up @@ -334,6 +358,17 @@ void ClientStateFromProto(proto::ClientState* proto,
FromSchedulerTaskTime(proto_impression.task_start_time());
impression.guid = proto_impression.guid();
impression.type = client_state->type;

for (int i = 0; i < proto_impression.impression_mapping_size(); ++i) {
const auto& proto_impression_mapping =
proto_impression.impression_mapping(i);
auto user_feedback =
FromUserFeedback(proto_impression_mapping.user_feedback());
auto impression_result =
FromImpressionResult(proto_impression_mapping.impression_result());
impression.impression_mapping[user_feedback] = impression_result;
}

client_state->impressions.emplace_back(std::move(impression));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@ TEST(ProtoConversionTest, ImpressionProtoConversion) {
first_impression.task_start_time = task_start_time;
TestClientStateConversion(&client_state);
}

// Verify impression mapping.
first_impression.impression_mapping[UserFeedback::kClick] =
ImpressionResult::kNeutral;
TestClientStateConversion(&client_state);
}

// Verifies multiple impressions are serialized correctly.
Expand Down Expand Up @@ -195,6 +200,11 @@ TEST(ProtoConversionTest, NotificationEntryConversion) {
entry.schedule_params.priority = priority;
TestNotificationEntryConversion(&entry);
}
entry.schedule_params.impression_mapping[UserFeedback::kDismiss] =
ImpressionResult::kPositive;
entry.schedule_params.impression_mapping[UserFeedback::kClick] =
ImpressionResult::kNeutral;
TestNotificationEntryConversion(&entry);
}

// Verifies buttons are converted correctly to proto buffers.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,11 @@ namespace notifications {

ScheduleParams::ScheduleParams() : priority(Priority::kLow) {}

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

bool ScheduleParams::operator==(const ScheduleParams& other) const {
return priority == other.priority;
return priority == other.priority &&
impression_mapping == other.impression_mapping;
}

ScheduleParams::~ScheduleParams() = default;
Expand Down
13 changes: 13 additions & 0 deletions chrome/browser/notifications/scheduler/public/schedule_params.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
#ifndef CHROME_BROWSER_NOTIFICATIONS_SCHEDULER_PUBLIC_SCHEDULE_PARAMS_H_
#define CHROME_BROWSER_NOTIFICATIONS_SCHEDULER_PUBLIC_SCHEDULE_PARAMS_H_

#include <map>

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

namespace notifications {

// Specifies when to show the scheduled notification, and throttling details.
Expand All @@ -23,10 +27,19 @@ struct ScheduleParams {
};

ScheduleParams();
ScheduleParams(const ScheduleParams& other);
bool operator==(const ScheduleParams& other) const;
~ScheduleParams();

Priority priority;

// Override the default mapping from an user action to impression result. By
// default, click on the notification and helpful button click are positive
// impression and may increase feature exposure. Unhelp button click is
// negative impression and may reduce feature exposure. Dimiss/close
// notification is neutural. Only put value when need to change the default
// mapping.
std::map<UserFeedback, ImpressionResult> impression_mapping;
};

} // namespace notifications
Expand Down
11 changes: 11 additions & 0 deletions chrome/browser/notifications/scheduler/test/test_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ std::string DebugString(const NotificationEntry* entry) {
<< " \n schedule params: priority:"
<< static_cast<int>(entry->schedule_params.priority);

for (const auto& mapping : entry->schedule_params.impression_mapping) {
stream << " \n impression mapping: " << static_cast<int>(mapping.first)
<< " : " << static_cast<int>(mapping.second);
}

stream << " \n icons_id:";
for (const auto& icon_id : entry->icons_uuid)
stream << icon_id << " ";
Expand Down Expand Up @@ -130,6 +135,12 @@ std::string DebugString(const ClientState* client_state) {
<< static_cast<int>(impression.task_start_time) << "\n"
<< "guid: " << impression.guid << "\n"
<< "type: " << static_cast<int>(impression.type);

for (const auto& mapping : impression.impression_mapping) {
stream << " \n impression mapping: " << static_cast<int>(mapping.first)
<< " : " << static_cast<int>(mapping.second);
}

log += stream.str();
}

Expand Down

0 comments on commit df3602c

Please sign in to comment.