Remove stl_util's deletion function use from components/signin/.

components/signin/core/browser/about_signin_internals.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 "components/signin/core/browser/about_signin_internals.h"
 
 #include <stddef.h>
 
+#include <tuple>
 #include <utility>
 
 #include "base/command_line.h"
 #include "base/hash.h"
 #include "base/i18n/time_formatting.h"
 #include "base/logging.h"
+#include "base/memory/ptr_util.h"
 #include "base/profiler/scoped_tracker.h"
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/trace_event/trace_event.h"
 #include "build/build_config.h"
 #include "components/pref_registry/pref_registry_syncable.h"
 #include "components/prefs/pref_service.h"
 #include "components/signin/core/browser/account_tracker_service.h"
 #include "components/signin/core/browser/profile_oauth2_token_service.h"
 #include "components/signin/core/browser/signin_client.h"
@@ skipping 274 matching lines @@
   // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is
   // fixed.
   tracked_objects::ScopedTracker tracking_profile(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422460 AboutSigninInternals::OnAccessTokenRequested"));
 
   TokenInfo* token = signin_status_.FindToken(account_id, consumer_id, scopes);
   if (token) {
     *token = TokenInfo(consumer_id, scopes);
   } else {
-    token = new TokenInfo(consumer_id, scopes);
-    signin_status_.token_info_map[account_id].push_back(token);
+    signin_status_.token_info_map[account_id].push_back(
+        base::MakeUnique<TokenInfo>(consumer_id, scopes));
   }
 
   NotifyObservers();
 }
 
 void AboutSigninInternals::OnFetchAccessTokenComplete(
     const std::string& account_id,
     const std::string& consumer_id,
     const OAuth2TokenService::ScopeSet& scopes,
     GoogleServiceAuthError error,
     base::Time expiration_time) {
   TokenInfo* token = signin_status_.FindToken(account_id, consumer_id, scopes);
   if (!token) {
     DVLOG(1) << "Can't find token: " << account_id << ", " << consumer_id;
     return;
   }
 
   token->receive_time = base::Time::Now();
   token->error = error;
   token->expiration_time = expiration_time;
 
   NotifyObservers();
 }
 
 void AboutSigninInternals::OnTokenRemoved(
     const std::string& account_id,
     const OAuth2TokenService::ScopeSet& scopes) {
-  for (size_t i = 0; i < signin_status_.token_info_map[account_id].size();
-       ++i) {
-    TokenInfo* token = signin_status_.token_info_map[account_id][i];
+  for (const auto& token : signin_status_.token_info_map[account_id]) {

[Roger Tawa OOO till Jul 10th, 2016/11/08 15:40:34]

For my own info, is there a danger of doing:

  for (const auto token : signin_status_.token_info_map[account_id])

or:

  for (auto token : signin_status_.token_info_map[account_id])

and having the map modified during iteration?

Is this possibly a case where auto hides too much info?  Maybe these loops
should be written:

  for (const std::unique_ptr<TokenInfo>& token :
           signin_status_.token_info_map[account_id]) {
  }

[Avi (use Gerrit), 2016/11/08 18:50:51]

You can't do either of those. auto would deduce the type as unique_ptr<> which
is non-copyable, so it would fail to compile as it tried to make a copy of each
of the unique_ptrs.
That would be clearer. I'll do that.

     if (token->scopes == scopes)
       token->Invalidate();
   }
   NotifyObservers();
 }
 
 void AboutSigninInternals::OnRefreshTokenReceived(const std::string& status) {
   NotifySigninValueChanged(REFRESH_TOKEN_RECEIVED, status);
 }
 
@@ skipping 56 matching lines @@
     const std::string& consumer_id,
     const OAuth2TokenService::ScopeSet& scopes)
     : consumer_id(consumer_id),
       scopes(scopes),
       request_time(base::Time::Now()),
       error(GoogleServiceAuthError::AuthErrorNone()),
       removed_(false) {}
 
 AboutSigninInternals::TokenInfo::~TokenInfo() {}
 
