TestOverrideEncoding hanging is because TabProxy::WaitForNavigation can not get reply if current last last_navigation_time for tab _tracker is great than the input last_navigation_time.

See AutomationProvider::WaitForNavigation, it is handled by IPC_MESSAGE_HANDLER_DELAY_REPLY, which means the message handler need to send the reply message by itself.
According to current WaitForNavigation logic, if current last last_navigation_time for tab _tracker is less than the input last_navigation_time, the replay will be sent by NavigationNotificationObserver. Otherwise, the reply will never be sent.
So if somehow the test machine is slow, the navigation has happened before calling AutomationProvider::WaitForNavigation, the caller will never get reply. That is why TestOverrideEncoding hangs sometimes.

Please refer to http://chrome-svn/viewvc/chrome?view=rev&revision=9585 to see how the logic was changed before.

The problem also happened on other two places. 

BUG=23121
TEST=BrowserEncodingTest.TestOverrideEncoding

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

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

2 files changed, 9 insertions, 1 deletions

diff --git a/chrome/browser/automation/automation_provider.cc b/chrome/browser/automation/automation_provider.cc
index 0f77ad3..f7fef6b 100644
--- a/chrome/browser/automation/automation_provider.cc
+++ b/chrome/browser/automation/automation_provider.cc
@@ -1346,6 +1346,7 @@ void AutomationProvider::CloseTab(int tab_handle,
   }
 
   AutomationMsg_CloseTab::WriteReplyParams(reply_message, false);
+  Send(reply_message);
 }
 
 void AutomationProvider::CloseBrowser(int browser_handle,
@@ -1747,6 +1748,12 @@ void AutomationProvider::ClickSSLInfoBarLink(int handle,
       }
     }
   }
+
+  // This "!wait_for_navigation || !success condition" logic looks suspicious.
+  // It will send a failure message when success is true but
+  // |wait_for_navigation| is false.
+  // TODO(phajdan.jr): investgate whether the reply param (currently
+  // AUTOMATION_MSG_NAVIGATION_ERROR) should depend on success.
   if (!wait_for_navigation || !success)
     AutomationMsg_ClickSSLInfoBarLink::WriteReplyParams(
         reply_message, AUTOMATION_MSG_NAVIGATION_ERROR);
@@ -1768,6 +1775,7 @@ void AutomationProvider::WaitForNavigation(int handle,
     AutomationMsg_WaitForNavigation::WriteReplyParams(reply_message,
         controller == NULL ? AUTOMATION_MSG_NAVIGATION_ERROR :
                              AUTOMATION_MSG_NAVIGATION_SUCCESS);
+    Send(reply_message);
     return;
   }
 
diff --git a/chrome/browser/browser_encoding_uitest.cc b/chrome/browser/browser_encoding_uitest.cc
index 7dec5b8..980aa30 100644
--- a/chrome/browser/browser_encoding_uitest.cc
+++ b/chrome/browser/browser_encoding_uitest.cc
@@ -115,7 +115,7 @@ TEST_F(BrowserEncodingTest, TestEncodingAliasMapping) {
 // We are disabling this test on MacOS and Linux because on those platforms
 // AutomationProvider::OverrideEncoding is not implemented yet.
 // TODO(port): Enable when encoding-related parts of Browser are ported.
-TEST_F(BrowserEncodingTest, DISABLED_TestOverrideEncoding) {
+TEST_F(BrowserEncodingTest, TestOverrideEncoding) {
   const char* const kTestFileName = "gb18030_with_iso88591_meta.html";
   const char* const kExpectedFileName =
       "expected_gb18030_saved_from_iso88591_meta.html";