[NTP::PhysicalWeb] Filter our pages with same group_id.

components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.cc

 // Copyright 2016 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 "components/ntp_snippets/physical_web_pages/physical_web_page_suggestions_provider.h"
 
 #include <algorithm>
 #include <memory>
 #include <set>
 #include <string>
@@ skipping 19 matching lines @@
 using base::DictionaryValue;
 using base::ListValue;
 using base::Value;
 
 namespace ntp_snippets {
 
 namespace {
 
 const size_t kMaxSuggestionsCount = 10;
 
+std::string GetGroupId(const DictionaryValue& page_dictionary) {
+  std::string group_id;
+  if (!page_dictionary.GetString(physical_web::kGroupIdKey, &group_id)) {
+    LOG(DFATAL) << physical_web::kGroupIdKey << " field is missing.";
+  }
+  return group_id;
+}
+
 std::string GetPageId(const DictionaryValue& page_dictionary) {
   std::string raw_resolved_url;
   if (!page_dictionary.GetString(physical_web::kResolvedUrlKey,
                                  &raw_resolved_url)) {
     LOG(DFATAL) << physical_web::kResolvedUrlKey << " field is missing.";
   }
   return raw_resolved_url;
 }
 
+bool CompareByDistance(const DictionaryValue* left,
+                       const DictionaryValue* right) {
+  double left_distance, right_distance;
+  bool success =
+      left->GetDouble(physical_web::kDistanceEstimateKey, &left_distance);
+  success =
+      right->GetDouble(physical_web::kDistanceEstimateKey, &right_distance) &&
+      success;
+  if (!success) {
+    LOG(DFATAL) << "Distance field is missing.";
+  }
+
+  // When there is no estimate, the value is <= 0, so we implicilty treat it as

[Marc Treib, 2017/01/18 11:54:48]

s/implicilty/implicitly/

[vitaliii, 2017/01/18 14:02:27]

Done.

+  // infinity.
+  bool is_left_estimated = left_distance > 0;
+  bool is_right_estimated = right_distance > 0;
+
+  if (is_left_estimated != is_right_estimated)
+    return is_left_estimated;
+  return left_distance < right_distance;
+}
+
+void FilterOutByGroupId(
+    std::vector<const DictionaryValue*>* page_dictionaries) {
+  // |std::unique| only removes duplicates that immediately follow each other.
+  // Thus, first, we have to sort by group_id and distance and only then remove
+  // duplicates.
+  std::sort(page_dictionaries->begin(), page_dictionaries->end(),
+            [](const DictionaryValue* left, const DictionaryValue* right) {
+              std::string left_group_id = GetGroupId(*left),
+                          right_group_id = GetGroupId(*right);

[Marc Treib, 2017/01/18 11:54:48]

nitty nit: I'd just make two declarations

[vitaliii, 2017/01/18 14:02:27]

Done.

+
+              if (left_group_id != right_group_id) {
+                return left_group_id < right_group_id;
+              }
+
+              // We want closest pages first, so in case of same group_id we
+              // sort by distance.
+              return CompareByDistance(left, right);
+            });
+
+  // Each empty group_id must be treated as unique, so we do not apply
+  // std::unique to them at all.
+  auto nonempty_group_id_begin = std::find_if(
+      page_dictionaries->begin(), page_dictionaries->end(),
+      [](const DictionaryValue* page) { return !GetGroupId(*page).empty(); });
+
+  auto new_end = std::unique(
+      nonempty_group_id_begin, page_dictionaries->end(),
+      [](const DictionaryValue* left, const DictionaryValue* right) {
+        return GetGroupId(*left) == GetGroupId(*right);
+      });
+
+  page_dictionaries->erase(new_end, page_dictionaries->end());
+}
+
 }  // namespace
 
 PhysicalWebPageSuggestionsProvider::PhysicalWebPageSuggestionsProvider(
     ContentSuggestionsProvider::Observer* observer,
     physical_web::PhysicalWebDataSource* physical_web_data_source,
     PrefService* pref_service)
     : ContentSuggestionsProvider(observer),
       category_status_(CategoryStatus::AVAILABLE),
       provided_category_(
           Category::FromKnownCategory(KnownCategories::PHYSICAL_WEB_PAGES)),
@@ skipping 156 matching lines @@
 
     if (old_dismissed_ids.count(page_id)) {
       new_dismissed_ids.insert(page_id);
     }
   }
 
   if (old_dismissed_ids.size() != new_dismissed_ids.size()) {
     StoreDismissedIDsToPrefs(new_dismissed_ids);
   }
 
+  FilterOutByGroupId(&page_dictionaries);
+
   std::sort(page_dictionaries.begin(), page_dictionaries.end(),
-            [](const DictionaryValue* left, const DictionaryValue* right) {
-              double left_distance, right_distance;
-              bool success = left->GetDouble(physical_web::kDistanceEstimateKey,
-                                             &left_distance);
-              success = right->GetDouble(physical_web::kDistanceEstimateKey,
-                                         &right_distance) &&
-                        success;
-              if (!success) {
-                LOG(DFATAL) << "Distance field is missing.";
-              }
-              return left_distance < right_distance;
-            });
+            CompareByDistance);
 
   std::vector<ContentSuggestion> suggestions;
   for (const DictionaryValue* page_dictionary : page_dictionaries) {
     if (static_cast<int>(suggestions.size()) == max_quantity) {
       break;
     }
     suggestions.push_back(ConvertPhysicalWebPage(*page_dictionary));
   }
 
   return suggestions;
@@ skipping 59 matching lines @@
 }
 
 void PhysicalWebPageSuggestionsProvider::StoreDismissedIDsToPrefs(
     const std::set<std::string>& dismissed_ids) {
   prefs::StoreDismissedIDsToPrefs(pref_service_,
                                   prefs::kDismissedPhysicalWebPageSuggestions,
                                   dismissed_ids);
 }
 
 }  // namespace ntp_snippets