Add AffiliationFetcher to fetch authoritative affiliation information regarding facets.

components/password_manager/core/browser/affiliation_fetcher.h

Index: components/password_manager/core/browser/affiliation_fetcher.h
diff --git a/components/password_manager/core/browser/affiliation_fetcher.h b/components/password_manager/core/browser/affiliation_fetcher.h
new file mode 100644
index 0000000000000000000000000000000000000000..e3857e770c701525a1a610f9bc9b1fdaf46fe1f4
--- /dev/null
+++ b/components/password_manager/core/browser/affiliation_fetcher.h
@@ -0,0 +1,136 @@
+// Copyright 2014 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 COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_AFFILIATION_FETCHER_H_
+#define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_AFFILIATION_FETCHER_H_
+
+#include <vector>
+
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/threading/thread_checker.h"
+#include "components/password_manager/core/browser/affiliation_utils.h"
+#include "net/url_request/url_fetcher_delegate.h"
+
+class GURL;
+
+namespace net {
+class URLRequestContextGetter;
+}  // namespace net
+
+namespace password_manager {
+
+class AffiliationFetcherDelegate;
+class AffiliationFetcherFactory;
+
+// Fetches authoritative information regarding which facets are affiliated with
+// each other, that is, which facets belong to the same logical application.
+// See affiliation_utils.h for a definition of what this means.
+//
+// An instance is good for exactly one fetch, and may be used from any thread.
+// However, it should always be accessed on the thread it was created on.

[Mike West, 2014/12/02 14:32:42]

"should" or "must"? It looks like you're only using the thread checker in
'StartRequest()'. If this is a "must", then perhaps you could consider asserting
thread safety in the accessor methods as well?

[engedy, 2014/12/02 18:33:56]

Actually, I have just removed the ThreadChecker. Unless the caller is crazy, I
do not foresee any use-cases where there would be a significant risk of misuse
that such a check could prevent -- overchecking this might not have much merit.

+class AffiliationFetcher : net::URLFetcherDelegate {
+ public:
+  // Encapsulates the response to an affiliations request.
+  typedef std::vector<AffiliatedFacets> Result;
+
+  ~AffiliationFetcher() override;
+
+  // Constructs a fetcher to retrieve affiliations for each facet in |facet_ids|
+  // using the specified |request_context_getter|, and will provide the results
+  // to the |delegate| on the same thread that creates the instance.
+  static AffiliationFetcher* Create(
+      net::URLRequestContextGetter* request_context_getter,
+      const std::vector<std::string>& facet_uris,
+      AffiliationFetcherDelegate* delegate);
+
+  // Sets the |factory| to be used by the Create() method instead of the vanilla
+  // constructor for creating AffiliationFetcher instances. To revert to using
+  // the vanilla constructor, set NULL. Used only for unit testing.
+  static void SetFactoryForTesting(AffiliationFetcherFactory* factory);
+
+  // Actually starts the request, and will call the delegate with the results
+  // once done. If |this| is destroyed before completion, the in-flight request
+  // is cancelled, and the delegate will not be called. Further details:
+  //   * No cookies are sent/saved with the request.
+  //   * In case of network/server errors, the request will not be retried.
+  //   * Results are guaranteed to be always fresh and will never be cached.

[Mike West, 2014/12/02 14:32:42]

Is it easily possible to add tests for these assertions to lock them in place?

[engedy, 2014/12/02 18:33:56]

We could verify in the unit test that the load flags are set on the
TestURLFetcher, but as far as I understand, that would be a change-detector
test, which are considered harmful. Let me know if you disagree.

(Another solution would be to add a browsertest in Chrome code to check the
state of the cache/cookie jar before and after. That would be a huge amount of
work, and it would mostly test URLFetcher, not our class.)

+  virtual void StartRequest();
+
+  const std::vector<std::string>& requested_facet_uris() const {
+    return requested_facet_uris_;
+  }
+
+  AffiliationFetcherDelegate* delegate() const { return delegate_; }
+
+ protected:
+  AffiliationFetcher(net::URLRequestContextGetter* request_context_getter,
+                     const std::vector<std::string>& facet_uris,
+                     AffiliationFetcherDelegate* delegate);
+
+ private:
+  // Builds the URL at which the affiliation service is available.
+  GURL BuildQueryURL() const;
+
+  // Prepares and returns the serialized form of the JSON dictionary that will
+  // be the payload of the POST request.
+  std::string PreparePayload() const;
+
+  // Parses the server response into |result| and returns true on success. We
+  // gracefully ignore most unknown fields in the response, so this method will
+  // return false only if the response is gravely ill-formed.
+  bool ParseResponse(Result* result) const;
+
+  // net::URLFetcherDelegate:
+  void OnURLFetchComplete(const net::URLFetcher* source) override;
+
+  base::ThreadChecker thread_checker_;
+
+  scoped_refptr<net::URLRequestContextGetter> request_context_getter_;
+  std::vector<std::string> requested_facet_uris_;
+  AffiliationFetcherDelegate* delegate_;
+
+  scoped_ptr<net::URLFetcher> fetcher_;
+
+  DISALLOW_COPY_AND_ASSIGN(AffiliationFetcher);
+};
+
+// Interface that users of AffiliationFetcher should implement to get results of
+// the fetch. It is safe to destroy the fetcher in any of the event handlers.
+class AffiliationFetcherDelegate {

[Mike West, 2014/12/02 14:32:43]

Nit: Why bundle this into the same file as the fetcher?

[engedy, 2014/12/02 18:33:55]

I just didn't want to frighten potential code reviewers with a large number of
files. :)

Factored it out.

+ public:
+  // Called when affiliation information has been successfully retrieved. The
+  // |result| will contain at most as many equivalence class as facet URIs
+  // requested. Each facet URI will appear in exactly one equivalence class.
+  virtual void OnFetchSucceeded(
+      scoped_ptr<AffiliationFetcher::Result> result) = 0;
+
+  // Called when affiliation information could not be fetched due to network or
+  // server error.
+  virtual void OnFetchFailed() = 0;
+
+  // This will be called when an affiliation response was received, but it was
+  // gravely malformed.
+  virtual void OnMalformedResponse() = 0;
+
+ protected:
+  virtual ~AffiliationFetcherDelegate() {}
+};
+
+// Interface for a factory to construct AffiliationFetcher objects, to be used
+// as an alternative instead of simply calling the constructor.
+class AffiliationFetcherFactory {

[Mike West, 2014/12/02 14:32:42]

Nit: Ditto.

[engedy, 2014/12/02 18:33:56]

Actually, I will remove this for now, and will add it back with the testing
helpers later.

+ public:
+  virtual AffiliationFetcher* CreateInstance(
+      net::URLRequestContextGetter* request_context_getter,
+      const std::vector<std::string>& facet_uris,
+      AffiliationFetcherDelegate* delegate) = 0;
+
+ protected:
+  virtual ~AffiliationFetcherFactory() {}
+};
+
+}  // namespace password_manager
+
+#endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_AFFILIATION_FETCHER_H_