Make all LayerAnimationElement::Create*Element return unique_ptr

ash/wm/session_state_animator_impl.cc

 // Copyright (c) 2012 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 "ash/wm/session_state_animator_impl.h"
 
 #include <vector>
 
 #include "ash/common/wm/wm_window_animations.h"
 #include "ash/public/cpp/shell_window_ids.h"
@@ skipping 236 matching lines @@
 // Starts grayscale/brightness animation for |window| over |duration|. Target
 // value for both grayscale and brightness are specified by |target|.
 void StartGrayscaleBrightnessAnimationForWindow(
     aura::Window* window,
     float target,
     base::TimeDelta duration,
     gfx::Tween::Type tween_type,
     ui::LayerAnimationObserver* observer) {
   ui::LayerAnimator* animator = window->layer()->GetAnimator();
 
-  std::unique_ptr<ui::LayerAnimationSequence> brightness_sequence(
-      new ui::LayerAnimationSequence());
-  std::unique_ptr<ui::LayerAnimationSequence> grayscale_sequence(
-      new ui::LayerAnimationSequence());
+  ui::LayerAnimationSequence* brightness_sequence =

[loyso (OOO), 2016/12/06 04:21:56]

Use auto and MakeUnique and std::move here and everywhere, please.

[Sunny, 2016/12/06 06:21:05]

Hi, the variable "brightness_sequence" here is only used for add one element and
then been added to std::vector<ui::LayerAnimationSequence*>, since the logic
here is simple and there is RAW pointer in vector, is it worthwhile to use
std::unique to wrap each raw pointer?

Also I've search across the chromium codebase, seems no one use
std::make_unique<>(), and it's a C++14 feature, shall we use it?

Thanks!

[loyso (OOO), 2016/12/06 06:32:35]

Thanks for working on this, Sunny!

On 2016/12/06 at 06:21:05, Sunny wrote:
Yes, the type checking should be pervasive and total. It doesn't work otherwise.
https://google.github.io/styleguide/cppguide.html#Ownership_and_Smart_Pointers
Discussion:
https://groups.google.com/a/chromium.org/forum/#!topic/cxx/aT2wsBLKvzI
Search for base::MakeUnique instead. See this discussion if interested:
https://groups.google.com/a/chromium.org/d/msg/chromium-dev/iQgMedVA8-k/u7HSa...

+      new ui::LayerAnimationSequence();
+  ui::LayerAnimationSequence* grayscale_sequence =
+      new ui::LayerAnimationSequence();
 
-  std::unique_ptr<ui::LayerAnimationElement> brightness_element(
-      ui::LayerAnimationElement::CreateBrightnessElement(target, duration));
+  ui::LayerAnimationElement* brightness_element =
+      ui::LayerAnimationElement::CreateBrightnessElement(target, duration);

[loyso (OOO), 2016/12/06 04:25:17]

It's better for Create*Element methods to use move semantics as well.

[Sunny, 2016/12/06 06:21:05]

Same question, is it worthwhile to wrap raw pointer with std::unique or are
there any guide in chromium team advise to use unique_ptr instead of raw pointer
everywhere?

Thanks

   brightness_element->set_tween_type(tween_type);
-  brightness_sequence->AddElement(brightness_element.release());
+  brightness_sequence->AddElement(brightness_element);
 
-  std::unique_ptr<ui::LayerAnimationElement> grayscale_element(
-      ui::LayerAnimationElement::CreateGrayscaleElement(target, duration));
+  ui::LayerAnimationElement* grayscale_element =
+      ui::LayerAnimationElement::CreateGrayscaleElement(target, duration);
   grayscale_element->set_tween_type(tween_type);
-  grayscale_sequence->AddElement(grayscale_element.release());
+  grayscale_sequence->AddElement(grayscale_element);
 
   std::vector<ui::LayerAnimationSequence*> animations;
-  animations.push_back(brightness_sequence.release());
-  animations.push_back(grayscale_sequence.release());
+  animations.push_back(brightness_sequence);
+  animations.push_back(grayscale_sequence);
 
   if (observer)
     animations[0]->AddObserver(observer);
 
   animator->set_preemption_strategy(
       ui::LayerAnimator::IMMEDIATELY_ANIMATE_TO_NEW_TARGET);
 
   animator->StartTogether(animations);
 }
 
@@ skipping 366 matching lines @@
                                                  gfx::Tween::EASE_IN, observer);
       break;
     case ANIMATION_UNDO_GRAYSCALE_BRIGHTNESS:
       StartGrayscaleBrightnessAnimationForWindow(
           window, 0.0, duration, gfx::Tween::EASE_IN_OUT, observer);
       break;
   }
 }
 
 }  // namespace ash