-bool AboutSigninInternals::TokenInfo::LessThan(const TokenInfo* a,
-                                               const TokenInfo* b) {
-  if (a->request_time == b->request_time) {
-    if (a->consumer_id == b->consumer_id) {
-      return a->scopes < b->scopes;
-    }
-    return a->consumer_id < b->consumer_id;
-  }
-  return a->request_time < b->request_time;
+bool AboutSigninInternals::TokenInfo::LessThan(
+    const std::unique_ptr<TokenInfo>& a,
+    const std::unique_ptr<TokenInfo>& b) {
+  return std::tie(a->request_time, a->consumer_id, a->scopes) <
+         std::tie(b->request_time, b->consumer_id, b->scopes);

[Roger Tawa OOO till Jul 10th, 2016/11/08 15:40:34]

That's neat :-)

[Avi (use Gerrit), 2016/11/08 18:50:51]

Indeed it is :)

 }
 
 void AboutSigninInternals::TokenInfo::Invalidate() { removed_ = true; }
 
 std::unique_ptr<base::DictionaryValue>
 AboutSigninInternals::TokenInfo::ToValue() const {
   std::unique_ptr<base::DictionaryValue> token_info(
       new base::DictionaryValue());
   token_info->SetString("service", consumer_id);
 
@@ skipping 33 matching lines @@
   } else {
     token_info->SetString("status", "Waiting for response");
   }
 
   return token_info;
 }
 
 AboutSigninInternals::SigninStatus::SigninStatus()
     : timed_signin_fields(TIMED_FIELDS_COUNT) {}
 
-AboutSigninInternals::SigninStatus::~SigninStatus() {
-  for (TokenInfoMap::iterator it = token_info_map.begin();
-       it != token_info_map.end();
-       ++it) {
-    base::STLDeleteElements(&it->second);
-  }
-}
+AboutSigninInternals::SigninStatus::~SigninStatus() {}
 
 AboutSigninInternals::TokenInfo* AboutSigninInternals::SigninStatus::FindToken(
     const std::string& account_id,
     const std::string& consumer_id,
     const OAuth2TokenService::ScopeSet& scopes) {
-  for (size_t i = 0; i < token_info_map[account_id].size(); ++i) {
-    TokenInfo* tmp = token_info_map[account_id][i];
-    if (tmp->consumer_id == consumer_id && tmp->scopes == scopes)
-      return tmp;
+  for (const auto& token : token_info_map[account_id]) {
+    if (token->consumer_id == consumer_id && token->scopes == scopes)
+      return token.get();
   }
-  return NULL;
+  return nullptr;
 }
 
 std::unique_ptr<base::DictionaryValue>
 AboutSigninInternals::SigninStatus::ToValue(
     AccountTrackerService* account_tracker,
     SigninManagerBase* signin_manager,
     SigninErrorController* signin_error_controller,
     ProfileOAuth2TokenService* token_service,
     GaiaCookieManagerService* cookie_manager_service_,
     const std::string& product_version) {
@@ skipping 111 matching lines @@
 
   // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460 is
   // fixed.
   tracked_objects::ScopedTracker tracking_profile4(
       FROM_HERE_WITH_EXPLICIT_FUNCTION(
           "422460 AboutSigninInternals::SigninStatus::ToValue4"));
 
   // Token information for all services.
   base::ListValue* token_info = new base::ListValue();
   signin_status->Set("token_info", token_info);
-  for (TokenInfoMap::iterator it = token_info_map.begin();
-       it != token_info_map.end();
-       ++it) {
+  for (auto it = token_info_map.begin(); it != token_info_map.end(); ++it) {
     // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460
     // is fixed.
     tracked_objects::ScopedTracker tracking_profile41(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "422460 AboutSigninInternals::SigninStatus::ToValue41"));
 
     base::ListValue* token_details = AddSection(token_info, it->first);
 
     // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460
     // is fixed.
     tracked_objects::ScopedTracker tracking_profile42(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "422460 AboutSigninInternals::SigninStatus::ToValue42"));
 
     std::sort(it->second.begin(), it->second.end(), TokenInfo::LessThan);
-    const std::vector<TokenInfo*>& tokens = it->second;
+    const auto& tokens = it->second;
 
     // TODO(robliao): Remove ScopedTracker below once https://crbug.com/422460
     // is fixed.
     tracked_objects::ScopedTracker tracking_profile43(
         FROM_HERE_WITH_EXPLICIT_FUNCTION(
             "422460 AboutSigninInternals::SigninStatus::ToValue43"));
 
-    for (size_t i = 0; i < tokens.size(); ++i) {
-      token_details->Append(tokens[i]->ToValue());
+    for (const auto& token : tokens) {
+      token_details->Append(token->ToValue());
     }
   }
 
   base::ListValue* account_info = new base::ListValue();
   signin_status->Set("accountInfo", account_info);
   const std::vector<std::string>& accounts_in_token_service =
       token_service->GetAccounts();
 
   if(accounts_in_token_service.size() == 0) {
     std::unique_ptr<base::DictionaryValue> no_token_entry(
         new base::DictionaryValue());
     no_token_entry->SetString("accountId", "No token in Token Service.");
     account_info->Append(std::move(no_token_entry));
   }
 
   for(const std::string& account_id : accounts_in_token_service) {
     std::unique_ptr<base::DictionaryValue> entry(new base::DictionaryValue());
     entry->SetString("accountId", account_id);
     account_info->Append(std::move(entry));
   }
 
   return signin_status;
 }