Download autocheckout whitelist and enable autocheckout for whitelisted sites only.

chrome/browser/autofill/autocheckout/whitelist_manager.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.
+
+#ifndef CHROME_BROWSER_AUTOFILL_AUTOCHECKOUT_WHITELIST_MANAGER_H_
+#define CHROME_BROWSER_AUTOFILL_AUTOCHECKOUT_WHITELIST_MANAGER_H_
+
+#include <string>
+#include <vector>
+
+#include "base/supports_user_data.h"
+#include "base/time.h"
+#include "base/timer.h"
+#include "net/url_request/url_fetcher_delegate.h"
+
+class GURL;
+
+namespace content {
+class BrowserContext;
+}
+
+namespace net {
+class URLRequestContextGetter;
+class URLFetcher;
+}
+
+namespace autocheckout {

[Ilya Sherman, 2013/01/24 22:01:55]

nit: Please also wrap this in the autofill namespace, so that the full namespace
is autofill::autocheckout.

[benquan, 2013/01/25 00:55:31]

Done.

+
+// WhitelistManager is responsible for download and caching Autocheckout
+// whitelist from the server.
+class WhitelistManager : public net::URLFetcherDelegate,
+                         public base::SupportsUserData::Data {

[Ilya Sherman, 2013/01/24 22:01:55]

nit: I don't think you need to inherit from SupportsUserData.  Instead, use
base::UserDataAdapter.

[benquan, 2013/01/25 00:55:31]

Done.

+ public:
+

[Ilya Sherman, 2013/01/24 22:01:55]

nit: Omit this newline

[benquan, 2013/01/25 00:55:31]

Done.

+  static WhitelistManager* GetForBrowserContext(
+      content::BrowserContext* context);
+
+  // Checks if the given url is whitelisted.
+  bool IsAutocheckoutEnabled(const GURL& url);
+
+  // Schedules a future call to TriggerDownload if one isn't already pending.
+  // Returns true if a new download activity is scheduled.
+  bool ScheduleDownload(int interval_seconds);

[Ilya Sherman, 2013/01/24 22:01:55]

Why is this public?

[Ilya Sherman, 2013/01/24 22:01:55]

nit: Use size_t rather than int, since you never want negative values.

[benquan, 2013/01/25 00:55:31]

Done.

[benquan, 2013/01/25 00:55:31]

Done.

+
+ private:
+  explicit WhitelistManager(net::URLRequestContextGetter* context_getter);
+
+  // Timer callback indicating it's time to download whitelist from server.
+  void TriggerDownload();
+
+  // Implements net::URLFetcherDelegate.
+  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+
+  // Parse whitelist data and build whitelist

[Ilya Sherman, 2013/01/24 22:01:55]

nit: End comment with a period.

[Ilya Sherman, 2013/01/24 22:01:55]

Please describe (at a high level) what format the data has.

[benquan, 2013/01/25 00:55:31]

Done.

[benquan, 2013/01/25 00:55:31]

Done.

+  void BuildWhitelist(const std::string& data);
+
+  // The context for the request.
+  // The pointer value is const, so this can only be set in the constructor.

[Ilya Sherman, 2013/01/24 22:01:55]

nit: Omit this line; it's redundant with the code.

[benquan, 2013/01/25 00:55:31]

Done.

+  // Must not be null.
+  net::URLRequestContextGetter* const context_getter_;  // WEAK
+
+  // Indicates that the last triggered download hasn't resolved yet.
+  bool callback_pending_;

[Ilya Sherman, 2013/01/24 22:01:55]

nit: Perhaps name this has_callback_pending_ or callback_is_pending_ or
something like that?

[benquan, 2013/01/25 00:55:31]

Done.

+
+  // State of the kEnableExperimentalFormFilling flag.
+  const bool experimental_form_filling_enabled_;
+
+  // The request object.
+  scoped_ptr<net::URLFetcher> request_;
+
+  // A list of whitelisted url prefixes.
+  std::vector<std::string> url_prefixes_;
+
+  base::OneShotTimer<WhitelistManager> download_timer_;
+
+  friend class WhitelistManagerTest;

[Ilya Sherman, 2013/01/24 22:01:55]

Rather than friending the test, which allows the test to make all sorts of calls
that break invariants, create a TestWhitelistManager that inherits from
WhitelistManager, and provide a minimal set of protected methods that the test
manager can override.

[benquan, 2013/01/25 00:55:31]

TestWhitelistManager still can not access WhitelistManager's private members,
should I friend TestWhitelistManager or make the members protected rather than
private?

[Ilya Sherman, 2013/01/25 01:22:40]

The members should remain private, but you can add protected accessors to them.

[benquan, 2013/01/26 02:05:53]

Some functions needed by tests are also lifted to protected section.

On 2013/01/25 01:22:40, Ilya Sherman wrote:

+
+  DISALLOW_COPY_AND_ASSIGN(WhitelistManager);
+};
+
+}  // namespace autocheckout
+
+#endif  // CHROME_BROWSER_AUTOFILL_AUTOCHECKOUT_WHITELIST_MANAGER_H_
+