Make all LayerAnimationElement::Create*Element return unique_ptr

ash/wm/session_state_animator_impl.cc

Index: ash/wm/session_state_animator_impl.cc
diff --git a/ash/wm/session_state_animator_impl.cc b/ash/wm/session_state_animator_impl.cc
index 783a2ba8c2f168c8a3b9879268042483943b2477..2224e26f37ec6c7c15a0e396651a79066dace789 100644
--- a/ash/wm/session_state_animator_impl.cc
+++ b/ash/wm/session_state_animator_impl.cc
@@ -254,24 +254,24 @@ void StartGrayscaleBrightnessAnimationForWindow(
     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);