Fix for bug 2932: Chrome should not trigger page reloading when turn off auto detect

In order to fix the bug I added new interfaces to allow resetting the override encoding (including a new render message). 

The new logic is as follows: 

-) If the user turns auto-detect OFF, nothing happens (as requested in the bug description) 
-) If the user turns auto-detect ON, then the page is reloaded with an empty override encoding 

I.e., turning auto-detect on resets a previous override setting. 
The reverse is not true, however: specifiying a new override setting will not turn auto-detect off. 


BUG=2932
TEST=do the following steps, using the test file (encoding-gb18030.htm) attached to comment 14 of the original bug http://crbug.com/2932 : 

1.) load the file while auto-detect is turned off 
-> garbage is displayed 
2.) turn auto-detect on 
-> proper Chinese text should appear 
-> in the encoding menu, Chinese encoding should be highlighted 
3.) turn auto-detect off 
-> proper Chinese text should remain 
-> in the encoding menu, Chinese encoding should remain highlighted 
4.) choose any other encoding 
-> garbage is again displayed (in the new encoding) 
5.) turn auto-detect on 
-> proper Chinese text should again appear 
-> in the encoding menu, Chinese should again be highlighted 

Original Review: http://codereview.chromium.org/173265
Patch by Roland Steiner (rolandsteiner@google.com)


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

8 files changed, 42 insertions, 8 deletions

diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc
index 981f8fd..0d8ac44 100644
--- a/chrome/browser/browser.cc
+++ b/chrome/browser/browser.cc
@@ -1017,8 +1017,15 @@ void Browser::Print() {
 void Browser::ToggleEncodingAutoDetect() {
   UserMetrics::RecordAction(L"AutoDetectChange", profile_);
   encoding_auto_detect_.SetValue(!encoding_auto_detect_.GetValue());
-  // Reload the page so we can try to auto-detect the charset.
-  Reload();
+  // If "auto detect" is turned on, then any current override encoding
+  // is cleared. This also implicitly performs a reload.
+  // OTOH, if "auto detect" is turned off, we don't change the currently
+  // active encoding.
+  if (encoding_auto_detect_.GetValue()) {
+    TabContents* contents = GetSelectedTabContents();
+    if (contents)
+      contents->reset_override_encoding();
+  }
 }
 
 void Browser::OverrideEncoding(int encoding_id) {
diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc
index 9253e87..bab8ac8 100644
--- a/chrome/browser/renderer_host/render_view_host.cc
+++ b/chrome/browser/renderer_host/render_view_host.cc
@@ -447,6 +447,10 @@ void RenderViewHost::SetPageEncoding(const std::string& encoding_name) {
   Send(new ViewMsg_SetPageEncoding(routing_id(), encoding_name));
 }
 
+void RenderViewHost::ResetPageEncodingToDefault() {
+  Send(new ViewMsg_ResetPageEncodingToDefault(routing_id()));
+}
+
 void RenderViewHost::SetAlternateErrorPageURL(const GURL& url) {
   Send(new ViewMsg_SetAltErrorPageURL(routing_id(), url));
 }
diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h
index 3a1c273..5be72a4 100644
--- a/chrome/browser/renderer_host/render_view_host.h
+++ b/chrome/browser/renderer_host/render_view_host.h
@@ -224,6 +224,9 @@ class RenderViewHost : public RenderWidgetHost,
   // Change the encoding of the page.
   void SetPageEncoding(const std::string& encoding);
 
+  // Reset any override encoding on the page and change back to default.
+  void ResetPageEncodingToDefault();
+
   // Change the alternate error page URL.  An empty GURL disables the use of
   // alternate error pages.
   void SetAlternateErrorPageURL(const GURL& url);
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index 30c29e9..ee1b5bc 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -234,6 +234,9 @@ class TabContents : public PageNavigator,
 
   const std::string& encoding() const { return encoding_; }
   void set_encoding(const std::string& encoding);
+  void reset_encoding() {
+    encoding_.clear();
+  }
 
   // Internal state ------------------------------------------------------------
 
@@ -581,6 +584,12 @@ class TabContents : public PageNavigator,
     set_encoding(encoding);
     render_view_host()->SetPageEncoding(encoding);
   }
+  // Remove any user-defined override encoding and reload by sending down
+  // ViewMsg_ResetPageEncodingToDefault to the renderer.
+  void reset_override_encoding() {
+    reset_encoding();
+    render_view_host()->ResetPageEncodingToDefault();
+  }
 
   void WindowMoveOrResizeStarted() {
     render_view_host()->WindowMoveOrResizeStarted();
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index ccea8b5..a2b99b5 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -335,6 +335,9 @@ IPC_BEGIN_MESSAGES(View)
   IPC_MESSAGE_ROUTED1(ViewMsg_SetPageEncoding,
                       std::string /*new encoding name*/)
 
+  // Reset encoding of page in the renderer back to default.
+  IPC_MESSAGE_ROUTED0(ViewMsg_ResetPageEncodingToDefault)
+
   // Requests the renderer to reserve a range of page ids.
   IPC_MESSAGE_ROUTED1(ViewMsg_ReservePageIDRange,
                       int  /* size_of_range */)
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index 099cd1e..f339309 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -373,6 +373,8 @@ void RenderView::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ViewMsg_DeterminePageText, OnDeterminePageText)
     IPC_MESSAGE_HANDLER(ViewMsg_Zoom, OnZoom)
     IPC_MESSAGE_HANDLER(ViewMsg_SetPageEncoding, OnSetPageEncoding)
+    IPC_MESSAGE_HANDLER(ViewMsg_ResetPageEncodingToDefault,
+                        OnResetPageEncodingToDefault)
     IPC_MESSAGE_HANDLER(ViewMsg_SetupDevToolsClient, OnSetupDevToolsClient)
     IPC_MESSAGE_HANDLER(ViewMsg_DownloadFavIcon, OnDownloadFavIcon)
     IPC_MESSAGE_HANDLER(ViewMsg_ScriptEvalRequest, OnScriptEvalRequest)
@@ -2659,6 +2661,11 @@ void RenderView::OnSetPageEncoding(const std::string& encoding_name) {
   webview()->SetPageEncoding(encoding_name);
 }
 
+void RenderView::OnResetPageEncodingToDefault() {
+  std::string no_encoding;
+  webview()->SetPageEncoding(no_encoding);
+}
+
 void RenderView::NavigateBackForwardSoon(int offset) {
   history_back_list_count_ += offset;
   history_forward_list_count_ -= offset;
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index f9d3784..5836b52 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -557,6 +557,7 @@ class RenderView : public RenderWidget,
   void OnDeterminePageText();
   void OnZoom(int function);
   void OnSetPageEncoding(const std::string& encoding_name);
+  void OnResetPageEncodingToDefault();
   void OnGetAllSavableResourceLinksForCurrentPage(const GURL& page_url);
   void OnGetSerializedHtmlDataForCurrentPageWithLocalLinks(
       const std::vector<GURL>& links,
diff --git a/webkit/glue/webview_impl.cc b/webkit/glue/webview_impl.cc
index a58b0c3..405a744 100644
--- a/webkit/glue/webview_impl.cc
+++ b/webkit/glue/webview_impl.cc
@@ -1419,12 +1419,12 @@ void WebViewImpl::SetPageEncoding(const std::string& encoding_name) {
   if (!page_.get())
     return;
 
-  if (!encoding_name.empty()) {
-    // only change override encoding, don't change default encoding
-    // TODO(brettw) use std::string for encoding names.
-    String new_encoding_name(webkit_glue::StdStringToString(encoding_name));
-    page_->mainFrame()->loader()->reloadWithOverrideEncoding(new_encoding_name);
-  }
+  // Only change override encoding, don't change default encoding.
+  // Note that the new encoding must be NULL if it isn't supposed to be set.
+  String new_encoding_name;
+  if (!encoding_name.empty())
+    new_encoding_name = webkit_glue::StdStringToString(encoding_name);
+  page_->mainFrame()->loader()->reloadWithOverrideEncoding(new_encoding_name);
 }
 
 // Return the canonical encoding name of current main webframe in webview.