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

chrome/browser/token_cache/token_cache.h

+// 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.
+
+// This class caches tokens for the current user.  It will flush tokens out

[dcheng, 2013/02/28 19:18:04]

"flush" is an ambiguous term, since it could mean flushing the entries out to
disk.

[Pete Williamson, 2013/03/04 18:32:53]

Done.

+// when the user logs out or after the specified timeout interval, or when
+// the instance of chrome shuts down.
+
+#ifndef CHROME_BROWSER_TOKEN_CACHE_TOKEN_CACHE_H_
+#define CHROME_BROWSER_TOKEN_CACHE_TOKEN_CACHE_H_
+
+#include <algorithm>

[dcheng, 2013/02/28 19:18:04]

Why is this in the header?

[Pete Williamson, 2013/03/04 18:32:53]

This was leftover from when I tried the find-if version.  I'll remove it for now
and put it back if I refactor to find-if.

+#include <string>
+#include <vector>
+
+#include "base/basictypes.h"
+#include "base/compiler_specific.h"
+#include "base/time.h"
+#include "chrome/browser/profiles/profile_keyed_service.h"
+#include "content/public/browser/notification_observer.h"
+#include "content/public/browser/notification_registrar.h"
+#include "content/public/browser/notification_service.h"
+
+class Profile;
+
+struct TokenCacheData {
+  std::string token_name;
+  std::string token;
+  base::Time expiration_time;
+};
+
+// TODO: Should I put this in a namespace?  Which one?
+
+class TokenCacheService : public ProfileKeyedService,
+                                 content::NotificationObserver {

[dcheng, 2013/02/28 19:18:04]

Indent.

[Pete Williamson, 2013/03/04 18:32:53]

Done.

[dcheng, 2013/03/04 22:40:59]

Sorry, I should have clarified (and I also missed something). This should be
public ProfiledKeyedService, public content::NotificationObserver, and the
"public" keywords should be aligned under each other.

[Pete Williamson, 2013/03/05 19:42:25]

Done.

+
+ public:
+  explicit TokenCacheService(Profile* profile);
+  virtual ~TokenCacheService();
+
+  // Store a token for the currently logged in user. We will look it up later by
+  // the name given here, and we will expire the token after the timeout.  For a
+  // timeout of 0, we never expire the token.  The timeout is the number of
+  // milliseconds after which to expire this token.
+  void StoreToken(const std::string& token_name, const std::string& token_value,
+                  const int64 expiration_timeout);

[dcheng, 2013/02/28 19:18:04]

Use base::TimeDelta instead.

[Pete Williamson, 2013/03/04 18:32:53]

Done.

+
+  // 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 RetrieveToken(const std::string& token_name);
+
+  std::vector<TokenCacheData>::iterator FindMatch(
+      const std::string& token_name);
+
+  // inherited from ProfileKeyedService
+  virtual void Shutdown() OVERRIDE;
+
+  // inherited from NotificationObserver
+  virtual void Observe(int type,
+                       const content::NotificationSource& source,
+                       const content::NotificationDetails& details) OVERRIDE;
+
+ private:
+  // TODO(petewil): If we start putting lots of tokens in here, use a map
+  // instead because the lookup time is O(n).
+  std::vector<TokenCacheData> token_cache_;

[dcheng, 2013/02/28 19:18:04]

Why not just use a map since that's logically what it is anyway.

[Pete Williamson, 2013/03/04 18:32:53]

My concern is efficiency.  Maps generate a lot more code, and are only faster
when you have lots of entries due to locality of reference.  Vectors are much
faster until the search time dominates.  Currently only one token will be here,
so I don't want to pay the overhead of a map.  I don't expect breakeven until we
store more than 100 tokens.

[dcheng, 2013/03/04 22:40:59]

But then you just have to reimplement the lookup logic for a map which almost
certainly won't be shared. I think it makes more sense to just use the concepts
that the code is expressing. Using a vector doesn't gain much other than making
the code more complicated.

[Pete Williamson, 2013/03/05 19:42:25]

Done.

+  content::NotificationRegistrar registrar_;
+  Profile* profile_;

[dcheng, 2013/02/28 19:18:04]

Profile* const.

[Pete Williamson, 2013/03/04 18:32:53]

changed to "const Profile* profile_;"

[dcheng, 2013/03/04 22:40:59]

In that case, const Profile* const profile_; please. Note the positioning of the
original const.

[Pete Williamson, 2013/03/05 19:42:25]

Done.

+
+  friend class TokenCacheTest;

[dcheng, 2013/02/28 19:18:04]

DISALLOW_COPY_AND_ASSIGN.

[Pete Williamson, 2013/03/04 18:32:53]

Done.

+};
+
+
+
+#endif  // CHROME_BROWSER_TOKEN_CACHE_TOKEN_CACHE_H_