Further reduction of feature_compiler.py generated code

extensions/common/features/complex_feature.cc

 // Copyright 2014 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 "extensions/common/features/complex_feature.h"
 
 namespace extensions {
 
-ComplexFeature::ComplexFeature(std::unique_ptr<FeatureList> features) {
-  DCHECK_GT(features->size(), 0UL);
-  features_.swap(*features);
+ComplexFeature::ComplexFeature(const std::vector<Feature*>& features) {
+  DCHECK_GT(features.size(), 0UL);
+  for (Feature* f : features)
+    features_.push_back(std::unique_ptr<Feature>(f));

[Devlin, 2016/07/29 20:13:49]

What if we did something in the generated code like:
std::vector<std::unique_ptr<Feature>> features;
{
  SimpleFeature* feature = new SimpleFeature();
  ...
  features.push_back(std::unique_ptr<Feature>(f));
}
new ComplexFeature(std::move(features));

(or ComplexFeature could take a std::vector<unique_ptr<Feature>>* and we could
call swap, or some other variation.)  That should avoid the wrap/unwrap, because
we're only wrapping them when they are placed in the vector, right?

Overall this is probably okay, but it seems intuitively like it would be faster
if we didn't have to construct two vectors.

[scottmg, 2016/07/29 20:24:53]

Sorry, I'm probably missing something, but the point is to avoid having N calls
to unique_ptr::unique_ptr() in the generated code, rather than only 1 in
ComplexFeature, so I don't see how that would help.

[Devlin, 2016/07/29 21:15:02]

Ah, right, sorry.  I misunderstood that the concern was the amount of moving
that had to happen (i.e., create a unique_ptr, then move it into the container
vs just construct it directly in the container) rather than the number of calls
to the ctor at all.

   no_parent_ = features_[0]->no_parent();
 
 #if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
   // Verify IsInternal and no_parent is consistent across all features.
   bool first_is_internal = features_[0]->IsInternal();
   for (FeatureList::const_iterator it = features_.begin() + 1;
        it != features_.end();
        ++it) {
     DCHECK(first_is_internal == (*it)->IsInternal())
         << "Complex feature must have consistent values of "
@@ skipping 74 matching lines @@
   return false;
 }
 
 bool ComplexFeature::IsInternal() const {
   // Constructor verifies that composed features are consistent, thus we can
   // return just the first feature's value.
   return features_[0]->IsInternal();
 }
 
 }  // namespace extensions