Abstract WebContentsObserver from Autofill shared code

components/autofill/browser/autofill_manager.h

Index: components/autofill/browser/autofill_manager.h
diff --git a/components/autofill/browser/autofill_manager.h b/components/autofill/browser/autofill_manager.h
index d164923c8e55a68e89ceec4297020cdad2b7bb89..486ffce4ea1b600e13f9e0a95c1e81eb88cc7c55 100644
--- a/components/autofill/browser/autofill_manager.h
+++ b/components/autofill/browser/autofill_manager.h
@@ -30,17 +30,16 @@
 #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 {
@@ -58,6 +57,7 @@ class PrefRegistrySyncable;
 
 namespace autofill {
 
+class AutofillDriver;
 class AutofillDataModel;
 class AutofillDownloadManager;
 class AutofillExternalDelegate;
@@ -75,8 +75,7 @@ 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 {
@@ -86,6 +85,7 @@ class AutofillManager : public content::WebContentsObserver,
 
   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);
@@ -144,16 +144,23 @@ class AutofillManager : public content::WebContentsObserver,
   // 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);
 
@@ -221,11 +228,6 @@ class AutofillManager : public content::WebContentsObserver,
       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(
@@ -343,6 +345,8 @@ class AutofillManager : public content::WebContentsObserver,
   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_;