Reduce visibility of methods in AutofillManager and AutofillDownload.

chrome/browser/autofill/autofill_download.cc

 // Copyright (c) 2011 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/autofill/autofill_download.h"
 
 #include <algorithm>
 #include <ostream>
 #include <vector>
 
@@ skipping 20 matching lines @@
 const char kAutofillQueryServerNameStartInHeader[] = "GFE/";
 
 const size_t kMaxFormCacheSize = 16;
 };
 
 struct AutofillDownloadManager::FormRequestData {
   std::vector<std::string> form_signatures;
   AutofillRequestType request_type;
 };
 
-AutofillDownloadManager::AutofillDownloadManager(Profile* profile)
+AutofillDownloadManager::AutofillDownloadManager(Profile* profile,
+                                                 Observer* observer)
     : profile_(profile),
-      observer_(NULL),
+      observer_(observer),
       max_form_cache_size_(kMaxFormCacheSize),
       next_query_request_(base::Time::Now()),
       next_upload_request_(base::Time::Now()),
       positive_upload_rate_(0),
       negative_upload_rate_(0),
       fetcher_id_for_unittest_(0) {
-  // |profile_| could be NULL in some unit-tests.
-  if (profile_) {
-    PrefService* preferences = profile_->GetPrefs();
-    positive_upload_rate_ =
-        preferences->GetDouble(prefs::kAutofillPositiveUploadRate);
-    negative_upload_rate_ =
-        preferences->GetDouble(prefs::kAutofillNegativeUploadRate);
-  }
+  DCHECK(observer_);

[dhollowa, 2011/10/26 15:58:33]

Should have a DCHECK(profile_) here if you're eliminating them below.

[Ilya Sherman, 2011/10/26 23:06:32]

|profile_| is used on line 52, so the DCHECK would be redundant.

[dhollowa, 2011/10/27 23:37:34]

Ok, that's fine.  But if PrefService has no virtuals then it may not crash. 
I.e. in C++ it is possible to do: (Foo *)(NULL)->DoIt(), and DoIt() will execute
fine.

On 2011/10/26 23:06:32, Ilya Sherman wrote:

+  PrefService* preferences = profile_->GetPrefs();
+  positive_upload_rate_ =
+      preferences->GetDouble(prefs::kAutofillPositiveUploadRate);
+  negative_upload_rate_ =
+      preferences->GetDouble(prefs::kAutofillNegativeUploadRate);
 }
 
 AutofillDownloadManager::~AutofillDownloadManager() {
   STLDeleteContainerPairFirstPointers(url_fetchers_.begin(),
                                       url_fetchers_.end());
 }
 
-void AutofillDownloadManager::SetObserver(
-    AutofillDownloadManager::Observer* observer) {
-  if (observer) {
-    DCHECK(!observer_);
-    observer_ = observer;
-  } else {
-    observer_ = NULL;
-  }
-}
-
 bool AutofillDownloadManager::StartQueryRequest(
     const std::vector<FormStructure*>& forms,
     const AutofillMetrics& metric_logger) {
   if (next_query_request_ > base::Time::Now()) {
     // We are in back-off mode: do not do the request.
     return false;
   }
   std::string form_xml;
   FormRequestData request_data;
   if (!FormStructure::EncodeQueryRequest(forms, &request_data.form_signatures,
                                          &form_xml)) {
     return false;
   }
 
   request_data.request_type = AutofillDownloadManager::REQUEST_QUERY;
   metric_logger.LogServerQueryMetric(AutofillMetrics::QUERY_SENT);
 
   std::string query_data;
   if (CheckCacheForQueryRequest(request_data.form_signatures, &query_data)) {
-    VLOG(1) << "AutofillDownloadManager: query request has been retrieved from"
-            << "the cache";
-    if (observer_)
-      observer_->OnLoadedServerPredictions(query_data);
+    DVLOG(1) << "AutofillDownloadManager: query request has been retrieved from"
+             << "the cache";
+    observer_->OnLoadedServerPredictions(query_data);
     return true;
   }
 
   return StartRequest(form_xml, request_data);
 }
 
 bool AutofillDownloadManager::StartUploadRequest(
     const FormStructure& form,
     bool form_was_autofilled,
     const FieldTypeSet& available_field_types) {
   if (next_upload_request_ > base::Time::Now()) {
     // We are in back-off mode: do not do the request.
-    VLOG(1) << "AutofillDownloadManager: Upload request is throttled.";
+    DVLOG(1) << "AutofillDownloadManager: Upload request is throttled.";
     return false;
   }
 
   // Flip a coin to see if we should upload this form.
   double upload_rate = form_was_autofilled ? GetPositiveUploadRate() :
                                              GetNegativeUploadRate();
   if (form.upload_required() == UPLOAD_NOT_REQUIRED ||
       (form.upload_required() == USE_UPLOAD_RATES &&
        base::RandDouble() > upload_rate)) {
-    VLOG(1) << "AutofillDownloadManager: Upload request is ignored.";
+    DVLOG(1) << "AutofillDownloadManager: Upload request is ignored.";
     // If we ever need notification that upload was skipped, add it here.
     return false;
   }
 
   std::string form_xml;
   if (!form.EncodeUploadRequest(available_field_types, form_was_autofilled,
                                 &form_xml))
     return false;
 
   FormRequestData request_data;
   request_data.form_signatures.push_back(form.FormSignature());
   request_data.request_type = AutofillDownloadManager::REQUEST_UPLOAD;
 
   return StartRequest(form_xml, request_data);
 }
 
