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

BUG=None
TEST=chrome_frame_unittests --gtest_filter=Ready*
Review URL: http://codereview.chromium.org/6314016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@72366 0039d316-1c4b-4281-b951-d872f2087c98

chrome/app/cf_resources.rc

@@ -40,9 +40,9 @@ IDD_CHROME_FRAME_READY_PROMPT DIALOGEX 0, 0, 446, 22
 STYLE DS_SETFONT | DS_FIXEDSYS | WS_CHILD | WS_BORDER
 FONT 8, "MS Shell Dlg", 400, 0, 0x0
 BEGIN
-    LTEXT           "This site recommends the Google Chrome Frame add-on for Internet Explorer. Would you like to enable it now? No download or browser restart is required.",IDC_PROMPT_MESSAGE,34,3,254,16
-    ICON            IDI_CHROME_FRAME_ICON,IDC_PROMPT_ICON,7,1,21,20
-    DEFPUSHBUTTON   "&Yes",IDACTIVATE,287,4,50,14
-    PUSHBUTTON      "Remind me &Later",IDLATER,340,4,66,14
-    PUSHBUTTON      "&No",IDNEVER,409,4,34,14
-END
+    LTEXT           "This site performs better with Google Chrome Frame.",IDC_PROMPT_MESSAGE,34,3,198,16
+    LTEXT           "Learn more.",IDC_PROMPT_LINK,300,3,42,16
+    ICON            10750,IDC_PROMPT_ICON,7,1,20,20
+    DEFPUSHBUTTON   "Enable",IDACTIVATE,349,4,50,14
+    PUSHBUTTON      "Ignore",IDNEVER,409,4,34,14
+END
\ No newline at end of file

chrome/app/chromium_strings.grd

@@ -176,6 +176,12 @@ be available for now. -->
         <message name="IDS_CHROME_FRAME_MENU_ABOUT" desc="About Chrome Frame label">
           About Chromium Frame...
         </message>
+	<message name="IDS_CHROME_FRAME_READY_MODE_LEARN_MORE_URL" desc="Link to knowledge-base article about Chromium Frame Ready Mode">
+	  http://www.google.com/support/chrome/bin/answer.py?hl=[GRITLANGCODE]&amp;answer=161796
+	</message>
+	<message name="IDS_CHROME_FRAME_READY_MODE_PROMPT" desc="Ready Mode prompt message">
+	  This site recommends Chromium Frame (already installed).
+	</message>
       </if>
       <message name="IDS_INSTANT_OPT_IN_TITLE" desc="Title of the instant opt-in dialog">
         Instant

chrome/app/google_chrome_strings.grd

@@ -205,6 +205,12 @@ Chrome supports. -->
         <message name="IDS_CHROME_FRAME_MENU_ABOUT" desc="About Chrome Frame label">
           About Chrome Frame...
         </message>
+	<message name="IDS_CHROME_FRAME_READY_MODE_LEARN_MORE_URL" desc="Link to knowledge-base article about Chrome Frame Ready Mode">
+	  http://www.google.com/support/chrome/bin/answer.py?hl=[GRITLANGCODE]&amp;answer=161796
+	</message>
+	<message name="IDS_CHROME_FRAME_READY_MODE_PROMPT" desc="Ready Mode prompt message">
+	  This site recommends Google Chrome Frame (already installed).
+	</message>
       </if>
       <message name="IDS_INSTANT_OPT_IN_TITLE" desc="Title of the instant opt-in dialog">
         Google Chrome Instant

chrome_frame/chrome_frame.gyp

@@ -872,6 +872,7 @@
         'ready_mode/internal/ready_prompt_window.h',
         'ready_mode/internal/registry_ready_mode_state.cc',
         'ready_mode/internal/registry_ready_mode_state.h',
+        'ready_mode/internal/url_launcher.h',
         'ready_mode/ready_mode.cc',
         'ready_mode/ready_mode.h',
         'register_bho.rgs',

chrome_frame/ready_mode/internal/ready_prompt_content.cc

@@ -4,15 +4,15 @@
 
 #include "chrome_frame/ready_mode/internal/ready_prompt_content.h"
 
