Miscellaneous ChromeFrame code cleanup. The automation client class maintained GURL members holding the url and

referrer. We can stuff this information into the ChromeFrameLaunchParams member maintained by the client. This
reduces the complexity in the code related to detecting whether we can navigate up front or after chrome initialization.

The ChromeFrame ActiveX now launches the automation server in its implementation of IPersistPropertyBag::Load. This avoids
race conditions between launching the automation server and navigating to it via put_src.

Review URL: http://codereview.chromium.org/3038038

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

5 files changed, 32 insertions, 35 deletions

diff --git a/chrome_frame/chrome_active_document.cc b/chrome_frame/chrome_active_document.cc
index 1fac95b..5a649cc 100644
--- a/chrome_frame/chrome_active_document.cc
+++ b/chrome_frame/chrome_active_document.cc
@@ -969,18 +969,9 @@ bool ChromeActiveDocument::LaunchUrl(const ChromeFrameUrl& cf_url,
 
   DLOG(INFO) << "Url is " << url_;
 
-  // Initiate navigation before launching chrome so that the url will be
-  // cached and sent with launch settings.
   if (cf_url.attach_to_external_tab()) {
     dimensions_ = cf_url.dimensions();
     automation_client_->AttachExternalTab(cf_url.cookie());
-  } else if (!automation_client_->InitiateNavigation(utf8_url,
-                                                     referrer,
-                                                     is_privileged_)) {
-    DLOG(ERROR) << "Invalid URL: " << url_;
-    Error(L"Invalid URL");
-    url_.Reset();
-    return false;
   }
 
   if (is_automation_client_reused_)
diff --git a/chrome_frame/chrome_frame_activex.cc b/chrome_frame/chrome_frame_activex.cc
index 0e6eb21..eeffb23 100644
--- a/chrome_frame/chrome_frame_activex.cc
+++ b/chrome_frame/chrome_frame_activex.cc
@@ -409,8 +409,9 @@ HRESULT ChromeFrameActivex::IOleObject_SetClientSite(
 
     // Probe to see whether the host implements the privileged service.
     ScopedComPtr<IChromeFramePrivileged> service;
-    HRESULT service_hr = DoQueryService(SID_ChromeFramePrivileged, client_site,
-                                service.Receive());
+    HRESULT service_hr = DoQueryService(SID_ChromeFramePrivileged,
+                                        m_spClientSite,
+                                        service.Receive());
     if (SUCCEEDED(service_hr) && service) {
       // Does the host want privileged mode?
       boolean wants_privileged = false;
@@ -462,6 +463,7 @@ HRESULT ChromeFrameActivex::IOleObject_SetClientSite(
     if (!InitializeAutomation(profile_name, chrome_extra_arguments,
                               IsIEInPrivate(), true, GURL(utf8_url),
                               GURL())) {
+      DLOG(ERROR) << "Failed to navigate to url:" << utf8_url;
       return E_FAIL;
     }
   }
diff --git a/chrome_frame/chrome_frame_automation.cc b/chrome_frame/chrome_frame_automation.cc
index bed7be2..fdf5831 100644
--- a/chrome_frame/chrome_frame_automation.cc
+++ b/chrome_frame/chrome_frame_automation.cc
@@ -485,7 +485,9 @@ bool ChromeFrameAutomationClient::Initialize(
     const ChromeFrameLaunchParams& chrome_launch_params) {
   DCHECK(!IsWindow());
   chrome_frame_delegate_ = chrome_frame_delegate;
+
   chrome_launch_params_ = chrome_launch_params;
+
   ui_thread_id_ = PlatformThread::CurrentId();
 #ifndef NDEBUG
   // In debug mode give more time to work with a debugger.
@@ -523,10 +525,6 @@ bool ChromeFrameAutomationClient::Initialize(
   if (chrome_launch_params_.url.is_valid())
     navigate_after_initialization_ = false;
 
-  if (!navigate_after_initialization_) {
-    chrome_launch_params_.url = url_;
-  }
-
   proxy_factory_->GetAutomationServer(
       static_cast<ProxyFactory::LaunchDelegate*>(this),
       chrome_launch_params_, &automation_server_id_);
@@ -591,19 +589,22 @@ bool ChromeFrameAutomationClient::InitiateNavigation(
     return false;
   }
 
-  // Important: Since we will be using the referrer_ variable from a different
-  // thread, we need to force a new std::string buffer instance for the
-  // referrer_ GURL variable.  Otherwise we can run into strangeness when the
-  // GURL is accessed and it could result in a bad URL that can cause the
-  // referrer to be dropped or something worse.
-  referrer_ = GURL(referrer.c_str());
-  url_ = parsed_url;
-  navigate_after_initialization_ = false;
+  if (parsed_url != chrome_launch_params_.url) {
+    // Important: Since we will be using the referrer_ variable from a
+    // different thread, we need to force a new std::string buffer instance for
+    // the referrer_ GURL variable.  Otherwise we can run into strangeness when
+    // the GURL is accessed and it could result in a bad URL that can cause the
+    // referrer to be dropped or something worse.
+    chrome_launch_params_.referrer = GURL(referrer.c_str());
+    chrome_launch_params_.url = parsed_url;
 
-  if (is_initialized()) {
-    BeginNavigate(url_, referrer_);
-  } else {
-    navigate_after_initialization_ = true;
+    navigate_after_initialization_ = false;
+
+    if (is_initialized()) {
+      BeginNavigate(chrome_launch_params_.url, chrome_launch_params_.referrer);
+    } else {
+      navigate_after_initialization_ = true;
+    }
   }
 
   return true;
@@ -648,7 +649,8 @@ void ChromeFrameAutomationClient::BeginNavigate(const GURL& url,
   // Could be NULL if we failed to launch Chrome in LaunchAutomationServer()
   if (!automation_server_ || !tab_.get()) {
     DLOG(WARNING) << "BeginNavigate - can't navigate.";
-    ReportNavigationError(AUTOMATION_MSG_NAVIGATION_ERROR, url_.spec());
+    ReportNavigationError(AUTOMATION_MSG_NAVIGATION_ERROR,
+                          chrome_launch_params_.url.spec());
     return;
   }
 
@@ -674,7 +676,8 @@ void ChromeFrameAutomationClient::BeginNavigate(const GURL& url,
 void ChromeFrameAutomationClient::BeginNavigateCompleted(
     AutomationMsg_NavigationResponseValues result) {
   if (result == AUTOMATION_MSG_NAVIGATION_ERROR)
-     ReportNavigationError(AUTOMATION_MSG_NAVIGATION_ERROR, url_.spec());
+     ReportNavigationError(AUTOMATION_MSG_NAVIGATION_ERROR,
+                           chrome_launch_params_.url.spec());
 }
 
 void ChromeFrameAutomationClient::FindInPage(const std::wstring& search_string,
@@ -794,8 +797,10 @@ void ChromeFrameAutomationClient::CreateExternalTab() {
   DCHECK(IsWindow());
   DCHECK(automation_server_ != NULL);
 
-  // TODO(ananta)
-  // We should pass in the referrer for the initial navigation.
+  if (chrome_launch_params_.url.is_valid()) {
+    navigate_after_initialization_ = false;
+  }
+
   const IPC::ExternalTabSettings settings = {
     m_hWnd,
     gfx::Rect(),
@@ -917,7 +922,8 @@ void ChromeFrameAutomationClient::InitializeComplete(
     // If host specified destination URL - navigate. Apparently we do not use
     // accelerator table.
     if (navigate_after_initialization_) {
-      BeginNavigate(url_, referrer_);
+      BeginNavigate(chrome_launch_params_.url,
+                    chrome_launch_params_.referrer);
     }
   }
 
diff --git a/chrome_frame/chrome_frame_automation.h b/chrome_frame/chrome_frame_automation.h
index 77c82a3..b38d4df 100644
--- a/chrome_frame/chrome_frame_automation.h
+++ b/chrome_frame/chrome_frame_automation.h
@@ -321,8 +321,6 @@ class ChromeFrameAutomationClient
   HWND chrome_window_;
   scoped_refptr<TabProxy> tab_;
   ChromeFrameDelegate* chrome_frame_delegate_;
-  GURL url_;
-  GURL referrer_;
 
   // Handle to the underlying chrome window. This is a child of the external
   // tab window.
diff --git a/chrome_frame/test/data/initialize_hidden.html b/chrome_frame/test/data/initialize_hidden.html
index 6ca5536..4f353b9 100644
--- a/chrome_frame/test/data/initialize_hidden.html
+++ b/chrome_frame/test/data/initialize_hidden.html
@@ -57,7 +57,7 @@
           // page will detect and notify us back by sending us a message.
           // We set focus on a timeout as on IE it takes a while for the window
           // to become visible.
-          setTimeout(SetFocusToCF2, 100);
+          setTimeout(SetFocusToCF2, 1000);
         } catch(e) {
           appendStatus('Error setting focus to CF2. Error: ' + e.description);
           onFailure(g_test_name, g_test_id, 'CF2 focus() error');