Integrate SMS service with Desktop iOS promotion

chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_controller.h

Index: chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_controller.h
diff --git a/chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_controller.h b/chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_controller.h
index c167041e9d958f3e1eeac350cf611fdbab3a8c0e..d6821de7e6272e82cac7fa7a071b0268fdd0d659 100644
--- a/chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_controller.h
+++ b/chrome/browser/ui/desktop_ios_promotion/desktop_ios_promotion_controller.h
@@ -5,28 +5,68 @@
 #ifndef CHROME_BROWSER_UI_DESKTOP_IOS_PROMOTION_DESKTOP_IOS_PROMOTION_CONTROLLER_H_
 #define CHROME_BROWSER_UI_DESKTOP_IOS_PROMOTION_DESKTOP_IOS_PROMOTION_CONTROLLER_H_
 
+#include <memory>
+#include <string>
+
 #include "base/macros.h"
+#include "chrome/browser/ui/desktop_ios_promotion/sms_service.h"
 
 namespace desktop_ios_promotion {
 enum class PromotionEntryPoint;
 }
 
+class DesktopIOSPromotion;
+class Profile;
+class PrefService;
+
 // This class provides data to the Desktop to mobile promotion and control the
 // promotion actions.
 class DesktopIOSPromotionController {
  public:
-  // Should be instantiated on the UI thread.
-  DesktopIOSPromotionController();
+  // Must be instantiated on the UI thread.
+  DesktopIOSPromotionController(
+      Profile* profile,
+      desktop_ios_promotion::PromotionEntryPoint entry_point);
   ~DesktopIOSPromotionController();
 
+  // Returns the current promotion entry point.
+  desktop_ios_promotion::PromotionEntryPoint GetEntryPoint();
+
+  void SetPromotionView(DesktopIOSPromotion* promotion);

[sky, 2017/02/17 00:57:22]

I find it confusing that this function is called SetPromotionView, but the class
it takes isn't a view.

[sky, 2017/02/17 00:57:23]

Document ownership. Can you pass the view into the constructor rather than a
setter? Less error prone that way.

[mrefaat, 2017/02/17 04:31:49]

Acknowledged.

[mrefaat, 2017/02/17 04:31:49]

It's not possible as the controller is created before the view, and view takes
the controller on its constructor then set it self in the controller.

[mrefaat, 2017/02/17 04:31:49]

I added weak pointer comment on the header file, but i prefer not to write
comment about the owner here, Should not care about which promotion entry point
created it.

[mrefaat, 2017/02/17 04:31:49]

Acknowledged.

+
   // Called by the view code when "Send SMS" button is clicked by the user.
   void OnSendSMSClicked();
 
+  // Called by the view code when the promotion is ready to show.
+  void OnPromotionShown();
+
   // Called by the view code when "No Thanks" button is clicked by the user.
   void OnNoThanksClicked();
 
-  // (TODO): If needed verify that functions that affect the UI are done
-  // on the same thread.
+  // Returns the Recovery phone number, returns empy string if the number is not
+  // set.
+  std::string GetUsersRecoveryPhoneNumber();
+
+ private:
+  // Updates the user's recovery phone number once the sms_service phone query
+  // returns a response.
+  void OnQueryPhoneNumber(SMSService::Request* request,

[sky, 2017/02/17 00:57:22]

optional: name this OnGotPhoneNumber to indicate what this means.

[mrefaat, 2017/02/17 04:31:49]

Done.

+                          bool success,
+                          const std::string& number);
+
+  // Callback that logs the result when sms_service send sms returns a response.
+  void OnSendSMS(SMSService::Request* request,
+                 bool success,
+                 const std::string& number);
+
+  PrefService* profile_prefs_;
+  const desktop_ios_promotion::PromotionEntryPoint entry_point_;
+  // Service used to send SMS to the user recovery phone number.
+  SMSService* sms_service_;
+  // User's recovery phone number, This is updated by the sms_service.

[sky, 2017/02/17 00:57:22]

'This' -> 'this'

[mrefaat, 2017/02/17 04:31:49]

Done.

+  std::string recovery_number_;
+  base::WeakPtrFactory<DesktopIOSPromotionController> weak_ptr_factory_;

[sky, 2017/02/17 00:57:23]

WeakPtrFactory should be your last member.

[mrefaat, 2017/02/17 04:31:49]

Done.

+  DesktopIOSPromotion* promotion_view_;

[sky, 2017/02/17 00:57:23]

You don't initialize this in the initializer list.

[mrefaat, 2017/02/17 04:31:49]

Acknowledged.

 
   DISALLOW_COPY_AND_ASSIGN(DesktopIOSPromotionController);
 };