Ensure nearby URL count metric is properly initialized

components/omnibox/browser/physical_web_provider.cc

Index: components/omnibox/browser/physical_web_provider.cc
diff --git a/components/omnibox/browser/physical_web_provider.cc b/components/omnibox/browser/physical_web_provider.cc
index 15ba262ab3d8c5e8acbdcc15a9a85de3dc69ab47..50b4767aaf3c95d238baa51caac5a9b592d496c1 100644
--- a/components/omnibox/browser/physical_web_provider.cc
+++ b/components/omnibox/browser/physical_web_provider.cc
@@ -78,8 +78,11 @@ void PhysicalWebProvider::Start(const AutocompleteInput& input,
   const bool input_from_focus = input.from_omnibox_focus();
   const bool empty_input_from_user = !input_from_focus && input.text().empty();
 
+  auto metadata_list = data_source->GetMetadataList();
+  nearby_url_count_ = metadata_list->size();
+
   if (input_from_focus || empty_input_from_user) {
-    ConstructZeroSuggestMatches(data_source->GetMetadataList());
+    ConstructZeroSuggestMatches(std::move(metadata_list));

[Mark P, 2017/02/11 05:17:39]

I don't understand this move operation.  Then again, I haven't thought much
about our smart pointer types in a while.  Won't this move mean you clean up the
pointer at the end of this call?  Why is that what you want?

[mattreynolds, 2017/02/13 19:00:04]

This preserves the same behavior as before. In the previous version,
GetMetadataList returns a prvalue so it didn't need std::move. Now that it has
identity, it is an xvalue and needs std::move.

Essentially, I'm choosing to be explicit that the metadata list will not be
accessed in PhysicalWebProvider::Start after we pass it to one of the
Construct.*Matches methods. As-is, the metadata list should be released when
Construct.*Matches returns. We could choose to pass a const reference instead
and then it would be cleaned up when PhysicalWebProvider::Start returns.

[Mark P, 2017/02/15 06:45:53]

Ah, okay.  Thanks for the explanation.

 
     if (!matches_.empty()) {
       had_physical_web_suggestions_ = true;
@@ -105,7 +108,7 @@ void PhysicalWebProvider::Start(const AutocompleteInput& input,
           client_, input, input.current_url(), history_url_provider_, -1));
     }
   } else {
-    ConstructQuerySuggestMatches(data_source->GetMetadataList(), input);
+    ConstructQuerySuggestMatches(std::move(metadata_list), input);
 
     if (!matches_.empty()) {
       had_physical_web_suggestions_ = true;
@@ -158,6 +161,7 @@ PhysicalWebProvider::PhysicalWebProvider(
     : AutocompleteProvider(AutocompleteProvider::TYPE_PHYSICAL_WEB),
       client_(client),
       history_url_provider_(history_url_provider),
+      nearby_url_count_(0),
       zero_suggest_enabled_(
           OmniboxFieldTrial::InPhysicalWebZeroSuggestFieldTrial()),
       after_typing_enabled_(
@@ -172,10 +176,10 @@ PhysicalWebProvider::~PhysicalWebProvider() {
 
 void PhysicalWebProvider::ConstructZeroSuggestMatches(
     std::unique_ptr<physical_web::MetadataList> metadata_list) {
-  nearby_url_count_ = metadata_list->size();
+  size_t nearby_url_count = metadata_list->size();
   size_t used_slots = 0;
 
-  for (size_t i = 0; i < nearby_url_count_; ++i) {
+  for (size_t i = 0; i < nearby_url_count; ++i) {
     const auto& metadata_item = (*metadata_list)[i];
     std::string url_string = metadata_item.resolved_url.spec();
     std::string title_string = metadata_item.title;
@@ -189,7 +193,7 @@ void PhysicalWebProvider::ConstructZeroSuggestMatches(
     // Append an overflow item if creating a match for each metadata item would
     // exceed the match limit.
     const size_t remaining_slots = kPhysicalWebMaxMatches - used_slots;
-    const size_t remaining_metadata = nearby_url_count_ - i;
+    const size_t remaining_metadata = nearby_url_count - i;
     if ((remaining_slots == 1) && (remaining_metadata > remaining_slots)) {
       AppendOverflowItem(remaining_metadata, relevance, title);
       break;