Remove stl_util's deletion function use from ui/message_center/.

Remove stl_util's deletion function use from ui/message_center/.

BUG=555865
Committed: https://crrev.com/3c43055c51017c62c08c7abb86e98d79d0222a8d
Cr-Commit-Position: refs/heads/master@{#427102}

[Avi (use Gerrit), 2016-10-21 01:23:20]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[Avi (use Gerrit), 2016-10-21 01:23:58]

Description was changed from

==========
# Enter a description of the change.
Remove stl_util's deletion function use from ui/message_center/.

BUG=555865
==========

to

==========
Remove stl_util's deletion function use from ui/message_center/.

BUG=555865
==========

[Avi (use Gerrit), 2016-10-21 01:23:58]

avi@chromium.org changed reviewers:
+ stevenjb@chromium.org

[commit-bot: I haz the power, 2016-10-21 01:24:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 01:50:51]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 01:50:52]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Avi (use Gerrit), 2016-10-21 14:37:08]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 14:37:23]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 15:04:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 15:04:03]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Avi (use Gerrit), 2016-10-21 17:38:10]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 17:38:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-21 18:18:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-21 18:18:36]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[stevenjb, 2016-10-21 18:43:51]

lgtm

[Avi (use Gerrit), 2016-10-21 18:46:26]

On 2016/10/21 18:43:51, stevenjb wrote:
> lgtm

Oh. I'd tagged you because I was going to eventually send it to you, but was
waiting until I got it green. Let me get it green and I'll re-send it.

[stevenjb, 2016-10-21 18:50:39]

Oops, sorry, I was working through a backlog of reviews :)


On Fri, Oct 21, 2016 at 11:46 AM, <avi@chromium.org> wrote:

