Introduce StrategyOnCacheMiss enum instead of |cached_only| boolean.

components/password_manager/core/browser/affiliation_service.h

 // Copyright 2015 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_SERVICE_H_
 #define COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_AFFILIATION_SERVICE_H_
 
 #include <string>
 
 #include "base/callback.h"
@@ skipping 23 matching lines @@
 //
 // The service must be accessed from the UI thread, and it can be utilized in
 // two ways:
 //
 //   1.) On-demand fetching: For the one-off query that wishes to learn
 //       affiliations of facet X when (potentially) issuing an on-demand
 //       network request to the Affiliation API containing the URI of facet X
 //       is acceptable from the privacy and/or performance perspective.
 //
 //       This mode of operation is achieved by invoking GetAffiliations() with
-//       |cached_only| set to false.
+//       StrategyOnCacheMiss::FETCH_OVER_NETWORK.
 //
 //   2.) Proactive fetching: For the compound query that is concerned with
 //       checking, over time, whether or not each element in a sequence of
 //       facets, W_1, W_2, ..., W_n, is affiliated with a fixed facet Y; and
 //       when it is desired, for privacy and/or performance reasons, that only
 //       facet Y be looked up against the Affiliation API and that subsequent
 //       requests regarding each W_i not trigger additional requests.
 //
 //       This mode of operation can be useful when, for example, the password
 //       manager has credentials stored for facet Y and wishes to check, for
 //       each visited web site W_i, whether these credentials should be offered
 //       to be autofilled onto W_i.
 //
 //       Example code:
 //
 //       class ExampleAffiliatedCredentialFiller
 //           : public base::SupportsWeakPtr<...> {
 //        public:
 //         ExampleAffiliatedCredentialFiller(AffiliationService* service,
 //                                           const FacetURI& y)
 //             : service_(service), y_(y) {
 //           cancel_handle_ = service_->Prefetch(y_, base::Time::Max());
 //         }
 //
 //         ~ExampleAffiliatedCredentialFiller() { cancel_handle_.Run(); }
 //
 //         void ShouldFillInto(const FacetURI& wi, FillDelegate* delegate) {
-//           service_->GetAffiliations(wi, false, base::Bind(
-//               &ExampleAffiliatedCredentialFiller::OnAffiliationResult,
-//               AsWeakPtr(),
-//               delegate));
+//           service_->GetAffiliations(wi, StrategyOnCacheMiss::FAIL,
+//               base::Bind(
+//                   &ExampleAffiliatedCredentialFiller::OnAffiliationResult,
+//                   AsWeakPtr(),
+//                   delegate));
 //         }
 //
 //         void OnAffiliationResult(FillDelegate* delegate,
 //                                  const AffiliatedFacets& results,
 //                                  bool success) {
 //           if (success && std::count(results.begin(), results.end(), y_))
 //             delegate->FillCredentialsFor(y_);
 //         }
 //
 //        private:
 //         AffiliationService* service_;
 //         const FacetURI& y_;
 //         CancelPrefetchingHandle cancel_handle_;
 //       };
 class AffiliationService : public KeyedService {
  public:
   typedef base::Callback<void(const AffiliatedFacets& /* results */,
                               bool /* success */)> ResultCallback;
 
+  // Controls whether to send a network request or fail on a cache miss.
+  enum class StrategyOnCacheMiss { FETCH_OVER_NETWORK, FAIL };

[Mike West, 2015/03/17 13:47:10]

These names seem fine to me. Failure is totally a strategy.

If you really want to change them, I'd suggest: `CacheDisposition { USE_CACHE,
DO_NOT_USE_CACHE }`, but I don't think that's so much better that you MUST MUST
MUST change it.

[engedy, 2015/03/17 13:54:07]

I dwelled on it for a bit more, and while a bit weirder, StrategyOnCacheMiss
seems to convey more information.

+
   // The |backend_task_runner| should be a task runner corresponding to a thread
   // that can take blocking I/O, and is normally Chrome's DB thread.
   AffiliationService(
       scoped_refptr<base::SingleThreadTaskRunner> backend_task_runner);
   ~AffiliationService() override;
 
   // Initializes the service by creating its backend and transferring it to the
   // thread corresponding to |backend_task_runner_|.
   void Initialize(net::URLRequestContextGetter* request_context_getter,
                   const base::FilePath& db_path);
 
-  // Looks up facets affiliated with the facet identified by |facet_uri|. If
-  // |cached_only| is true, the results will be based solely on prefetched
-  // information already stored in the cache. Otherwise, on-demand network
-  // requests will be issued if there is no up-to-date data in the cache.
+  // Looks up facets affiliated with the facet identified by |facet_uri|, and
+  // invokes |result_callback| with the results.
+  //
+  // If the local cache contains fresh affiliation information for |facet_uri|,
+  // the request will be served from cache. Otherwise, |cache_miss_policy|
+  // controls whether to issue an on-demand network request, or to fail the
+  // request without fetching.
   virtual void GetAffiliations(const FacetURI& facet_uri,
-                               bool cached_only,
+                               StrategyOnCacheMiss cache_miss_strategy,
                                const ResultCallback& result_callback);
 
   // Prefetches affiliation information for the facet identified by |facet_uri|,
   // and keeps the information fresh by periodic re-fetches (as needed) until
   // the clock strikes |keep_fresh_until| (exclusive), until a matching call to
   // CancelPrefetch(), or until Chrome is shut down, whichever is sooner. It is
   // a supported use-case to pass base::Time::Max() as |keep_fresh_until|.
   //
   // Canceling can be useful when a password is deleted, so that resources are
   // no longer wasted on repeatedly refreshing affiliation information. Note
@@ skipping 24 matching lines @@
 
   base::ThreadChecker thread_checker_;
   base::WeakPtrFactory<AffiliationService> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(AffiliationService);
 };
 
 }  // namespace password_manager
 
 #endif  // COMPONENTS_PASSWORD_MANAGER_CORE_BROWSER_AFFILIATION_SERVICE_H_