Abstract WebContentsObserver from Autofill shared code

components/autofill/browser/autofill_manager.h

 // Copyright (c) 2012 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_AUTOFILL_BROWSER_AUTOFILL_MANAGER_H_
 #define COMPONENTS_AUTOFILL_BROWSER_AUTOFILL_MANAGER_H_
 
 #include <list>
 #include <map>
 #include <string>
@@ skipping 12 matching lines @@
 #include "components/autofill/browser/autocheckout_manager.h"
 #include "components/autofill/browser/autocomplete_history_manager.h"
 #include "components/autofill/browser/autofill_download.h"
 #include "components/autofill/browser/autofill_manager_delegate.h"
 #include "components/autofill/browser/field_types.h"
 #include "components/autofill/browser/form_structure.h"
 #include "components/autofill/browser/personal_data_manager.h"
 #include "components/autofill/common/autocheckout_status.h"
 #include "components/autofill/common/form_data.h"
 #include "components/autofill/common/forms_seen_state.h"
-#include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/ssl_status.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebFormElement.h"
 
 class GURL;
 
-struct ViewHostMsg_FrameNavigate_Params;
-
 namespace content {
 class RenderViewHost;
 class WebContents;
+struct FrameNavigateParams;
+struct LoadCommittedDetails;
 }
 
 namespace gfx {
 class Rect;
 class RectF;
 }
 
 namespace IPC {
 class Message;
 }
 
 namespace user_prefs {
 class PrefRegistrySyncable;
 }
 
 namespace autofill {
 
+class AutofillDriver;
 class AutofillDataModel;
 class AutofillDownloadManager;
 class AutofillExternalDelegate;
 class AutofillField;
 class AutofillProfile;
 class AutofillManagerDelegate;
 class AutofillManagerTestDelegate;
 class AutofillMetrics;
 class CreditCard;
 class FormStructureBrowserTest;
 
 struct FormData;
 struct FormFieldData;
 struct PasswordFormFillData;
 
 // Manages saving and restoring the user's personal information entered into web
 // forms.
-class AutofillManager : public content::WebContentsObserver,
-                        public AutofillDownloadManager::Observer,
+class AutofillManager : public AutofillDownloadManager::Observer,
                         public base::SupportsUserData::Data {
  public:
   enum AutofillDownloadManagerState {
     ENABLE_AUTOFILL_DOWNLOAD_MANAGER,
     DISABLE_AUTOFILL_DOWNLOAD_MANAGER,
   };
 
   static void CreateForWebContentsAndDelegate(
       content::WebContents* contents,

[Ilya Sherman, 2013/06/12 00:07:46]

nit: Does the WebContents still need to be passed in, or can we simply pass in
the driver?

[blundell, 2013/06/12 16:29:37]

Eliminated this method as part of making the AutofillManager owned by the
AutofillDriver instead of by the WebContents.

On 2013/06/12 00:07:46, Ilya Sherman wrote:

+      AutofillDriver* driver,
       autofill::AutofillManagerDelegate* delegate,
       const std::string& app_locale,
       AutofillDownloadManagerState enable_download_manager);
   static AutofillManager* FromWebContents(content::WebContents* contents);
 
   // Registers our Enable/Disable Autofill pref.
   static void RegisterUserPrefs(user_prefs::PrefRegistrySyncable* registry);
 
   // Set an external delegate.
   void SetExternalDelegate(AutofillExternalDelegate* delegate);
@@ skipping 38 matching lines @@
 
   autofill::AutofillManagerDelegate* delegate() const {
     return manager_delegate_;
   }
 
   const std::string& app_locale() const { return app_locale_; }
 
   // Only for testing.
   void SetTestDelegate(autofill::AutofillManagerTestDelegate* delegate);
 
+  // TODO(blundell): Move the logic in these methods into AutofillDriver and
+  // eliminate these methods.
+  virtual void DidNavigateMainFrame(
+      const content::LoadCommittedDetails& details,
+      const content::FrameNavigateParams& params);
+  virtual bool OnMessageReceived(const IPC::Message& message);
+
  protected:
   // Only test code should subclass AutofillManager.
-  AutofillManager(content::WebContents* web_contents,
+  AutofillManager(AutofillDriver* driver,
                   autofill::AutofillManagerDelegate* delegate,
                   const std::string& app_locale,
                   AutofillDownloadManagerState enable_download_manager);
   virtual ~AutofillManager();
 
   // Test code should prefer to use this constructor.
-  AutofillManager(content::WebContents* web_contents,
+  AutofillManager(AutofillDriver* driver,
                   autofill::AutofillManagerDelegate* delegate,
                   PersonalDataManager* personal_data);
 
   // Returns the value of the AutofillEnabled pref.
   virtual bool IsAutofillEnabled() const;
 
   // Uploads the form data to the Autofill server.
   virtual void UploadFormData(const FormStructure& submitted_form);
 
   // Reset cache.
@@ skipping 47 matching lines @@
   // server.  Returns false if this form is not relevant for Autofill.
   bool OnFormSubmitted(const FormData& form,
                        const base::TimeTicks& timestamp);
 
   // Tell the renderer the current interactive autocomplete finished.
   virtual void ReturnAutocompleteResult(
       WebKit::WebFormElement::AutocompleteResult result,
       const FormData& form_data);
 
  private:
-  // content::WebContentsObserver:
-  virtual void DidNavigateMainFrame(
-      const content::LoadCommittedDetails& details,
-      const content::FrameNavigateParams& params) OVERRIDE;
-  virtual bool OnMessageReceived(const IPC::Message& message) OVERRIDE;
 
   // AutofillDownloadManager::Observer:
   virtual void OnLoadedServerPredictions(
       const std::string& response_xml) OVERRIDE;
 
   void OnFormsSeen(const std::vector<FormData>& forms,
                    const base::TimeTicks& timestamp,
                    autofill::FormsSeenState state);
   void OnTextFieldDidChange(const FormData& form,
                             const FormFieldData& field,
@@ skipping 97 matching lines @@
   // |interaction_timestamp|, updates the cached timestamp.  The latter check is
   // needed because IPC messages can arrive out of order.
   void UpdateInitialInteractionTimestamp(
       const base::TimeTicks& interaction_timestamp);
 
   // Send our current field type predictions to the renderer. This is a no-op if
   // the appropriate command-line flag is not set.
   void SendAutofillTypePredictions(
       const std::vector<FormStructure*>& forms) const;
 
+  AutofillDriver* driver_;

[Ilya Sherman, 2013/06/12 00:07:46]

nit: Please document this variable, including its lifetime expectation (i.e.
that it is expected to outlive this class).

[blundell, 2013/06/12 16:29:37]

Done. Additionally, your comment made me realize that leaving the
AutofillManager as a WebContentsUserData was problematic, as WebContentsUserData
objects are destroyed in an arbitrary order. I've thus moved the ownership of
the AutofillManager to the AutofillDriverImpl in this CL instead of doing it in
a follow-up CL as I had originally planned.

On 2013/06/12 00:07:46, Ilya Sherman wrote:

+
   autofill::AutofillManagerDelegate* const manager_delegate_;
 
   std::string app_locale_;
 
   // The personal data manager, used to save and load personal data to/from the
   // web database.  This is overridden by the AutofillManagerTest.
   // Weak reference.
   // May be NULL.  NULL indicates OTR.
   PersonalDataManager* personal_data_;
 
@@ skipping 70 matching lines @@
   FRIEND_TEST_ALL_PREFIXES(AutofillMetricsTest,
                            UserHappinessFormLoadAndSubmission);
   FRIEND_TEST_ALL_PREFIXES(AutofillMetricsTest, UserHappinessFormInteraction);
 
   DISALLOW_COPY_AND_ASSIGN(AutofillManager);
 };
 
 }  // namespace autofill
 
 #endif  // COMPONENTS_AUTOFILL_BROWSER_AUTOFILL_MANAGER_H_