> Reviewers: stevenjb
> CL: https://codereview.chromium.org/2440443005/
>
> Message:
> On 2016/10/21 18:43:51, stevenjb wrote:
> > lgtm
>
> Oh. I'd tagged you because I was going to eventually send it to you, but
> was
> waiting until I got it green. Let me get it green and I'll re-send it.
>
> Description:
> Remove stl_util's deletion function use from ui/message_center/.
>
> BUG=555865
>
> Affected files (+45, -37 lines):
> M ui/message_center/views/message_center_view_unittest.cc
> M ui/message_center/views/message_list_view.cc
> M ui/message_center/views/notification_view.cc
>
>
> Index: ui/message_center/views/message_center_view_unittest.cc
> diff --git a/ui/message_center/views/message_center_view_unittest.cc
> b/ui/message_center/views/message_center_view_unittest.cc
> index 268d5bdcd9291d202cb54884e4bdad81e69d9e8a..
> 2aefb98da05474e7eb5c03ff7d519175d056d147 100644
> --- a/ui/message_center/views/message_center_view_unittest.cc
> +++ b/ui/message_center/views/message_center_view_unittest.cc
> @@ -10,6 +10,7 @@
>
> #include "base/logging.h"
> #include "base/macros.h"
> +#include "base/memory/ptr_util.h"
> #include "base/run_loop.h"
> #include "base/strings/utf_string_conversions.h"
> #include "testing/gtest/include/gtest/gtest.h"
> @@ -101,8 +102,11 @@ class FakeMessageCenterImpl : public
> FakeMessageCenter {
> const NotificationList::Notifications& GetVisibleNotifications() override
> {
> return visible_notifications_;
> }
> - void SetVisibleNotifications(NotificationList::Notifications
> notifications) {
> - visible_notifications_ = notifications;
> + void SetVisibleNotifications(
> + const NotificationList::OwnedNotifications& notifications) {
> + visible_notifications_.clear();
> + for (const auto& notification : notifications)
> + visible_notifications_.insert(notification.get());
> }
> void RemoveAllNotifications(bool by_user, RemoveType type) override {
> if (type == RemoveType::NON_PINNED)
> @@ -166,10 +170,12 @@ class MessageCenterViewTest : public
> views::ViewsTestBase,
>
> void RemoveDefaultNotifications();
>
> + void RemoveNotificationImpl(const std::string& notification_id);
> +
> private:
> views::View* MakeParent(views::View* child1, views::View* child2);
>
> - NotificationList::Notifications notifications_;
> + NotificationList::OwnedNotifications notifications_;
> std::unique_ptr<views::Widget> widget_;
> std::unique_ptr<MessageCenterView> message_center_view_;
> std::unique_ptr<FakeMessageCenterImpl> message_center_;
> @@ -205,8 +211,8 @@ void MessageCenterViewTest::SetUp() {
> message_center::RichNotificationData(), NULL);
>
> // ...and a list for it.
> - notifications_.insert(notification1);
> - notifications_.insert(notification2);
> + notifications_.insert(std::move(notification1));
> + notifications_.insert(std::move(notification2));
> message_center_->SetVisibleNotifications(notifications_);
>
> // Then create a new MessageCenterView with that single notification.
> @@ -237,7 +243,7 @@ void MessageCenterViewTest::TearDown() {
> widget_->CloseNow();
> widget_.reset();
> message_center_view_.reset();
> - base::STLDeleteElements(&notifications_);
> + notifications_.clear();
> views::ViewsTestBase::TearDown();
> }
>
> @@ -291,7 +297,7 @@ void MessageCenterViewTest::ClickOnNotification(
> void MessageCenterViewTest::AddNotification(
> std::unique_ptr<Notification> notification) {
> std::string notification_id = notification->id();
> - notifications_.insert(notification.release());
> + notifications_.insert(std::move(notification));
> message_center_->SetVisibleNotifications(notifications_);
> message_center_view_->OnNotificationAdded(notification_id);
> }
> @@ -299,16 +305,11 @@ void MessageCenterViewTest::AddNotification(
> void MessageCenterViewTest::UpdateNotification(
> const std::string& notification_id,
> std::unique_ptr<Notification> notification) {
> - for (auto it = notifications_.begin(); it != notifications_.end(); it++)
> {
> - if ((*it)->id() == notification_id) {
> - delete *it;
> - notifications_.erase(it);
> - break;
> - }
> - }
> + RemoveNotificationImpl(notification_id);
> +
> // |notifications| is a "set" container so we don't need to be aware the
> // order.
> - notifications_.insert(notification.release());
> + notifications_.insert(std::move(notification));
> message_center_->SetVisibleNotifications(notifications_);
> message_center_view_->OnNotificationUpdated(notification_id);
> }
> @@ -316,13 +317,8 @@ void MessageCenterViewTest::UpdateNotification(
> void MessageCenterViewTest::RemoveNotification(
> const std::string& notification_id,
> bool by_user) {
> - for (auto it = notifications_.begin(); it != notifications_.end(); it++)
> {
> - if ((*it)->id() == notification_id) {
> - delete *it;
> - notifications_.erase(it);
> - break;
> - }
> - }
> + RemoveNotificationImpl(notification_id);
> +
> message_center_->SetVisibleNotifications(notifications_);
> message_center_view_->OnNotificationRemoved(notification_id, by_user);
> }
> @@ -385,6 +381,17 @@ void MessageCenterViewTest::RemoveDefaultNotifications()
> {
> RemoveNotification(kNotificationId2, false);
> }
>
> +void MessageCenterViewTest::RemoveNotificationImpl(
> + const std::string& notification_id) {
> + auto it =
> + std::find_if(notifications_.begin(), notifications_.end,
> + [notification_id](const std::unique_ptr<Notification>& ptr) {
> + return ptr->id() == notification_id;
> + });
> + if (it != notifications_.end())
> + notifications_.erase(it);
> +}
> +
> /* Unit tests ************************************************************
> *****/
>
> TEST_F(MessageCenterViewTest, CallTest) {
> Index: ui/message_center/views/message_list_view.cc
> diff --git a/ui/message_center/views/message_list_view.cc
> b/ui/message_center/views/message_list_view.cc
> index f2efc22f410c2a1886f2078580446a430793d8d6..
> 4dc04a60b0917175cc6fe0cc5f415a8ad66a4a6a 100644
> --- a/ui/message_center/views/message_list_view.cc
> +++ b/ui/message_center/views/message_list_view.cc
> @@ -185,8 +185,8 @@ void MessageListView::ResetRepositionSession() {
> has_deferred_task_ = false;
> // cancel cause OnBoundsAnimatorDone which deletes |deleted_when_done_|.
> animator_.Cancel();
> - base::STLDeleteContainerPointers(deleting_views_.begin(),
> - deleting_views_.end());
> + for (auto view : deleting_views_)
> + delete view;
> deleting_views_.clear();
> adding_views_.clear();
> }
> @@ -218,17 +218,16 @@ void MessageListView::ClearAllClosableNotifications(
> void MessageListView::OnBoundsAnimatorProgressed(
> views::BoundsAnimator* animator) {
> DCHECK_EQ(&animator_, animator);
> - for (std::set<views::View*>::iterator iter = deleted_when_done_.begin();
> - iter != deleted_when_done_.end(); ++iter) {
> - const gfx::SlideAnimation* animation = animator->GetAnimationForView(
> *iter);
> + for (auto view : deleted_when_done_) {
> + const gfx::SlideAnimation* animation = animator->GetAnimationForView(
> view);
> if (animation)
> - (*iter)->layer()->SetOpacity(animation->CurrentValueBetween(1.0, 0.0));
> + view->layer()->SetOpacity(animation->CurrentValueBetween(1.0, 0.0));
> }
> }
>
> void MessageListView::OnBoundsAnimatorDone(views::BoundsAnimator*
> animator) {
> - base::STLDeleteContainerPointers(deleted_when_done_.begin(),
> - deleted_when_done_.end());
> + for (auto view : deleted_when_done_)
> + delete view;
> deleted_when_done_.clear();
>
> if (clear_all_started_) {
> @@ -379,7 +378,7 @@ void MessageListView::AnimateNotificationsAboveTarget()
> {
> }
>
> // If the calculated length is changed from |repositon_top_|, it means that
> - // some of items above the targe are updated and their height are
> changed.
> + // some of items above the target are updated and their height are
> changed.
> // Adjust the vertical length above the target.
> if (reposition_top_ != vertical_gap_to_target_from_top) {
> fixed_height_ -= reposition_top_ - vertical_gap_to_target_from_top;
> Index: ui/message_center/views/notification_view.cc
> diff --git a/ui/message_center/views/notification_view.cc
> b/ui/message_center/views/notification_view.cc
> index 6475098832d0b18cc9fc78bb026152c4d934c825..
> f58889727ee6331d12e31fe4d3ba722440b67791 100644
> --- a/ui/message_center/views/notification_view.cc
> +++ b/ui/message_center/views/notification_view.cc
> @@ -8,7 +8,6 @@
>
> #include "base/command_line.h"
> #include "base/macros.h"
> -#include "base/stl_util.h"
> #include "base/strings/string_util.h"
> #include "build/build_config.h"
> #include "components/url_formatter/elide_url.h"
> @@ -509,8 +508,8 @@ void NotificationView::CreateOrUpdateProgressBarView(
>
> void NotificationView::CreateOrUpdateListItemViews(
> const Notification& notification) {
> - for (size_t i = 0; i < item_views_.size(); ++i)
> - delete item_views_[i];
> + for (auto item_view : item_views_)
> + delete item_view;
> item_views_.clear();
>
> int padding = kMessageLineHeight - views::Label().font_list().GetHeight();
> @@ -590,9 +589,12 @@ void NotificationView::CreateOrUpdateActionButtonView
> s(
> bool new_buttons = action_buttons_.size() != buttons.size();
>
> if (new_buttons || buttons.size() == 0) {
> - // STLDeleteElements also clears the container.
> - base::STLDeleteElements(&separators_);
> - base::STLDeleteElements(&action_buttons_);
> + for (auto item : separators_)
> + delete item;
> + separators_.clear();
> + for (auto item : action_buttons_)
> + delete item;
> + action_buttons_.clear();
> }
>
> DCHECK(bottom_view_);
>
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Avi (use Gerrit), 2016-10-21 23:31:48]

The CQ bit was checked by avi@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-10-21 23:33:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-22 01:38:47]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-22 01:38:48]

Dry run: This issue passed the CQ dry run.

[Avi (use Gerrit), 2016-10-22 01:47:52]

It's green now if you still approve.

[stevenjb, 2016-10-24 17:37:29]

still lgtm

[Avi (use Gerrit), 2016-10-24 17:40:52]

The CQ bit was checked by avi@chromium.org

[commit-bot: I haz the power, 2016-10-24 17:41:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-24 18:40:55]

Description was changed from

==========
Remove stl_util's deletion function use from ui/message_center/.

BUG=555865
==========

to

==========
Remove stl_util's deletion function use from ui/message_center/.

BUG=555865
==========

[commit-bot: I haz the power, 2016-10-24 18:40:56]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-10-24 18:54:08]

Description was changed from

==========
Remove stl_util's deletion function use from ui/message_center/.

BUG=555865
==========

to

==========
Remove stl_util's deletion function use from ui/message_center/.

BUG=555865

Committed: https://crrev.com/3c43055c51017c62c08c7abb86e98d79d0222a8d
Cr-Commit-Position: refs/heads/master@{#427102}
==========

[commit-bot: I haz the power, 2016-10-24 18:54:09]

Patchset 4 (id:??) landed as
https://crrev.com/3c43055c51017c62c08c7abb86e98d79d0222a8d
Cr-Commit-Position: refs/heads/master@{#427102}