-#include <atlbase.h>
-#include <atlwin.h>
-
 #include "base/logging.h"
 #include "chrome_frame/ready_mode/internal/ready_mode_state.h"
 #include "chrome_frame/ready_mode/internal/ready_prompt_window.h"
+#include "chrome_frame/ready_mode/internal/url_launcher.h"
 
-ReadyPromptContent::ReadyPromptContent(ReadyModeState* ready_mode_state)
-    : ready_mode_state_(ready_mode_state) {
+ReadyPromptContent::ReadyPromptContent(ReadyModeState* ready_mode_state,
+                                       UrlLauncher* url_launcher)
+    : ready_mode_state_(ready_mode_state),
+      url_launcher_(url_launcher) {
 }
 
 ReadyPromptContent::~ReadyPromptContent() {
@@ -26,10 +26,11 @@ ReadyPromptContent::~ReadyPromptContent() {
 bool ReadyPromptContent::InstallInFrame(Frame* frame) {
   DCHECK(window_ == NULL);
   DCHECK(ready_mode_state_ != NULL);
+  DCHECK(url_launcher_ != NULL);
 
-  // The window owns itself upon call to Initialize.
-  ReadyPromptWindow* new_window_ = new ReadyPromptWindow();
-  window_ = new_window_->Initialize(frame, ready_mode_state_.release());
+  // Pass ownership of our ready_mode_state_ and url_launcher_ to the window.
+  window_ = ReadyPromptWindow::CreateInstance(
+      frame, ready_mode_state_.release(), url_launcher_.release());
 
   return window_ != NULL;
 }

chrome_frame/ready_mode/internal/ready_prompt_content.h

@@ -13,14 +13,18 @@
 
 class ReadyModeState;
 class ReadyPromptWindow;
+class UrlLauncher;
 
 // Encapsulates the Ready Mode prompt inviting users to permanently activate
 // Chrome Frame, temporarily disable Ready Mode, or permanently disable Ready
 // Mode.
 class ReadyPromptContent : public InfobarContent {
  public:
-  explicit ReadyPromptContent(ReadyModeState* ready_mode_state);
-  ~ReadyPromptContent();
+  // Takes ownership of the ReadyModeState and UrlLauncher instances, which
+  // will be freed upon destruction of the ReadyPromptContent.
+  ReadyPromptContent(ReadyModeState* ready_mode_state,
+                     UrlLauncher* url_launcher);
+  virtual ~ReadyPromptContent();
 
   // InfobarContent implementation
   virtual bool InstallInFrame(Frame* frame);
@@ -30,6 +34,7 @@ class ReadyPromptContent : public InfobarContent {
  private:
   base::WeakPtr<ReadyPromptWindow> window_;
   scoped_ptr<ReadyModeState> ready_mode_state_;
+  scoped_ptr<UrlLauncher> url_launcher_;
 
   DISALLOW_COPY_AND_ASSIGN(ReadyPromptContent);
 };  // class ReadyPromptContent

chrome_frame/ready_mode/internal/ready_prompt_window.cc

@@ -4,33 +4,63 @@
 
 #include "chrome_frame/ready_mode/internal/ready_prompt_window.h"
 
+#include <atlctrls.h>
+#include <Shellapi.h>  // Must appear before atlctrlx.h
+
+// These seem to be required by atlctrlx?
+template<class A>
+A min(A const& a, A const& b) { return a < b ? a : b; }
+
+template<class A>
+A max(A const& a, A const& b) { return a > b ? a : b; }
+
+#include <atlctrlx.h>
+
 #include "base/compiler_specific.h"
+#include "base/win/scoped_bstr.h"
+#include "base/win/scoped_comptr.h"
 #include "chrome_frame/ready_mode/internal/ready_mode_state.h"
+#include "chrome_frame/ready_mode/internal/url_launcher.h"
+#include "chrome_frame/simple_resource_loader.h"
+#include "grit/chromium_strings.h"
 
-ReadyPromptWindow::ReadyPromptWindow()
-    : frame_(NULL),
+ReadyPromptWindow::ReadyPromptWindow(
+    InfobarContent::Frame* frame, ReadyModeState* ready_mode_state,
+    UrlLauncher* url_launcher)
+    : frame_(frame),
+      ready_mode_state_(ready_mode_state),
+      url_launcher_(url_launcher),
       weak_ptr_factory_(ALLOW_THIS_IN_INITIALIZER_LIST(this)) {
 }
+ReadyPromptWindow::~ReadyPromptWindow() {
+}
 
-base::WeakPtr<ReadyPromptWindow> ReadyPromptWindow::Initialize(
-    InfobarContent::Frame* frame, ReadyModeState* ready_mode_state) {
+base::WeakPtr<ReadyPromptWindow> ReadyPromptWindow::CreateInstance(
+    InfobarContent::Frame* frame, ReadyModeState* ready_mode_state,
+    UrlLauncher* url_launcher) {
   DCHECK(frame != NULL);
-  DCHECK(frame_ == NULL);
   DCHECK(ready_mode_state != NULL);
-  DCHECK(ready_mode_state_ == NULL);
+  DCHECK(url_launcher != NULL);
 
-  frame_ = frame;
-  ready_mode_state_.reset(ready_mode_state);
+  base::WeakPtr<ReadyPromptWindow> instance((new ReadyPromptWindow(
+      frame, ready_mode_state, url_launcher))->weak_ptr_factory_.GetWeakPtr());
 
-  DCHECK(!IsWindow());
+  DCHECK(!instance->IsWindow());
 
-  if (Create(frame->GetFrameWindow()) == NULL) {
+  if (instance->Create(frame->GetFrameWindow()) == NULL) {
     DPLOG(ERROR) << "Failed to create HWND for ReadyPromptWindow.";
-    delete this;
     return base::WeakPtr<ReadyPromptWindow>();
   }
 
-  return weak_ptr_factory_.GetWeakPtr();
+  // Subclass the "Learn more." text to make it behave like a link. Clicks are
+  // routed to OnLearnMore().
+  CWindow rte = instance->GetDlgItem(IDC_PROMPT_LINK);
+  instance->link_.reset(new CHyperLink());
+  instance->link_->SubclassWindow(rte);
+  instance->link_->SetHyperLinkExtendedStyle(HLINK_NOTIFYBUTTON,
+                                             HLINK_NOTIFYBUTTON);
+
+  return instance;
 }
 
 void ReadyPromptWindow::OnDestroy() {
@@ -69,6 +99,14 @@ LRESULT ReadyPromptWindow::OnNo(WORD /*wNotifyCode*/,
   return 0;
 }
 
+LRESULT ReadyPromptWindow::OnLearnMore(WORD /*wParam*/,
+                                       LPNMHDR /*lParam*/,
+                                       BOOL& /*bHandled*/) {
+  url_launcher_->LaunchUrl(SimpleResourceLoader::Get(
+      IDS_CHROME_FRAME_READY_MODE_LEARN_MORE_URL));
+  return 0;
+}
+
 void ReadyPromptWindow::OnFinalMessage(HWND) {
   delete this;
 }

chrome_frame/ready_mode/internal/ready_prompt_window.h

@@ -14,11 +14,17 @@
 
 #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;
+class UrlLauncher;
+
+namespace WTL {
+class CHyperLink;
+}  // namespace WTL
 
 // Implements a dialog with text and buttons inviting the user to permanently
 // activate the product or temporarily/permanently disable Ready Mode.
@@ -28,39 +34,49 @@ class ReadyPromptWindow
  public:
   enum { IDD = IDD_CHROME_FRAME_READY_PROMPT };
 
-  ReadyPromptWindow();
-  ~ReadyPromptWindow() {}
-
-  // Initializes the dialog for display in the provided frame. The
+  // Creates and initializes a dialog for display in the provided frame. The
   // ReadyModeState will be invoked to capture the user's response, if any.
+  // The UrlLauncher 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.
-  base::WeakPtr<ReadyPromptWindow> Initialize(InfobarContent::Frame* frame,
-                                              ReadyModeState* ready_mode_state);
+  // In either case, takes ownership of the ReadyModeState and UrlLauncher, but
+  // not the frame.
+  static base::WeakPtr<ReadyPromptWindow> CreateInstance(
+      InfobarContent::Frame* frame,
+      ReadyModeState* ready_mode_state,
+      UrlLauncher* url_launcher);
 
   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:
+  // Call CreateInstance() to get an initialized ReadyPromptWindow.
+  ReadyPromptWindow(InfobarContent::Frame* frame,
+                    ReadyModeState* ready_mode_state,
+                    UrlLauncher* url_launcher);
+
+  // The ReadyPromptWindow manages its own destruction.
+  virtual ~ReadyPromptWindow();
+
   // Event handlers.
   void OnDestroy();
   BOOL OnInitDialog(CWindow wndFocus, LPARAM lInitParam);
@@ -72,6 +88,7 @@ class ReadyPromptWindow
                           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 +96,8 @@ class ReadyPromptWindow
 
   InfobarContent::Frame* frame_;  // Not owned by this instance
   scoped_ptr<ReadyModeState> ready_mode_state_;
+  scoped_ptr<WTL::CHyperLink> link_;
+  scoped_ptr<UrlLauncher> url_launcher_;
 
   base::WeakPtrFactory<ReadyPromptWindow> weak_ptr_factory_;
   DISALLOW_COPY_AND_ASSIGN(ReadyPromptWindow);

chrome_frame/ready_mode/internal/url_launcher.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_URL_LAUNCHER_H_
+#define CHROME_FRAME_READY_MODE_INTERNAL_URL_LAUNCHER_H_
+#pragma once
+
+// Implements the launching of a new browser window/tab. The
+// ReadyPromptContent invokes this, in response to user action, to display
+// documentation about Ready Mode.
+class UrlLauncher {
+ public:
+  virtual ~UrlLauncher() {}
+
+  // Displays the specified URL in a new window or tab.
+  virtual void LaunchUrl(const std::wstring& url) = 0;
+};  // class UrlHelper
+
+#endif  // CHROME_FRAME_READY_MODE_INTERNAL_URL_LAUNCHER_H_

chrome_frame/ready_mode/ready_mode.cc

@@ -21,6 +21,7 @@
 #include "chrome_frame/ready_mode/internal/ready_mode_web_browser_adapter.h"
 #include "chrome_frame/ready_mode/internal/ready_prompt_content.h"
 #include "chrome_frame/ready_mode/internal/registry_ready_mode_state.h"
+#include "chrome_frame/ready_mode/internal/url_launcher.h"
 #include "chrome_frame/utils.h"
 
 namespace {
@@ -90,6 +91,33 @@ class BrowserObserver : public ReadyModeWebBrowserAdapter::Observer {
   DISALLOW_COPY_AND_ASSIGN(BrowserObserver);
 };  // class BrowserObserver
 
+// Implements launching of a URL in an instance of IWebBrowser2.
+class UrlLauncherImpl : public UrlLauncher {
+ public:
+  explicit UrlLauncherImpl(IWebBrowser2* web_browser);
+
+  // UrlLauncher implementation
+  void LaunchUrl(const std::wstring& url);
+
+ private:
+  base::win::ScopedComPtr<IWebBrowser2> web_browser_;
+};  // class UrlLaucherImpl
+
+UrlLauncherImpl::UrlLauncherImpl(IWebBrowser2* web_browser) {
+  DCHECK(web_browser);
+  web_browser_ = web_browser;
+}
+
+void UrlLauncherImpl::LaunchUrl(const std::wstring& url) {
+  VARIANT flags = { VT_I4 };
+  V_I4(&flags) = navOpenInNewWindow;
+  base::win::ScopedBstr location(url.c_str());
+
+  HRESULT hr = web_browser_->Navigate(location, &flags, NULL, NULL, NULL);
+  DLOG_IF(ERROR, FAILED(hr)) << "Failed to invoke Navigate on IWebBrowser2. "
+                             << "Error: " << hr;
+}
+
 StateObserver::StateObserver(
     const base::WeakPtr<BrowserObserver>& ready_mode_ui)
     : ready_mode_ui_(ready_mode_ui) {
@@ -200,9 +228,12 @@ void BrowserObserver::ShowPrompt() {
         base::TimeDelta::FromMinutes(kTemporaryDeclineDurationMinutes),
         ready_mode_state_observer.release()));
 
+    // Owned by infobar_content
+    scoped_ptr<UrlLauncher> url_launcher(new UrlLauncherImpl(web_browser_));
+
     // Owned by infobar_manager
-    scoped_ptr<InfobarContent> infobar_content(
-        new ReadyPromptContent(ready_mode_state.release()));
+    scoped_ptr<InfobarContent> infobar_content(new ReadyPromptContent(
+        ready_mode_state.release(), url_launcher.release()));
 
     infobar_manager->Show(infobar_content.release(), TOP_INFOBAR);
   }

chrome_frame/test/ready_mode_unittest.cc

@@ -18,6 +18,7 @@
 #include "chrome_frame/ready_mode/internal/ready_prompt_content.h"
 #include "chrome_frame/ready_mode/internal/ready_prompt_window.h"
 #include "chrome_frame/ready_mode/internal/registry_ready_mode_state.h"
+#include "chrome_frame/ready_mode/internal/url_launcher.h"
 #include "chrome_frame/simple_resource_loader.h"
 #include "chrome_frame/test/chrome_frame_test_utils.h"
 
@@ -97,6 +98,12 @@ class MockReadyModeState : public ReadyModeState {
   MOCK_METHOD0(AcceptChromeFrame, void(void));
 };  // class MockReadyModeState
 
+class MockUrlLauncher : public UrlLauncher {
+ public:
+  // UrlLauncher implementation
+  MOCK_METHOD1(LaunchUrl, void(const std::wstring& url));
+};  // class MockUrlLauncher
+
 }  // namespace
 
 class ReadyPromptTest : public testing::Test {
@@ -126,8 +133,10 @@ class ReadyPromptWindowTest : public ReadyPromptTest {
 
     // owned by ReadyPromptWindow
     state_ = new MockReadyModeState();
-    ready_prompt_window_ = (new ReadyPromptWindow())->Initialize(&frame_,
-                                                                 state_);
+    url_launcher_ = new MockUrlLauncher();
+
+    ready_prompt_window_ = ReadyPromptWindow::CreateInstance(
+        &frame_, state_, url_launcher_);
 
     ASSERT_TRUE(ready_prompt_window_ != NULL);
     RECT position = {0, 0, 800, 39};
@@ -137,6 +146,7 @@ class ReadyPromptWindowTest : public ReadyPromptTest {
 
  protected:
   MockReadyModeState* state_;
+  MockUrlLauncher* url_launcher_;
   base::WeakPtr<ReadyPromptWindow> ready_prompt_window_;
 };  // class ReadyPromptWindowTest
 
@@ -212,7 +222,10 @@ class ReadyPromptWindowButtonTest : public ReadyPromptWindowTest {
 TEST_F(ReadyPromptTest, ReadyPromptContentTest) {
   // owned by ReadyPromptContent
   MockReadyModeState* state = new MockReadyModeState();
-  scoped_ptr<ReadyPromptContent> content_(new ReadyPromptContent(state));
+  MockUrlLauncher* url_launcher = new MockUrlLauncher();
+
+  scoped_ptr<ReadyPromptContent> content_(new ReadyPromptContent(state,
+                                                                 url_launcher));
 
   content_->InstallInFrame(&frame_);
 
@@ -255,21 +268,15 @@ TEST_F(ReadyPromptWindowTest, Destroy) {
   ready_prompt_window_->DestroyWindow();
 }
 
-TEST_F(ReadyPromptWindowButtonTest, ClickYes) {
+TEST_F(ReadyPromptWindowButtonTest, ClickEnable) {
   EXPECT_CALL(*state_, AcceptChromeFrame());
-  ASSERT_TRUE(ClickOnCaption(L"&Yes"));
-  RunUntilCloseInfobar();
-}
-
-TEST_F(ReadyPromptWindowButtonTest, ClickRemindMeLater) {
-  EXPECT_CALL(*state_, TemporarilyDeclineChromeFrame());
-  ASSERT_TRUE(ClickOnCaption(L"Remind me &Later"));
+  ASSERT_TRUE(ClickOnCaption(L"Enable"));
   RunUntilCloseInfobar();
 }
 
-TEST_F(ReadyPromptWindowButtonTest, ClickNo) {
+TEST_F(ReadyPromptWindowButtonTest, ClickIgnore) {
   EXPECT_CALL(*state_, PermanentlyDeclineChromeFrame());
-  ASSERT_TRUE(ClickOnCaption(L"&No"));
+  ASSERT_TRUE(ClickOnCaption(L"Ignore"));
   RunUntilCloseInfobar();
 }