Update the Ready Mode UI in response to feedback from UI leads, Chrome Frame ...

chrome_frame/ready_mode/internal/ready_prompt_window.h

Index: chrome_frame/ready_mode/internal/ready_prompt_window.h
===================================================================
--- chrome_frame/ready_mode/internal/ready_prompt_window.h	(revision 72310)
+++ chrome_frame/ready_mode/internal/ready_prompt_window.h	(working copy)
@@ -14,12 +14,19 @@
 
 #include "base/weak_ptr.h"
 #include "base/scoped_ptr.h"
+#include "base/win/scoped_comptr.h"
 #include "chrome_frame/infobars/infobar_content.h"
 #include "chrome_frame/resource.h"
 #include "grit/generated_resources.h"
 
 class ReadyModeState;
 
+namespace WTL {
+class CHyperLink;
+}  // namespace WTL
+
+interface IWebBrowser2;
+
 // Implements a dialog with text and buttons inviting the user to permanently
 // activate the product or temporarily/permanently disable Ready Mode.
 class ReadyPromptWindow
@@ -29,33 +36,37 @@
   enum { IDD = IDD_CHROME_FRAME_READY_PROMPT };
 
   ReadyPromptWindow();
-  ~ReadyPromptWindow() {}
+  virtual ~ReadyPromptWindow();

[robertshield, 2011/01/24 16:10:21]

It looks like this object shouldn't be created on the stack (given the delete
call in Initialize). As such, the destructor can be marked private to prevent
accidental stack creation. (More Effective C++, item 27).

[erikwright (departed), 2011/01/24 17:14:01]

Done.

 
   // Initializes the dialog for display in the provided frame. The
   // ReadyModeState will be invoked to capture the user's response, if any.
+  // The IWebBrowser2 may be used to launch a new tab containing a
+  // knowledge-base article about Ready Mode.
   //
   // Upon success, takes ownership of itself (to be deleted upon WM_DESTROY) and
   // returns a weak pointer to this dialog. Upon failure, returns a null weak
   // pointer and deletes self.
   //
   // In either case, takes ownership of the ReadyModeState, but not the frame.
+  // Holds a reference to web_browser until destruction.
   base::WeakPtr<ReadyPromptWindow> Initialize(InfobarContent::Frame* frame,
-                                              ReadyModeState* ready_mode_state);
+                                              ReadyModeState* ready_mode_state,
+                                              IWebBrowser2* web_browser);

[robertshield, 2011/01/24 16:10:21]

I should have made this comment on an earlier review, but since I'm here, I'll
nag now :-)

It looks like this object manages its own life-cycle. As such, having the caller
new() it up and then hand over ownership (or not) back to the instance in
Initialize feels awkward. 

Why not simply have a static factory method along the lines of

base::WeakPtr<ReadyPromptWindow> CreateReadyPromptWindow(...)

[erikwright (departed), 2011/01/24 17:14:01]

Done.

 
   BEGIN_MSG_MAP(InfobarWindow)
     MSG_WM_INITDIALOG(OnInitDialog)
     MSG_WM_DESTROY(OnDestroy)
+    NOTIFY_HANDLER(IDC_PROMPT_LINK, NM_CLICK, OnLearnMore)
     COMMAND_HANDLER(IDACTIVATE, BN_CLICKED, OnYes)
-    COMMAND_HANDLER(IDLATER, BN_CLICKED, OnRemindMeLater)
     COMMAND_HANDLER(IDNEVER, BN_CLICKED, OnNo)
     CHAIN_MSG_MAP(CDialogResize<ReadyPromptWindow>)
   END_MSG_MAP()
 
   BEGIN_DLGRESIZE_MAP(InfobarWindow)
     DLGRESIZE_CONTROL(IDACTIVATE, DLSZ_CENTER_Y | DLSZ_MOVE_X)
-    DLGRESIZE_CONTROL(IDLATER, DLSZ_CENTER_Y | DLSZ_MOVE_X)
     DLGRESIZE_CONTROL(IDNEVER, DLSZ_CENTER_Y | DLSZ_MOVE_X)
     DLGRESIZE_CONTROL(IDC_PROMPT_MESSAGE, DLSZ_SIZE_Y | DLSZ_SIZE_X)
+    DLGRESIZE_CONTROL(IDC_PROMPT_LINK, DLSZ_CENTER_Y | DLSZ_MOVE_X)
   END_DLGRESIZE_MAP()
 
   virtual void OnFinalMessage(HWND);
@@ -72,6 +83,7 @@
                           WORD wID,
                           HWND hWndCtl,
                           BOOL& bHandled);
+  LRESULT OnLearnMore(WORD wParam, LPNMHDR lParam, BOOL& bHandled);  // NOLINT
   LRESULT OnNo(WORD wNotifyCode,
                WORD wID,
                HWND hWndCtl,
@@ -79,6 +91,8 @@
 
   InfobarContent::Frame* frame_;  // Not owned by this instance
   scoped_ptr<ReadyModeState> ready_mode_state_;
+  scoped_ptr<WTL::CHyperLink> link_;
+  base::win::ScopedComPtr<IWebBrowser2, NULL> web_browser_;
 
   base::WeakPtrFactory<ReadyPromptWindow> weak_ptr_factory_;
   DISALLOW_COPY_AND_ASSIGN(ReadyPromptWindow);