mac: Prevent Address Book permissions dialog from erroneously appearing.

chrome/browser/ui/autofill/chrome_autofill_client.h

 // Copyright 2014 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 CHROME_BROWSER_UI_AUTOFILL_CHROME_AUTOFILL_CLIENT_H_
 #define CHROME_BROWSER_UI_AUTOFILL_CHROME_AUTOFILL_CLIENT_H_
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/i18n/rtl.h"
 #include "base/memory/weak_ptr.h"
 #include "components/autofill/core/browser/autofill_client.h"
 #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;
 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:
   virtual ~ChromeAutofillClient();
 
   // Called when the tab corresponding to |this| instance is activated.
   void TabActivated();
 
   // AutofillClient:
   virtual PersonalDataManager* GetPersonalDataManager() OVERRIDE;
@@ skipping 20 matching lines @@
       const std::vector<base::string16>& values,
       const std::vector<base::string16>& labels) OVERRIDE;
   virtual void HideAutofillPopup() OVERRIDE;
   virtual bool IsAutocompleteEnabled() OVERRIDE;
   virtual void DetectAccountCreationForms(
       const std::vector<autofill::FormStructure*>& forms) OVERRIDE;
   virtual void DidFillOrPreviewField(
       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;
 
   // Exposed for testing.
   AutofillDialogController* GetDialogControllerForTesting() {
     return dialog_controller_.get();
   }
   void SetDialogControllerForTesting(
       const base::WeakPtr<AutofillDialogController>& dialog_controller) {
     dialog_controller_ = dialog_controller;
   }
 
  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);
 };
 
 }  // namespace autofill
 
 #endif  // CHROME_BROWSER_UI_AUTOFILL_CHROME_AUTOFILL_CLIENT_H_