Commit ebb92497 authored by Findit's avatar Findit

Revert "Serialize more of message_center::Notification for mojo."

This reverts commit c4ea951f.

Reason for revert:

Findit (https://goo.gl/kROfz5) identified CL at revision 515427 as the
culprit for failures in the build cycles as shown on:
https://findit-for-me.appspot.com/waterfall/culprit?key=ag9zfmZpbmRpdC1mb3ItbWVyRAsSDVdmU3VzcGVjdGVkQ0wiMWNocm9taXVtL2M0ZWE5NTFmMzZlYTFjMTk2MGQ1Njk0OTQyZGE5OWMzN2M2ODE3YWYM

Sample Failed Build: https://ci.chromium.org/buildbot/chromium/Mac/34475

Original change's description:
> Serialize more of message_center::Notification for mojo.
> 
> Adds NotificationType and (partial) RichNotificationData. Now you can
> see progress bars in download item notifications on Chrome OS when
> using --enable-features=NativeNotifications
> 
> Bug: 578868
> Change-Id: I781c10387479ed921f0a7b4efce262b18f04844f
> Reviewed-on: https://chromium-review.googlesource.com/755317
> Reviewed-by: Tom Sepez <tsepez@chromium.org>
> Reviewed-by: James Cook <jamescook@chromium.org>
> Reviewed-by: Scott Violet <sky@chromium.org>
> Commit-Queue: Evan Stade <estade@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#515427}

Change-Id: I3eac7eb2400781c22fd59c8f6eb7fc2cdd80e1b3
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 578868
Reviewed-on: https://chromium-review.googlesource.com/762881
Cr-Commit-Position: refs/heads/master@{#515450}
parent 97875dcb
...@@ -183,6 +183,8 @@ DownloadItemNotification::DownloadItemNotification( ...@@ -183,6 +183,8 @@ DownloadItemNotification::DownloadItemNotification(
// overridden by UpdateNotificationData() below. // overridden by UpdateNotificationData() below.
message_center::RichNotificationData rich_notification_data; message_center::RichNotificationData rich_notification_data;
rich_notification_data.should_make_spoken_feedback_for_popup_updates = false; rich_notification_data.should_make_spoken_feedback_for_popup_updates = false;
// Dangerous notifications don't have a click handler.
rich_notification_data.clickable = !item_->IsDangerous();
notification_ = std::make_unique<message_center::Notification>( notification_ = std::make_unique<message_center::Notification>(
message_center::NOTIFICATION_TYPE_PROGRESS, GetNotificationId(), message_center::NOTIFICATION_TYPE_PROGRESS, GetNotificationId(),
base::string16(), // title base::string16(), // title
...@@ -196,8 +198,7 @@ DownloadItemNotification::DownloadItemNotification( ...@@ -196,8 +198,7 @@ DownloadItemNotification::DownloadItemNotification(
rich_notification_data, nullptr); rich_notification_data, nullptr);
notification_->set_progress(0); notification_->set_progress(0);
// Dangerous notifications don't have a click handler. notification_->set_never_timeout(false);
notification_->set_clickable(!item_->IsDangerous());
Update(); Update();
} }
......
...@@ -212,6 +212,7 @@ if (enable_message_center) { ...@@ -212,6 +212,7 @@ if (enable_message_center) {
"cocoa/popup_controller_unittest.mm", "cocoa/popup_controller_unittest.mm",
"message_center_impl_unittest.cc", "message_center_impl_unittest.cc",
"message_center_tray_unittest.cc", "message_center_tray_unittest.cc",
"mojo/struct_traits_unittest.cc",
"notification_delegate_unittest.cc", "notification_delegate_unittest.cc",
"notification_list_unittest.cc", "notification_list_unittest.cc",
"test/run_all_unittests.cc", "test/run_all_unittests.cc",
...@@ -222,6 +223,7 @@ if (enable_message_center) { ...@@ -222,6 +223,7 @@ if (enable_message_center) {
":test_support", ":test_support",
"//base", "//base",
"//base/test:test_support", "//base/test:test_support",
"//mojo/edk/system",
"//skia", "//skia",
"//testing/gmock", "//testing/gmock",
"//testing/gtest", "//testing/gtest",
...@@ -234,6 +236,7 @@ if (enable_message_center) { ...@@ -234,6 +236,7 @@ if (enable_message_center) {
"//ui/gfx/geometry", "//ui/gfx/geometry",
"//ui/gl", "//ui/gl",
"//ui/gl:test_support", "//ui/gl:test_support",
"//ui/message_center/mojo:test_interfaces",
"//ui/message_center/public/cpp", "//ui/message_center/public/cpp",
"//ui/resources", "//ui/resources",
"//ui/resources:ui_test_pak", "//ui/resources:ui_test_pak",
...@@ -245,14 +248,6 @@ if (enable_message_center) { ...@@ -245,14 +248,6 @@ if (enable_message_center) {
"//ui/resources:ui_test_pak_data", "//ui/resources:ui_test_pak_data",
] ]
if (is_chromeos) {
sources += [ "mojo/struct_traits_unittest.cc" ]
deps += [
"//mojo/edk/system",
"//ui/message_center/mojo:test_interfaces",
]
}
if (is_mac) { if (is_mac) {
deps += [ "//ui/gfx:test_support" ] deps += [ "//ui/gfx:test_support" ]
} }
......
...@@ -8,34 +8,8 @@ import "mojo/common/string16.mojom"; ...@@ -8,34 +8,8 @@ import "mojo/common/string16.mojom";
import "ui/gfx/image/mojo/image.mojom"; import "ui/gfx/image/mojo/image.mojom";
import "url/mojo/url.mojom"; import "url/mojo/url.mojom";
// Matches message_center::NotificationType.
enum NotificationType {
SIMPLE = 0,
BASE_FORMAT = 1,
IMAGE = 2,
MULTIPLE = 3,
PROGRESS = 4,
CUSTOM = 5,
};
// These fields and their meanings are identical to those in
// message_center::RichNotificationData.
// TODO(estade): Add the rest of the fields for RichNotificationData.
struct RichNotificationData {
int32 progress;
mojo.common.mojom.String16 progress_status;
bool should_make_spoken_feedback_for_popup_updates;
bool clickable;
bool pinned;
mojo.common.mojom.String16 accessible_name;
uint32 accent_color;
bool use_image_as_icon;
};
// TODO(mhashmi): Add the rest of the fields for a Notification // TODO(mhashmi): Add the rest of the fields for a Notification
struct Notification { struct Notification {
NotificationType type;
// TODO(mhashmi): Server-side code (in Ash) needs to make sure this id won't // TODO(mhashmi): Server-side code (in Ash) needs to make sure this id won't
// collide with ids from different clients // collide with ids from different clients
string id; string id;
...@@ -45,6 +19,4 @@ struct Notification { ...@@ -45,6 +19,4 @@ struct Notification {
gfx.mojom.ImageSkia? icon; gfx.mojom.ImageSkia? icon;
mojo.common.mojom.String16 display_source; mojo.common.mojom.String16 display_source;
url.mojom.Url origin_url; url.mojom.Url origin_url;
RichNotificationData optional_fields;
}; };
...@@ -9,8 +9,5 @@ deps = [ ...@@ -9,8 +9,5 @@ deps = [
"//ui/gfx/image/mojo:struct_traits", "//ui/gfx/image/mojo:struct_traits",
"//ui/message_center/mojo:struct_traits", "//ui/message_center/mojo:struct_traits",
] ]
type_mappings = [ type_mappings =
"message_center.mojom.RichNotificationData=message_center::RichNotificationData", [ "message_center.mojom.Notification=message_center::Notification" ]
"message_center.mojom.NotificationType=message_center::NotificationType",
"message_center.mojom.Notification=message_center::Notification",
]
...@@ -10,136 +10,61 @@ ...@@ -10,136 +10,61 @@
namespace mojo { namespace mojo {
using message_center::mojom::NotificationDataView;
using message_center::mojom::RichNotificationDataDataView;
using message_center::Notification;
using message_center::RichNotificationData;
using NotificationStructTraits =
StructTraits<NotificationDataView, Notification>;
using RichNotificationDataStructTraits =
StructTraits<RichNotificationDataDataView, RichNotificationData>;
// static
int RichNotificationDataStructTraits::progress(
const message_center::RichNotificationData& r) {
return r.progress;
}
// static
const base::string16& RichNotificationDataStructTraits::progress_status(
const message_center::RichNotificationData& r) {
return r.progress_status;
}
// static
bool RichNotificationDataStructTraits::
should_make_spoken_feedback_for_popup_updates(
const message_center::RichNotificationData& r) {
return r.should_make_spoken_feedback_for_popup_updates;
}
// static
bool RichNotificationDataStructTraits::clickable(
const message_center::RichNotificationData& r) {
return r.clickable;
}
// static
bool RichNotificationDataStructTraits::pinned(
const message_center::RichNotificationData& r) {
return r.pinned;
}
// static
const base::string16& RichNotificationDataStructTraits::accessible_name(
const message_center::RichNotificationData& r) {
return r.accessible_name;
}
// static
SkColor RichNotificationDataStructTraits::accent_color(
const message_center::RichNotificationData& r) {
return r.accent_color;
}
// static // static
bool RichNotificationDataStructTraits::use_image_as_icon( const std::string& StructTraits<
const message_center::RichNotificationData& r) { message_center::mojom::NotificationDataView,
return r.use_image_as_icon; message_center::Notification>::id(const message_center::Notification& n) {
}
// static
bool RichNotificationDataStructTraits::Read(RichNotificationDataDataView data,
RichNotificationData* out) {
out->progress = data.progress();
out->should_make_spoken_feedback_for_popup_updates =
data.should_make_spoken_feedback_for_popup_updates();
out->clickable = data.clickable();
out->pinned = data.pinned();
out->accent_color = data.accent_color();
out->use_image_as_icon = data.use_image_as_icon();
return data.ReadProgressStatus(&out->progress_status) &&
data.ReadAccessibleName(&out->accessible_name);
}
// static
message_center::NotificationType NotificationStructTraits::type(
const Notification& n) {
return n.type();
}
// static
const std::string& NotificationStructTraits::id(const Notification& n) {
return n.id(); return n.id();
} }
// static // static
const base::string16& NotificationStructTraits::title(const Notification& n) { const base::string16& StructTraits<message_center::mojom::NotificationDataView,
message_center::Notification>::
title(const message_center::Notification& n) {
return n.title(); return n.title();
} }
// static // static
const base::string16& NotificationStructTraits::message(const Notification& n) { const base::string16& StructTraits<message_center::mojom::NotificationDataView,
message_center::Notification>::
message(const message_center::Notification& n) {
return n.message(); return n.message();
} }
// static // static
gfx::ImageSkia NotificationStructTraits::icon(const Notification& n) { gfx::ImageSkia StructTraits<
message_center::mojom::NotificationDataView,
message_center::Notification>::icon(const message_center::Notification& n) {
return n.icon().AsImageSkia(); return n.icon().AsImageSkia();
} }
// static // static
const base::string16& NotificationStructTraits::display_source( const base::string16& StructTraits<message_center::mojom::NotificationDataView,
const Notification& n) { message_center::Notification>::
display_source(const message_center::Notification& n) {
return n.display_source(); return n.display_source();
} }
// static // static
const GURL& NotificationStructTraits::origin_url(const Notification& n) { const GURL& StructTraits<message_center::mojom::NotificationDataView,
message_center::Notification>::
origin_url(const message_center::Notification& n) {
return n.origin_url(); return n.origin_url();
} }
// static // static
const RichNotificationData& NotificationStructTraits::optional_fields( bool StructTraits<message_center::mojom::NotificationDataView,
const Notification& n) { message_center::Notification>::
return n.rich_notification_data(); Read(message_center::mojom::NotificationDataView data,
} message_center::Notification* out) {
// static
bool NotificationStructTraits::Read(NotificationDataView data,
Notification* out) {
gfx::ImageSkia icon; gfx::ImageSkia icon;
if (!data.ReadIcon(&icon)) if (!data.ReadIcon(&icon))
return false; return false;
out->set_icon(gfx::Image(icon)); out->icon_ = gfx::Image(icon);
return EnumTraits<message_center::mojom::NotificationType, return data.ReadId(&out->id_) && data.ReadTitle(&out->title_) &&
message_center::NotificationType>::FromMojom(data.type(),
&out->type_) &&
data.ReadId(&out->id_) && data.ReadTitle(&out->title_) &&
data.ReadMessage(&out->message_) && data.ReadMessage(&out->message_) &&
data.ReadDisplaySource(&out->display_source_) && data.ReadDisplaySource(&out->display_source_) &&
data.ReadOriginUrl(&out->origin_url_) && data.ReadOriginUrl(&out->origin_url_);
data.ReadOptionalFields(&out->optional_fields_);
} }
} // namespace mojo } // namespace mojo
...@@ -5,85 +5,14 @@ ...@@ -5,85 +5,14 @@
#ifndef UI_MESSAGE_CENTER_NOTIFICATION_STRUCT_TRAITS_H_ #ifndef UI_MESSAGE_CENTER_NOTIFICATION_STRUCT_TRAITS_H_
#define UI_MESSAGE_CENTER_NOTIFICATION_STRUCT_TRAITS_H_ #define UI_MESSAGE_CENTER_NOTIFICATION_STRUCT_TRAITS_H_
#include "third_party/skia/include/core/SkColor.h"
#include "ui/message_center/mojo/notification.mojom-shared.h" #include "ui/message_center/mojo/notification.mojom-shared.h"
#include "ui/message_center/notification.h" #include "ui/message_center/notification.h"
namespace mojo { namespace mojo {
template <>
struct EnumTraits<message_center::mojom::NotificationType,
message_center::NotificationType> {
static message_center::mojom::NotificationType ToMojom(
message_center::NotificationType type) {
switch (type) {
case message_center::NOTIFICATION_TYPE_SIMPLE:
return message_center::mojom::NotificationType::SIMPLE;
case message_center::NOTIFICATION_TYPE_BASE_FORMAT:
return message_center::mojom::NotificationType::BASE_FORMAT;
case message_center::NOTIFICATION_TYPE_IMAGE:
return message_center::mojom::NotificationType::IMAGE;
case message_center::NOTIFICATION_TYPE_MULTIPLE:
return message_center::mojom::NotificationType::MULTIPLE;
case message_center::NOTIFICATION_TYPE_PROGRESS:
return message_center::mojom::NotificationType::PROGRESS;
case message_center::NOTIFICATION_TYPE_CUSTOM:
return message_center::mojom::NotificationType::CUSTOM;
}
NOTREACHED();
return message_center::mojom::NotificationType::SIMPLE;
}
static bool FromMojom(message_center::mojom::NotificationType input,
message_center::NotificationType* out) {
switch (input) {
case message_center::mojom::NotificationType::SIMPLE:
*out = message_center::NOTIFICATION_TYPE_SIMPLE;
return true;
case message_center::mojom::NotificationType::BASE_FORMAT:
*out = message_center::NOTIFICATION_TYPE_BASE_FORMAT;
return true;
case message_center::mojom::NotificationType::IMAGE:
*out = message_center::NOTIFICATION_TYPE_IMAGE;
return true;
case message_center::mojom::NotificationType::MULTIPLE:
*out = message_center::NOTIFICATION_TYPE_MULTIPLE;
return true;
case message_center::mojom::NotificationType::PROGRESS:
*out = message_center::NOTIFICATION_TYPE_PROGRESS;
return true;
case message_center::mojom::NotificationType::CUSTOM:
*out = message_center::NOTIFICATION_TYPE_CUSTOM;
return true;
}
NOTREACHED();
return false;
}
};
template <>
struct StructTraits<message_center::mojom::RichNotificationDataDataView,
message_center::RichNotificationData> {
static int progress(const message_center::RichNotificationData& r);
static const base::string16& progress_status(
const message_center::RichNotificationData& r);
static bool should_make_spoken_feedback_for_popup_updates(
const message_center::RichNotificationData& r);
static bool clickable(const message_center::RichNotificationData& r);
static bool pinned(const message_center::RichNotificationData& r);
static const base::string16& accessible_name(
const message_center::RichNotificationData& r);
static SkColor accent_color(const message_center::RichNotificationData& r);
static bool use_image_as_icon(const message_center::RichNotificationData& r);
static bool Read(message_center::mojom::RichNotificationDataDataView data,
message_center::RichNotificationData* out);
};
template <> template <>
struct StructTraits<message_center::mojom::NotificationDataView, struct StructTraits<message_center::mojom::NotificationDataView,
message_center::Notification> { message_center::Notification> {
static message_center::NotificationType type(
const message_center::Notification& n);
static const std::string& id(const message_center::Notification& n); static const std::string& id(const message_center::Notification& n);
static const base::string16& title(const message_center::Notification& n); static const base::string16& title(const message_center::Notification& n);
static const base::string16& message(const message_center::Notification& n); static const base::string16& message(const message_center::Notification& n);
...@@ -91,8 +20,6 @@ struct StructTraits<message_center::mojom::NotificationDataView, ...@@ -91,8 +20,6 @@ struct StructTraits<message_center::mojom::NotificationDataView,
static const base::string16& display_source( static const base::string16& display_source(
const message_center::Notification& n); const message_center::Notification& n);
static const GURL& origin_url(const message_center::Notification& n); static const GURL& origin_url(const message_center::Notification& n);
static const message_center::RichNotificationData& optional_fields(
const message_center::Notification& n);
static bool Read(message_center::mojom::NotificationDataView data, static bool Read(message_center::mojom::NotificationDataView data,
message_center::Notification* out); message_center::Notification* out);
}; };
......
...@@ -28,7 +28,6 @@ class StructTraitsTest : public testing::Test, public mojom::TraitsTestService { ...@@ -28,7 +28,6 @@ class StructTraitsTest : public testing::Test, public mojom::TraitsTestService {
} }
void Compare(const Notification& input, const Notification& output) { void Compare(const Notification& input, const Notification& output) {
EXPECT_EQ(input.type(), output.type());
EXPECT_EQ(input.id(), output.id()); EXPECT_EQ(input.id(), output.id());
EXPECT_EQ(input.title(), output.title()); EXPECT_EQ(input.title(), output.title());
EXPECT_EQ(input.message(), output.message()); EXPECT_EQ(input.message(), output.message());
...@@ -36,16 +35,6 @@ class StructTraitsTest : public testing::Test, public mojom::TraitsTestService { ...@@ -36,16 +35,6 @@ class StructTraitsTest : public testing::Test, public mojom::TraitsTestService {
EXPECT_EQ(input.icon().Height(), output.icon().Height()); EXPECT_EQ(input.icon().Height(), output.icon().Height());
EXPECT_EQ(input.display_source(), output.display_source()); EXPECT_EQ(input.display_source(), output.display_source());
EXPECT_EQ(input.origin_url(), output.origin_url()); EXPECT_EQ(input.origin_url(), output.origin_url());
EXPECT_EQ(input.progress(), output.progress());
EXPECT_EQ(input.progress_status(), output.progress_status());
EXPECT_EQ(input.rich_notification_data()
.should_make_spoken_feedback_for_popup_updates,
output.rich_notification_data()
.should_make_spoken_feedback_for_popup_updates);
EXPECT_EQ(input.clickable(), output.clickable());
EXPECT_EQ(input.accessible_name(), output.accessible_name());
EXPECT_EQ(input.accent_color(), output.accent_color());
EXPECT_EQ(input.use_image_as_icon(), output.use_image_as_icon());
} }
private: private:
...@@ -74,21 +63,10 @@ TEST_F(StructTraitsTest, Notification) { ...@@ -74,21 +63,10 @@ TEST_F(StructTraitsTest, Notification) {
NotifierId notifier_id(NotifierId::NotifierType::APPLICATION, id); NotifierId notifier_id(NotifierId::NotifierType::APPLICATION, id);
Notification input(type, id, title, message, icon, display_source, origin_url, Notification input(type, id, title, message, icon, display_source, origin_url,
notifier_id, RichNotificationData(), nullptr); notifier_id, RichNotificationData(), nullptr);
mojom::TraitsTestServicePtr proxy = GetTraitsTestProxy(); mojom::TraitsTestServicePtr proxy = GetTraitsTestProxy();
Notification output; Notification output;
proxy->EchoNotification(input, &output); proxy->EchoNotification(input, &output);
Compare(input, output); Compare(input, output);
// Set some optional fields to non-default values and test again.
input.set_type(NotificationType::NOTIFICATION_TYPE_PROGRESS);
input.set_progress(50);
input.set_progress_status(base::ASCIIToUTF16("progress text"));
input.set_clickable(!input.clickable());
input.set_accent_color(SK_ColorMAGENTA);
input.set_use_image_as_icon(!input.use_image_as_icon());
proxy->EchoNotification(input, &output);
Compare(input, output);
} }
TEST_F(StructTraitsTest, EmptyIconNotification) { TEST_F(StructTraitsTest, EmptyIconNotification) {
......
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