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

chrome_frame/ready_mode/internal/ready_prompt_window.h

 // Copyright (c) 2010 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_FRAME_READY_MODE_INTERNAL_READY_PROMPT_WINDOW_H_
 #define CHROME_FRAME_READY_MODE_INTERNAL_READY_PROMPT_WINDOW_H_
 #pragma once
 
 #include <atlbase.h>
 #include <atlapp.h>
 #include <atlcrack.h>
 #include <atlframe.h>
 #include <atlwin.h>
 
 #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
     : public CDialogImpl<ReadyPromptWindow, CWindow>,
       public CDialogResize<ReadyPromptWindow> {
  public:
   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);
 
  private:
   // Event handlers.
   void OnDestroy();
   BOOL OnInitDialog(CWindow wndFocus, LPARAM lInitParam);
   LRESULT OnYes(WORD wNotifyCode,
                 WORD wID,
                 HWND hWndCtl,
                 BOOL& bHandled);
   LRESULT OnRemindMeLater(WORD wNotifyCode,
                           WORD wID,
                           HWND hWndCtl,
                           BOOL& bHandled);
+  LRESULT OnLearnMore(WORD wParam, LPNMHDR lParam, BOOL& bHandled);  // NOLINT
   LRESULT OnNo(WORD wNotifyCode,
                WORD wID,
                HWND hWndCtl,
                BOOL& bHandled);
 
   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);
 };  // class ReadyPromptWindow
 
 #endif  // CHROME_FRAME_READY_MODE_INTERNAL_READY_PROMPT_WINDOW_H_