Build a new TokenCacheService so I can stop using TokenService for something it wasn't designed for.

chrome/browser/extensions/token_cache/token_cache.cc

+// Copyright (c) 2013 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 "chrome/browser/extensions/token_cache/token_cache.h"
+
+#include "base/logging.h"
+#include "chrome/common/chrome_notification_types.h"
+
+using base::Time;
+using base::TimeDelta;
+
+namespace extensions {
+
+TokenCacheService::TokenCacheService(Profile* profile) : profile_(profile) {
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_GOOGLE_SIGNED_OUT,
+                 content::Source<Profile>(profile_));
+}
+
+TokenCacheService::~TokenCacheService() {
+}
+
+void TokenCacheService::StoreToken(const std::string& token_name,
+                                   const std::string& token_value,
+                                   base::TimeDelta time_to_live) {
+  TokenCacheData token_data;
+
+  // Get the current time, and make sure that the token has not already expired.
+  Time expiration_time;
+  TimeDelta zero_delta;
+
+  // Negative time deltas are meaningless to this function.
+  DCHECK(time_to_live >= zero_delta);
+
+  if (zero_delta < time_to_live) {
+    expiration_time = Time::Now();
+    expiration_time += time_to_live;
+  }
+
+  token_data.token = token_value;
+  token_data.expiration_time = expiration_time;
+
+  // Find if the token exists, and if so replace it.
+  std::map<std::string, TokenCacheData>::iterator it = FindMatch(token_name);

[dcheng, 2013/03/05 22:38:44]

Why do we need to use FindMatch() for this? Isn't it the same thing either way?

[Pete Williamson, 2013/03/06 00:09:40]

FindMatch has the desirable side effect of lazily removing timed out entries. 
Let me know if you can think of a better name which denotes this.

[dcheng, 2013/03/06 00:26:35]

Even if you remove it, aren't you going to immediately populate it again? I'm
not seeing what calling FindMatch() accomplishes here.

[Pete Williamson, 2013/03/06 00:50:05]

No, we don't always populate it again.  In the retrieve token case, we need to
make sure that the token is deleted, and we are not going to reset it
immediately here (though the calling code may decide to get a new token and
reset it).

[dcheng, 2013/03/06 02:30:25]

I still don't understand. I'm talking specifically about StoreToken(). What is
the point of calling FindMatch() in StoreToken()? Either way, we end up calling
token_cache_[token_name] and don't use the returned iterator in any way. Why
can't lines 44 to 50 simply be removed?

+  if (it != token_cache_.end()) {
+    token_cache_[token_name] = token_data;
+    return;
+  }
+  // Otherwise, add the token to our cache.
+  token_cache_[token_name] = token_data;
+}
+
+// Retrieve a token for the currently logged in user.  This returns an empty
+// string if the token was not found or timed out.
+std::string TokenCacheService::RetrieveToken(const std::string& token_name) {
+  std::map<std::string, TokenCacheData>::iterator it = FindMatch(token_name);
+
+  if (it != token_cache_.end()) {
+    return it->second.token;
+  }
+
+  return std::string();
+}
+
+// Inherited from ProfileKeyedService.
+void TokenCacheService::Shutdown() {

[dcheng, 2013/03/05 22:38:44]

You can remove this, since it simply matches the default implementation now.

[Pete Williamson, 2013/03/06 00:09:40]

Done.

+  // Nothing to do.
+}
+
+// Inherited from NotificationObserver.
+void TokenCacheService::Observe(int type,
+                                const content::NotificationSource& source,
+                                const content::NotificationDetails& details)  {
+  if (type == chrome::NOTIFICATION_GOOGLE_SIGNED_OUT)
+    token_cache_.clear();
+}
+
+std::map<std::string, TokenCacheData>::iterator TokenCacheService::FindMatch(

[dcheng, 2013/03/05 22:38:44]

Just move this logic into RetrieveToken, since this logic isn't needed in
StoreToken anyway.

[Pete Williamson, 2013/03/06 00:09:40]

I use this from the unit test also, so I'd like to leave it here.

[dcheng, 2013/03/06 00:26:35]

Looking at the unit test, it seems like it'd be better to use RetrieveToken() in
all instances, since that's the logic we actually want to test.

[Pete Williamson, 2013/03/06 00:50:05]

The advantage of FindMatch in the unit test is that it can return the actual
item in the test instead of just the string.  RetrieveToken returns a null
string in both the case where there is a null string stored as the token and no
token is found.  Granted, my code ended up not needing to know the difference,
and I don't feel strongly about it, so I changed the unit test as you suggest.

+    const std::string& token_name) {
+

[dcheng, 2013/03/05 22:38:44]

Extra newline.

[Pete Williamson, 2013/03/06 00:09:40]

Done.

+  std::map<std::string, TokenCacheData>::iterator it;
+
+  it = token_cache_.find(token_name);

[dcheng, 2013/03/05 22:38:44]

Combine declaration and initialization.

[Pete Williamson, 2013/03/06 00:09:40]

Done.

+
+  if (it != token_cache_.end()) {
+    Time now = Time::Now();
+    Time zero;

[dcheng, 2013/03/05 22:38:44]

Unused.

[Pete Williamson, 2013/03/06 00:09:40]

Done.

+    if (it->second.expiration_time.is_null() ||
+        now < it->second.expiration_time) {
+      return it;
+    } else {
+      // Remove this entry if it is expired.
+      token_cache_.erase(it);
+      return token_cache_.end();
+    }
+  }
+
+  return token_cache_.end();
+}
+
+
+}  // namespace extensions