Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(48)

Issue 2496423004: arc: enable use_new_wrapper_types for notifications.mojom (Closed)

Created:
4 years, 1 month ago by Yusuke Sato
Modified:
4 years, 1 month ago
CC:
chromium-reviews, elijahtaylor+arcwatch_chromium.org, hidehiko+watch_chromium.org, lhchavez+watch_chromium.org, yusukes+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

arc: enable use_new_wrapper_types for notifications.mojom BUG=662510 BUG=624136 TEST=try Committed: https://crrev.com/4554b46f099cc837d4f5aa574a67541eccc0f150 Cr-Commit-Position: refs/heads/master@{#432313}

Patch Set 1 #

Patch Set 2 : fix ui_arc_unittests #

Total comments: 5

Patch Set 3 : address comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+41 lines, -36 lines) Patch
M components/arc/BUILD.gn View 2 chunks +1 line, -1 line 0 comments Download
M components/arc/test/fake_notifications_instance.h View 2 chunks +6 lines, -5 lines 0 comments Download
M components/arc/test/fake_notifications_instance.cc View 1 chunk +4 lines, -4 lines 0 comments Download
M ui/arc/notification/arc_custom_notification_item.cc View 2 chunks +8 lines, -6 lines 0 comments Download
M ui/arc/notification/arc_notification_item.cc View 5 chunks +12 lines, -14 lines 0 comments Download
M ui/arc/notification/arc_notification_manager.h View 1 chunk +1 line, -1 line 0 comments Download
M ui/arc/notification/arc_notification_manager.cc View 1 2 3 chunks +8 lines, -4 lines 0 comments Download
M ui/arc/notification/arc_notification_manager_unittest.cc View 1 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 36 (22 generated)
Yusuke Sato
I have 3 migration CLs and this is the first one. Luis: PTAL. Yoshiki: FYI. ...
4 years, 1 month ago (2016-11-15 19:19:17 UTC) #11
Luis Héctor Chávez
lgtm https://codereview.chromium.org/2496423004/diff/20001/ui/arc/notification/arc_notification_manager.cc File ui/arc/notification/arc_notification_manager.cc (right): https://codereview.chromium.org/2496423004/diff/20001/ui/arc/notification/arc_notification_manager.cc#newcode241 ui/arc/notification/arc_notification_manager.cc:241: base::UTF8ToUTF16(data->text.has_value() ? *data->text : "")); nit: s/""/std::string()/
4 years, 1 month ago (2016-11-15 19:22:59 UTC) #12
Yusuke Sato
https://codereview.chromium.org/2496423004/diff/20001/ui/arc/notification/arc_notification_manager.cc File ui/arc/notification/arc_notification_manager.cc (right): https://codereview.chromium.org/2496423004/diff/20001/ui/arc/notification/arc_notification_manager.cc#newcode241 ui/arc/notification/arc_notification_manager.cc:241: base::UTF8ToUTF16(data->text.has_value() ? *data->text : "")); On 2016/11/15 19:22:59, Luis ...
4 years, 1 month ago (2016-11-15 19:27:35 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2496423004/40001
4 years, 1 month ago (2016-11-15 19:30:19 UTC) #16
commit-bot: I haz the power
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linux/builds/260849)
4 years, 1 month ago (2016-11-15 19:47:46 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2496423004/40001
4 years, 1 month ago (2016-11-15 20:29:56 UTC) #20
commit-bot: I haz the power
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linux/builds/260954)
4 years, 1 month ago (2016-11-15 20:45:18 UTC) #22
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2496423004/40001
4 years, 1 month ago (2016-11-15 21:32:19 UTC) #24
commit-bot: I haz the power
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linux/builds/261076)
4 years, 1 month ago (2016-11-15 21:44:43 UTC) #26
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2496423004/40001
4 years, 1 month ago (2016-11-15 22:04:51 UTC) #28
commit-bot: I haz the power
Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linux/builds/261189)
4 years, 1 month ago (2016-11-15 22:33:11 UTC) #30
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2496423004/40001
4 years, 1 month ago (2016-11-15 23:41:36 UTC) #32
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 1 month ago (2016-11-16 00:26:34 UTC) #34
commit-bot: I haz the power
4 years, 1 month ago (2016-11-16 00:31:33 UTC) #36
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/4554b46f099cc837d4f5aa574a67541eccc0f150
Cr-Commit-Position: refs/heads/master@{#432313}

Powered by Google App Engine
This is Rietveld 408576698