Reduce visibility of methods in AutofillManager and AutofillDownload.
Also remove some obsolete code and simplify some of the code's expectations.
BUG=none
TEST=none
Review URL: http://codereview.chromium.org/8351027
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@107680 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/chrome/browser/autofill/autofill_download.cc b/chrome/browser/autofill/autofill_download.cc
index 69f151dd..529a77b6 100644
--- a/chrome/browser/autofill/autofill_download.cc
+++ b/chrome/browser/autofill/autofill_download.cc
@@ -38,23 +38,22 @@
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_);
+ PrefService* preferences = profile_->GetPrefs();
+ positive_upload_rate_ =
+ preferences->GetDouble(prefs::kAutofillPositiveUploadRate);
+ negative_upload_rate_ =
+ preferences->GetDouble(prefs::kAutofillNegativeUploadRate);
}
AutofillDownloadManager::~AutofillDownloadManager() {
@@ -62,16 +61,6 @@
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) {
@@ -91,10 +80,9 @@
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;
}
@@ -107,7 +95,7 @@
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;
}
@@ -117,7 +105,7 @@
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;
}
@@ -134,25 +122,6 @@
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_;
}
@@ -167,7 +136,6 @@
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);
}
@@ -178,7 +146,6 @@
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);
}
@@ -307,23 +274,20 @@
}
}
- 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;
@@ -336,8 +300,7 @@
SetNegativeUploadRate(new_negative_upload_rate);
}
- if (observer_)
- observer_->OnUploadedPossibleFieldTypes();
+ observer_->OnUploadedPossibleFieldTypes();
}
}
delete it->first;
diff --git a/chrome/browser/autofill/autofill_download.h b/chrome/browser/autofill/autofill_download.h
index ce5aa95..6144b90 100644
--- a/chrome/browser/autofill/autofill_download.h
+++ b/chrome/browser/autofill/autofill_download.h
@@ -37,33 +37,31 @@
};
// An interface used to notify clients of AutofillDownloadManager.
- // Notifications are *not* guaranteed to be called.
class Observer {
public:
// Called when field type predictions are successfully received from the
- // server.
- // |response_xml| - server response.
+ // server. |response_xml| contains the server response.
virtual void OnLoadedServerPredictions(const std::string& response_xml) = 0;
+
+ // These notifications are used to help with testing.
// Called when heuristic either successfully considered for upload and
// not send or uploaded.
- virtual void OnUploadedPossibleFieldTypes() = 0;
+ virtual void OnUploadedPossibleFieldTypes() {}
// Called when there was an error during the request.
// |form_signature| - the signature of the requesting form.
// |request_type| - type of request that failed.
// |http_error| - HTTP error code.
virtual void OnServerRequestError(const std::string& form_signature,
AutofillRequestType request_type,
- int http_error) = 0;
+ int http_error) {}
+
protected:
virtual ~Observer() {}
};
- // |profile| can be NULL in unit-tests only.
- explicit AutofillDownloadManager(Profile* profile);
- virtual ~AutofillDownloadManager();
-
// |observer| - observer to notify on successful completion or error.
- void SetObserver(AutofillDownloadManager::Observer* observer);
+ AutofillDownloadManager(Profile* profile, Observer* observer);
+ virtual ~AutofillDownloadManager();
// Starts a query request to Autofill servers. The observer is called with the
// list of the fields of all requested forms.
@@ -82,14 +80,6 @@
bool form_was_autofilled,
const FieldTypeSet& available_field_types);
- // Cancels pending request.
- // |form_signature| - signature of the form being cancelled. Warning:
- // for query request if more than one form sent in the request, all other
- // forms will be cancelled as well.
- // |request_type| - type of the request.
- bool CancelRequest(const std::string& form_signature,
- AutofillRequestType request_type);
-
private:
friend class AutofillDownloadTest;
FRIEND_TEST_ALL_PREFIXES(AutofillDownloadTest, QueryAndUploadTest);
@@ -136,14 +126,18 @@
void SetPositiveUploadRate(double rate);
void SetNegativeUploadRate(double rate);
- // Profile for preference storage.
- Profile* profile_;
+ // Profile for preference storage. The pointer value is const, so this can
+ // only be set in the constructor. Must not be null.
+ Profile* const profile_; // WEAK
+ // The observer to notify when server predictions are successfully received.
+ // The pointer value is const, so this can only be set in the constructor.
+ // Must not be null.
+ AutofillDownloadManager::Observer* const observer_; // WEAK
// For each requested form for both query and upload we create a separate
// request and save its info. As url fetcher is identified by its address
// we use a map between fetchers and info.
std::map<content::URLFetcher*, FormRequestData> url_fetchers_;
- AutofillDownloadManager::Observer *observer_;
// Cached QUERY requests.
QueryRequestCache cached_forms_;
diff --git a/chrome/browser/autofill/autofill_download_unittest.cc b/chrome/browser/autofill/autofill_download_unittest.cc
index 325a3da..ad76e1f 100644
--- a/chrome/browser/autofill/autofill_download_unittest.cc
+++ b/chrome/browser/autofill/autofill_download_unittest.cc
@@ -64,7 +64,7 @@
public testing::Test {
public:
AutofillDownloadTest()
- : download_manager_(&profile_),
+ : download_manager_(&profile_, this),
io_thread_(BrowserThread::IO) {
}
@@ -73,13 +73,11 @@
options.message_loop_type = MessageLoop::TYPE_IO;
io_thread_.StartWithOptions(options);
profile_.CreateRequestContext();
- download_manager_.SetObserver(this);
}
virtual void TearDown() {
profile_.ResetRequestContext();
io_thread_.Stop();
- download_manager_.SetObserver(NULL);
}
void LimitCache(size_t cache_size) {
@@ -87,14 +85,15 @@
}
// AutofillDownloadManager::Observer implementation.
- virtual void OnLoadedServerPredictions(const std::string& response_xml) {
+ virtual void OnLoadedServerPredictions(
+ const std::string& response_xml) OVERRIDE {
ResponseData response;
response.response = response_xml;
response.type_of_response = QUERY_SUCCESSFULL;
responses_.push_back(response);
}
- virtual void OnUploadedPossibleFieldTypes() {
+ virtual void OnUploadedPossibleFieldTypes() OVERRIDE {
ResponseData response;
response.type_of_response = UPLOAD_SUCCESSFULL;
responses_.push_back(response);
@@ -103,7 +102,7 @@
virtual void OnServerRequestError(
const std::string& form_signature,
AutofillDownloadManager::AutofillRequestType request_type,
- int http_error) {
+ int http_error) OVERRIDE {
ResponseData response;
response.signature = form_signature;
response.error = http_error;
@@ -113,7 +112,7 @@
responses_.push_back(response);
}
- enum TYPE_OF_RESPONSE {
+ enum ResponseType {
QUERY_SUCCESSFULL,
UPLOAD_SUCCESSFULL,
REQUEST_QUERY_FAILED,
@@ -121,12 +120,12 @@
};
struct ResponseData {
- TYPE_OF_RESPONSE type_of_response;
+ ResponseType type_of_response;
int error;
std::string signature;
std::string response;
- ResponseData() : type_of_response(REQUEST_QUERY_FAILED), error(0) {
- }
+
+ ResponseData() : type_of_response(REQUEST_QUERY_FAILED), error(0) {}
};
std::list<ResponseData> responses_;
diff --git a/chrome/browser/autofill/autofill_manager.cc b/chrome/browser/autofill/autofill_manager.cc
index 3fbbc99..0bc082a 100644
--- a/chrome/browser/autofill/autofill_manager.cc
+++ b/chrome/browser/autofill/autofill_manager.cc
@@ -208,7 +208,7 @@
: TabContentsObserver(tab_contents->tab_contents()),
tab_contents_wrapper_(tab_contents),
personal_data_(NULL),
- download_manager_(tab_contents->profile()),
+ download_manager_(tab_contents->profile(), this),
disable_download_manager_requests_(false),
metric_logger_(new AutofillMetrics),
has_logged_autofill_enabled_(false),
@@ -222,11 +222,9 @@
// |personal_data_| is NULL when using TestTabContents.
personal_data_ = PersonalDataManagerFactory::GetForProfile(
tab_contents->profile()->GetOriginalProfile());
- download_manager_.SetObserver(this);
}
AutofillManager::~AutofillManager() {
- download_manager_.SetObserver(NULL);
}
// static
@@ -618,15 +616,6 @@
SendAutofillTypePredictions(form_structures_.get());
}
-void AutofillManager::OnUploadedPossibleFieldTypes() {
-}
-
-void AutofillManager::OnServerRequestError(
- const std::string& form_signature,
- AutofillDownloadManager::AutofillRequestType request_type,
- int http_error) {
-}
-
bool AutofillManager::IsAutofillEnabled() const {
Profile* profile = Profile::FromBrowserContext(
const_cast<AutofillManager*>(this)->tab_contents()->browser_context());
@@ -736,7 +725,7 @@
: TabContentsObserver(tab_contents->tab_contents()),
tab_contents_wrapper_(tab_contents),
personal_data_(personal_data),
- download_manager_(NULL),
+ download_manager_(tab_contents->profile(), this),
disable_download_manager_requests_(true),
metric_logger_(new AutofillMetrics),
has_logged_autofill_enabled_(false),
diff --git a/chrome/browser/autofill/autofill_manager.h b/chrome/browser/autofill/autofill_manager.h
index 80c4ce0..e4fd014d 100644
--- a/chrome/browser/autofill/autofill_manager.h
+++ b/chrome/browser/autofill/autofill_manager.h
@@ -54,51 +54,26 @@
// Registers our Enable/Disable Autofill pref.
static void RegisterUserPrefs(PrefService* prefs);
- // TabContentsObserver implementation.
- virtual void DidNavigateMainFramePostCommit(
- const content::LoadCommittedDetails& details,
- const ViewHostMsg_FrameNavigate_Params& params);
- virtual bool OnMessageReceived(const IPC::Message& message);
-
- // AutofillDownloadManager::Observer implementation:
- virtual void OnLoadedServerPredictions(const std::string& response_xml);
- virtual void OnUploadedPossibleFieldTypes();
- virtual void OnServerRequestError(
- const std::string& form_signature,
- AutofillDownloadManager::AutofillRequestType request_type,
- int http_error);
-
- // Returns the value of the AutofillEnabled pref.
- virtual bool IsAutofillEnabled() const;
-
- // Imports the form data, submitted by the user, into |personal_data_|.
- void ImportFormData(const FormStructure& submitted_form);
-
- // Uploads the form data to the Autofill server.
- virtual void UploadFormData(const FormStructure& submitted_form);
-
- // Reset cache.
- void Reset();
-
protected:
- // For tests:
+ // Only test code should subclass AutofillManager.
// The string/int pair is composed of the guid string and variant index
// respectively. The variant index is an index into the multi-valued item
// (where applicable).
typedef std::pair<std::string, size_t> GUIDPair;
+ // Test code should prefer to use this constructor.
AutofillManager(TabContentsWrapper* tab_contents,
PersonalDataManager* personal_data);
- void set_personal_data_manager(PersonalDataManager* personal_data) {
- personal_data_ = personal_data;
- }
+ // Returns the value of the AutofillEnabled pref.
+ virtual bool IsAutofillEnabled() const;
- const AutofillMetrics* metric_logger() const { return metric_logger_.get(); }
- void set_metric_logger(const AutofillMetrics* metric_logger);
+ // Uploads the form data to the Autofill server.
+ virtual void UploadFormData(const FormStructure& submitted_form);
- ScopedVector<FormStructure>* form_structures() { return &form_structures_; }
+ // Reset cache.
+ void Reset();
// Maps GUIDs to and from IDs that are used to identify profiles and credit
// cards sent to and from the renderer process.
@@ -110,7 +85,22 @@
int PackGUIDs(const GUIDPair& cc_guid, const GUIDPair& profile_guid) const;
void UnpackGUIDs(int id, GUIDPair* cc_guid, GUIDPair* profile_guid) const;
+ const AutofillMetrics* metric_logger() const { return metric_logger_.get(); }
+ void set_metric_logger(const AutofillMetrics* metric_logger);
+
+ ScopedVector<FormStructure>* form_structures() { return &form_structures_; }
+
private:
+ // TabContentsObserver:
+ virtual void DidNavigateMainFramePostCommit(
+ const content::LoadCommittedDetails& details,
+ const ViewHostMsg_FrameNavigate_Params& params) OVERRIDE;
+ virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
+
+ // AutofillDownloadManager::Observer:
+ virtual void OnLoadedServerPredictions(
+ const std::string& response_xml) OVERRIDE;
+
void OnFormSubmitted(const webkit_glue::FormData& form,
const base::TimeTicks& timestamp);
void OnFormsSeen(const std::vector<webkit_glue::FormData>& forms,
@@ -217,6 +207,9 @@
// |submitted_form|.
void DeterminePossibleFieldTypesForUpload(FormStructure* submitted_form);
+ // Imports the form data, submitted by the user, into |personal_data_|.
+ void ImportFormData(const FormStructure& submitted_form);
+
// If |initial_interaction_timestamp_| is unset or is set to a later time than
// |interaction_timestamp|, updates the cached timestamp. The latter check is
// needed because IPC messages can arrive out of order.