-bool AutofillDownloadManager::CancelRequest(
-    const std::string& form_signature,
-    AutofillDownloadManager::AutofillRequestType request_type) {
-  for (std::map<content::URLFetcher*, FormRequestData>::iterator it =
-       url_fetchers_.begin();
-       it != url_fetchers_.end();
-       ++it) {
-    if (std::find(it->second.form_signatures.begin(),
-        it->second.form_signatures.end(), form_signature) !=
-        it->second.form_signatures.end() &&
-        it->second.request_type == request_type) {
-      delete it->first;
-      url_fetchers_.erase(it);
-      return true;
-    }
-  }
-  return false;
-}
-
 double AutofillDownloadManager::GetPositiveUploadRate() const {
   return positive_upload_rate_;
 }
 
 double AutofillDownloadManager::GetNegativeUploadRate() const {
   return negative_upload_rate_;
 }
 
 void AutofillDownloadManager::SetPositiveUploadRate(double rate) {
   if (rate == positive_upload_rate_)
     return;
   positive_upload_rate_ = rate;
   DCHECK_GE(rate, 0.0);
   DCHECK_LE(rate, 1.0);
-  DCHECK(profile_);
   PrefService* preferences = profile_->GetPrefs();
   preferences->SetDouble(prefs::kAutofillPositiveUploadRate, rate);
 }
 
 void AutofillDownloadManager::SetNegativeUploadRate(double rate) {
   if (rate == negative_upload_rate_)
     return;
   negative_upload_rate_ = rate;
   DCHECK_GE(rate, 0.0);
   DCHECK_LE(rate, 1.0);
-  DCHECK(profile_);
   PrefService* preferences = profile_->GetPrefs();
   preferences->SetDouble(prefs::kAutofillNegativeUploadRate, rate);
 }
 
 bool AutofillDownloadManager::StartRequest(
     const std::string& form_xml,
     const FormRequestData& request_data) {
   net::URLRequestContextGetter* request_context = profile_->GetRequestContext();
   DCHECK(request_context);
   std::string request_url;
@@ skipping 109 matching lines @@
 
     if (back_off) {
       base::Time back_off_time(base::Time::Now() + source->GetBackoffDelay());
       if (it->second.request_type == AutofillDownloadManager::REQUEST_QUERY) {
         next_query_request_ = back_off_time;
       } else {
         next_upload_request_ = back_off_time;
       }
     }
 
-    LOG(WARNING) << "AutofillDownloadManager: " << type_of_request
-                 << " request has failed with response "
-                 << source->GetResponseCode();
-    if (observer_) {
-      observer_->OnServerRequestError(it->second.form_signatures[0],
-                                      it->second.request_type,
-                                      source->GetResponseCode());
-    }
+    DVLOG(1) << "AutofillDownloadManager: " << type_of_request
+             << " request has failed with response "
+             << source->GetResponseCode();
+    observer_->OnServerRequestError(it->second.form_signatures[0],
+                                    it->second.request_type,
+                                    source->GetResponseCode());
   } else {
-    VLOG(1) << "AutofillDownloadManager: " << type_of_request
-            << " request has succeeded";
+    DVLOG(1) << "AutofillDownloadManager: " << type_of_request
+             << " request has succeeded";
     std::string response_body;
     source->GetResponseAsString(&response_body);
     if (it->second.request_type == AutofillDownloadManager::REQUEST_QUERY) {
       CacheQueryRequest(it->second.form_signatures, response_body);
-      if (observer_)
-        observer_->OnLoadedServerPredictions(response_body);
+      observer_->OnLoadedServerPredictions(response_body);
     } else {
       double new_positive_upload_rate = 0;
       double new_negative_upload_rate = 0;
       AutofillUploadXmlParser parse_handler(&new_positive_upload_rate,
                                             &new_negative_upload_rate);
       buzz::XmlParser parser(&parse_handler);
       parser.Parse(response_body.data(), response_body.length(), true);
       if (parse_handler.succeeded()) {
         SetPositiveUploadRate(new_positive_upload_rate);
         SetNegativeUploadRate(new_negative_upload_rate);
       }
 
-      if (observer_)
-        observer_->OnUploadedPossibleFieldTypes();
+      observer_->OnUploadedPossibleFieldTypes();
     }
   }
   delete it->first;
   url_fetchers_.erase(it);
 }