mac: Prevent Address Book permissions dialog from erroneously appearing.

chrome/browser/ui/autofill/chrome_autofill_client.h

Index: chrome/browser/ui/autofill/chrome_autofill_client.h
diff --git a/chrome/browser/ui/autofill/chrome_autofill_client.h b/chrome/browser/ui/autofill/chrome_autofill_client.h
index 57a19de4aef4d6b53afd3b5bb42735c555df6272..6ab68db5b919547bdbcc0dc8b192806b48683f78 100644
--- a/chrome/browser/ui/autofill/chrome_autofill_client.h
+++ b/chrome/browser/ui/autofill/chrome_autofill_client.h
@@ -13,6 +13,10 @@
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/browser/web_contents_user_data.h"
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)

[erikchen, 2014/06/12 23:54:41]

This was a lot of #ifdef-ing. Let me know if you prefer a different solution.

[Ilya Sherman, 2014/06/14 01:18:51]

Would it make sense for the client to own a class that is a delegate, rather
than being one directly?  Then you would only need two #ifdefs: One for the
member variable, and one for the initialization of that member variable.

[erikchen, 2014/06/16 20:30:46]

That adds another layer of indirection to reduce the number of ifdefs in this
file by 3, which I do not think is a worthwhile trade..

+#include "chrome/browser/autofill/autofill_keystone_observer_mac_delegate.h"
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
+
 namespace content {
 struct FrameNavigateParams;
 struct LoadCommittedDetails;
@@ -22,12 +26,18 @@ class WebContents;
 namespace autofill {
 
 class AutofillDialogController;
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+class AutofillKeystoneObserverMac;
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)

[Ilya Sherman, 2014/06/14 01:18:51]

It's fine to forward-declare the class on all platforms, even if it's only
needed on one platform.  If you prefer to keep the #ifdefs, though, please move
this to be below the other forward-declarations.

[erikchen, 2014/06/16 20:30:46]

I've removed the ifdefs here.

 class AutofillPopupControllerImpl;
 struct FormData;
 
 // Chrome implementation of AutofillClient.
 class ChromeAutofillClient
     : public AutofillClient,
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+      public AutofillKeystoneObserverMacDelegate,
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
       public content::WebContentsUserData<ChromeAutofillClient>,
       public content::WebContentsObserver {
  public:
@@ -68,6 +78,12 @@ class ChromeAutofillClient
       const base::string16& autofilled_value,
       const base::string16& profile_full_name) OVERRIDE;
 
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // AutofillKeystoneObserverMacDelegate

[Ilya Sherman, 2014/06/14 01:18:51]

nit: Please add a trailing colon.

[erikchen, 2014/06/16 20:30:46]

Done.

+  virtual void OnKeystoneNotification(
+      keystone_glue::AutoupdateStatus status) OVERRIDE;
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
+
   // content::WebContentsObserver implementation.
   virtual void WebContentsDestroyed() OVERRIDE;
 
@@ -81,12 +97,23 @@ class ChromeAutofillClient
   }
 
  private:
+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  void AddKeystoneObserver();
+  void RemoveKeystoneObserver();
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
+
   explicit ChromeAutofillClient(content::WebContents* web_contents);
   friend class content::WebContentsUserData<ChromeAutofillClient>;
 
   content::WebContents* const web_contents_;
   base::WeakPtr<AutofillDialogController> dialog_controller_;
   base::WeakPtr<AutofillPopupControllerImpl> popup_controller_;

[Ilya Sherman, 2014/06/14 01:18:51]

nit: Please leave a blank line below this one.

[erikchen, 2014/06/16 20:30:46]

Done.

+#if defined(OS_MACOSX) && !defined(OS_IOS)
+  // The class of this property must remain a forward declaration, since the
+  // header file for the class requires ObjC++ to compile. This means that the
+  // pointer cannot be wrapped in a scoped_ptr.

[Ilya Sherman, 2014/06/14 01:18:51]

I'm pretty sure that scoped_ptrs work fine with forward-declared classes.

[erikchen, 2014/06/16 20:30:46]

I've updated the comment to further indicate why this must remain a not-smart
pointer.

+  AutofillKeystoneObserverMac* keystone_observer_;
+#endif  // defined(OS_MACOSX) && !defined(OS_IOS)
 
   DISALLOW_COPY_AND_ASSIGN(ChromeAutofillClient);
 };