One more attempt at landing this CL.

There are now memory errors in the CLD library that cause memory crashers in reliability tests.
I will add reliability failures to the know_crashes.txt file while the memory errors are being fixed.

Original review:
http://codereview.chromium.org/492024/show

BUG=30662
TEST=Run the unit-tests.
TBR=brettw

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

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

16 files changed, 160 insertions, 84 deletions

diff --git a/chrome/browser/browser_browsertest.cc b/chrome/browser/browser_browsertest.cc
index 5dcfe57..35a04da5 100644
--- a/chrome/browser/browser_browsertest.cc
+++ b/chrome/browser/browser_browsertest.cc
@@ -292,3 +292,33 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, FaviconOfOnloadRedirectToAnchorPage) {
       controller().GetActiveEntry();
   EXPECT_EQ(expected_favicon_url.spec(), entry->favicon().url().spec());
 }
+
+// Tests that the CLD (Compact Language Detection) works properly.
+IN_PROC_BROWSER_TEST_F(BrowserTest, PageLanguageDetection) {
+  static const wchar_t kDocRoot[] = L"chrome/test/data";
+  scoped_refptr<HTTPTestServer> server(
+        HTTPTestServer::CreateServer(kDocRoot, NULL));
+  ASSERT_TRUE(NULL != server.get());
+
+  TabContents* current_tab = browser()->GetSelectedTabContents();
+
+  // Navigate to a page in English.
+  ui_test_utils::NavigateToURL(
+      browser(), GURL(server->TestServerPage("files/english_page.html")));
+  NavigationEntry* entry = current_tab->controller().GetActiveEntry();
+  ASSERT_TRUE(NULL != entry);
+  EXPECT_TRUE(entry->language().empty());
+  std::string lang = ui_test_utils::WaitForLanguageDetection(current_tab);
+  EXPECT_EQ("en", lang);
+  EXPECT_EQ("en", entry->language());
+
+  // Now navigate to a page in French.
+  ui_test_utils::NavigateToURL(
+      browser(), GURL(server->TestServerPage("files/french_page.html")));
+  entry = current_tab->controller().GetActiveEntry();
+  ASSERT_TRUE(NULL != entry);
+  EXPECT_TRUE(entry->language().empty());
+  lang = ui_test_utils::WaitForLanguageDetection(current_tab);
+  EXPECT_EQ("fr", lang);
+  EXPECT_EQ("fr", entry->language());
+}
diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc
index 8e6bf2f..36cf400 100644
--- a/chrome/browser/extensions/extension_tabs_module.cc
+++ b/chrome/browser/extensions/extension_tabs_module.cc
@@ -855,26 +855,49 @@ bool DetectTabLanguageFunction::RunImpl() {
       return false;
   }
 
-  // Figure out what language |contents| contains. This sends an async call via
-  // the browser to the renderer to determine the language of the tab the
-  // renderer has. The renderer sends back the language of the tab after the
-  // tab loads (it may be delayed) to the browser, which in turn notifies this
-  // object that the language has been received.
-  contents->GetPageLanguage();
+  AddRef();  // Balanced in GotLanguage()
+
+  NavigationEntry* entry = contents->controller().GetActiveEntry();
+  if (entry) {
+    std::string language = entry->language();
+    if (!language.empty()) {
+      // Delay the callback invocation until after the current JS call has
+      // returned.
+      MessageLoop::current()->PostTask(FROM_HERE, NewRunnableMethod(
+          this, &DetectTabLanguageFunction::GotLanguage, language));
+      return true;
+    }
+  }
+  // The tab contents does not know its language yet.  Let's  wait until it
+  // receives it, or until the tab is closed/navigates to some other page.
   registrar_.Add(this, NotificationType::TAB_LANGUAGE_DETERMINED,
                  Source<RenderViewHost>(contents->render_view_host()));
-  AddRef();  // balanced in Observe()
+  registrar_.Add(this, NotificationType::TAB_CLOSING,
+                 Source<NavigationController>(&(contents->controller())));
+  registrar_.Add(this, NotificationType::NAV_ENTRY_COMMITTED,
+                 Source<NavigationController>(&(contents->controller())));
   return true;
 }
 
 void DetectTabLanguageFunction::Observe(NotificationType type,
                                         const NotificationSource& source,
                                         const NotificationDetails& details) {
-  DCHECK(type == NotificationType::TAB_LANGUAGE_DETERMINED);
-  std::string language(*Details<std::string>(details).ptr());
+  std::string language;
+  if (type == NotificationType::TAB_LANGUAGE_DETERMINED)
+    language = *Details<std::string>(details).ptr();
+
+  registrar_.RemoveAll();
+
+  // Call GotLanguage in all cases as we want to guarantee the callback is
+  // called for every API call the extension made.
+  GotLanguage(language);
+}
+
+void DetectTabLanguageFunction::GotLanguage(const std::string& language) {
   result_.reset(Value::CreateStringValue(language.c_str()));
   SendResponse(true);
-  Release();  // balanced in Run()
+
+  Release();  // Balanced in Run()
 }
 
 // static helpers
diff --git a/chrome/browser/extensions/extension_tabs_module.h b/chrome/browser/extensions/extension_tabs_module.h
index a4d6f5d..5282a17 100644
--- a/chrome/browser/extensions/extension_tabs_module.h
+++ b/chrome/browser/extensions/extension_tabs_module.h
@@ -133,6 +133,7 @@ class DetectTabLanguageFunction : public AsyncExtensionFunction,
   virtual void Observe(NotificationType type,
                        const NotificationSource& source,
                        const NotificationDetails& details);
+  void GotLanguage(const std::string& language);
   NotificationRegistrar registrar_;
   DECLARE_EXTENSION_FUNCTION_NAME("tabs.detectLanguage")
 };
diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc
index ba2a503..7a89215 100644
--- a/chrome/browser/renderer_host/render_view_host.cc
+++ b/chrome/browser/renderer_host/render_view_host.cc
@@ -400,10 +400,6 @@ void RenderViewHost::StopFinding(bool clear_selection) {
   Send(new ViewMsg_StopFinding(routing_id(), clear_selection));
 }
 
-void RenderViewHost::GetPageLanguage() {
-  Send(new ViewMsg_DeterminePageLanguage(routing_id()));
-}
-
 void RenderViewHost::Zoom(PageZoom::Function function) {
   Send(new ViewMsg_Zoom(routing_id(), function));
 }
@@ -741,8 +737,6 @@ void RenderViewHost::OnMessageReceived(const IPC::Message& msg) {
     IPC_MESSAGE_HANDLER(ViewHostMsg_DidFailProvisionalLoadWithError,
                         OnMsgDidFailProvisionalLoadWithError)
     IPC_MESSAGE_HANDLER(ViewHostMsg_Find_Reply, OnMsgFindReply)
-    IPC_MESSAGE_HANDLER(ViewHostMsg_PageLanguageDetermined,
-                        OnPageLanguageDetermined)
     IPC_MESSAGE_HANDLER(ViewMsg_ExecuteCodeFinished,
                         OnExecuteCodeFinished)
     IPC_MESSAGE_HANDLER(ViewHostMsg_UpdateFavIconURL, OnMsgUpdateFavIconURL)
@@ -1140,14 +1134,6 @@ void RenderViewHost::OnMsgFindReply(int request_id,
   Send(new ViewMsg_FindReplyACK(routing_id()));
 }
 
-void RenderViewHost::OnPageLanguageDetermined(const std::string& language) {
-  std::string lang(language);
-  NotificationService::current()->Notify(
-      NotificationType::TAB_LANGUAGE_DETERMINED,
-      Source<RenderViewHost>(this),
-      Details<std::string>(&lang));
-}
-
 void RenderViewHost::OnExecuteCodeFinished(int request_id, bool success) {
   std::pair<int, bool> result_details(request_id, success);
   NotificationService::current()->Notify(
@@ -1786,10 +1772,12 @@ void RenderViewHost::OnCSSInserted() {
 
 void RenderViewHost::OnPageContents(const GURL& url,
                                     int32 page_id,
-                                    const std::wstring& contents) {
+                                    const std::wstring& contents,
+                                    const std::string& language) {
   RenderViewHostDelegate::BrowserIntegration* integration_delegate =
       delegate_->GetBrowserIntegrationDelegate();
   if (!integration_delegate)
     return;
-  integration_delegate->OnPageContents(url, process()->id(), page_id, contents);
+  integration_delegate->OnPageContents(url, process()->id(), page_id, contents,
+                                       language);
 }
diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h
index 62a5e1f..2201b80 100644
--- a/chrome/browser/renderer_host/render_view_host.h
+++ b/chrome/browser/renderer_host/render_view_host.h
@@ -211,12 +211,6 @@ class RenderViewHost : public RenderWidgetHost {
   // clear the selection on the focused frame.
   void StopFinding(bool clear_selection);
 
-  // Gets the most probable language of the text content in the tab. (This sends
-  // a message to the render view.)  The caller gets the language via the
-  // NotificationService by registering to the
-  // NotificationType TAB_LANGUAGE_DETERMINED.
-  void GetPageLanguage();
-
   // Change the zoom level of a page.
   void Zoom(PageZoom::Function function);
 
@@ -499,7 +493,6 @@ class RenderViewHost : public RenderWidgetHost {
                       const gfx::Rect& selection_rect,
                       int active_match_ordinal,
                       bool final_update);
-  void OnPageLanguageDetermined(const std::string& language);
   void OnExecuteCodeFinished(int request_id, bool success);
   void OnMsgUpdateFavIconURL(int32 page_id, const GURL& icon_url);
   void OnMsgDidDownloadFavIcon(int id,
@@ -599,7 +592,9 @@ class RenderViewHost : public RenderWidgetHost {
   void OnCSSInserted();
   void OnPageContents(const GURL& url,
                       int32 page_id,
-                      const std::wstring& contents);
+                      const std::wstring& contents,
+                      const std::string& language);
+
  private:
   friend class TestRenderViewHost;
 
diff --git a/chrome/browser/renderer_host/render_view_host_delegate.h b/chrome/browser/renderer_host/render_view_host_delegate.h
index 013b913..205c2d0 100644
--- a/chrome/browser/renderer_host/render_view_host_delegate.h
+++ b/chrome/browser/renderer_host/render_view_host_delegate.h
@@ -222,7 +222,8 @@ class RenderViewHostDelegate {
     virtual void OnPageContents(const GURL& url,
                                 int renderer_process_id,
                                 int32 page_id,
-                                const std::wstring& contents) = 0;
+                                const std::wstring& contents,
+                                const std::string& language) = 0;
   };
 
   // Resource ------------------------------------------------------------------
diff --git a/chrome/browser/tab_contents/navigation_entry.h b/chrome/browser/tab_contents/navigation_entry.h
index e679c10..df6ad06 100644
--- a/chrome/browser/tab_contents/navigation_entry.h
+++ b/chrome/browser/tab_contents/navigation_entry.h
@@ -389,6 +389,15 @@ class NavigationEntry {
     return restore_type_;
   }
 
+  // The ISO 639-1 language code (ex: en, fr, zh...) for the page.
+  // Can be empty if the language was not detected yet or is unknown.
+  void set_language(const std::string& language) {
+    language_ = language;
+  }
+  std::string language() const {
+    return language_;
+  }
+
  private:
   // WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING
   // Session/Tab restore save portions of this class so that it can be recreated
@@ -413,6 +422,7 @@ class NavigationEntry {
   GURL user_typed_url_;
   bool has_post_data_;
   RestoreType restore_type_;
+  std::string language_;  // ISO 639-1 language code.
 
   // This is a cached version of the result of GetTitleForDisplay. It prevents
   // us from having to do URL formatting on the URL evey time the title is
diff --git a/chrome/browser/tab_contents/tab_contents.cc b/chrome/browser/tab_contents/tab_contents.cc
index 4474458..f4bc6d4 100644
--- a/chrome/browser/tab_contents/tab_contents.cc
+++ b/chrome/browser/tab_contents/tab_contents.cc
@@ -1098,10 +1098,6 @@ void TabContents::StopFinding(bool clear_selection) {
   render_view_host()->StopFinding(clear_selection);
 }
 
-void TabContents::GetPageLanguage() {
-  render_view_host()->GetPageLanguage();
-}
-
 void TabContents::OnSavePage() {
   // If we can not save the page, try to download it.
   if (!SavePackage::IsSavableContents(contents_mime_type())) {
@@ -1763,7 +1759,8 @@ void TabContents::OnDidGetApplicationInfo(
 void TabContents::OnPageContents(const GURL& url,
                                  int renderer_process_id,
                                  int32 page_id,
-                                 const std::wstring& contents) {
+                                 const std::wstring& contents,
+                                 const std::string& language) {
   // Don't index any https pages. People generally don't want their bank
   // accounts, etc. indexed on their computer, especially since some of these
   // things are not marked cachable.
@@ -1778,6 +1775,18 @@ void TabContents::OnPageContents(const GURL& url,
         hs->SetPageContents(url, contents);
     }
   }
+
+  NavigationEntry* entry = controller_.GetActiveEntry();
+  if (process()->id() == renderer_process_id &&
+      entry && entry->page_id() == page_id) {
+    entry->set_language(language);
+  }
+
+  std::string lang = language;
+  NotificationService::current()->Notify(
+      NotificationType::TAB_LANGUAGE_DETERMINED,
+      Source<RenderViewHost>(render_view_host()),
+      Details<std::string>(&lang));
 }
 
 void TabContents::DidStartProvisionalLoadForFrame(
diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h
index fb8597e..1418684 100644
--- a/chrome/browser/tab_contents/tab_contents.h
+++ b/chrome/browser/tab_contents/tab_contents.h
@@ -538,9 +538,6 @@ class TabContents : public PageNavigator,
     return last_search_result_;
   }
 
-  // Get the most probable language of the text content in the tab.
-  void GetPageLanguage();
-
   // Misc state & callbacks ----------------------------------------------------
 
   // Set whether the contents should block javascript message boxes or not.
@@ -817,7 +814,8 @@ class TabContents : public PageNavigator,
   virtual void OnPageContents(const GURL& url,
                               int renderer_process_id,
                               int32 page_id,
-                              const std::wstring& contents);
+                              const std::wstring& contents,
+                              const std::string& language);
 
   // RenderViewHostDelegate::Resource implementation.
   virtual void DidStartProvisionalLoadForFrame(RenderViewHost* render_view_host,
diff --git a/chrome/common/render_messages_internal.h b/chrome/common/render_messages_internal.h
index 6f74aa7..fcf448c 100644
--- a/chrome/common/render_messages_internal.h
+++ b/chrome/common/render_messages_internal.h
@@ -256,9 +256,6 @@ IPC_BEGIN_MESSAGES(View)
                       string16 /* search_text */,
                       WebKit::WebFindOptions)
 
-  // Asks the renderer for the language of the current page.
-  IPC_MESSAGE_ROUTED0(ViewMsg_DeterminePageLanguage)
-
   // Send from the renderer to the browser to return the script running result.
   IPC_MESSAGE_ROUTED2(ViewMsg_ExecuteCodeFinished,
                       int, /* request id */
@@ -1147,10 +1144,11 @@ IPC_BEGIN_MESSAGES(ViewHost)
                              std::wstring /* out - prompt field */)
 
   // Provides the contents for the given page that was loaded recently.
-  IPC_MESSAGE_ROUTED3(ViewHostMsg_PageContents,
+  IPC_MESSAGE_ROUTED4(ViewHostMsg_PageContents,
                       GURL         /* URL of the page */,
                       int32        /* page id */,
-                      std::wstring /* page contents */)
+                      std::wstring /* page contents */,
+                      std::string  /* page ISO639_1 language code */)
 
   // Used to get the extension message bundle.
   IPC_SYNC_MESSAGE_CONTROL1_1(ViewHostMsg_GetExtensionMessageBundle,
diff --git a/chrome/renderer/render_view.cc b/chrome/renderer/render_view.cc
index ed8b7a3..ae63e2b 100644
--- a/chrome/renderer/render_view.cc
+++ b/chrome/renderer/render_view.cc
@@ -301,7 +301,6 @@ RenderView::RenderView(RenderThreadBase* render_thread,
       send_preferred_size_changes_(false),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           notification_provider_(new NotificationProvider(this))),
-      determine_page_text_after_loading_stops_(false),
       view_type_(ViewType::INVALID),
       browser_window_id_(-1),
       last_top_level_navigation_page_id_(-1),
@@ -492,7 +491,6 @@ void RenderView::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ViewMsg_CopyImageAt, OnCopyImageAt)
     IPC_MESSAGE_HANDLER(ViewMsg_ExecuteEditCommand, OnExecuteEditCommand)
     IPC_MESSAGE_HANDLER(ViewMsg_Find, OnFind)
-    IPC_MESSAGE_HANDLER(ViewMsg_DeterminePageLanguage, OnDeterminePageLanguage)
     IPC_MESSAGE_HANDLER(ViewMsg_Zoom, OnZoom)
     IPC_MESSAGE_HANDLER(ViewMsg_SetZoomLevelForLoadingHost,
                         OnSetZoomLevelForLoadingHost)
@@ -663,24 +661,24 @@ void RenderView::CapturePageInfo(int load_id, bool preliminary_capture) {
   if (!preliminary_capture)
     last_indexed_page_id_ = load_id;
 
-  // get the URL for this page
+  // Get the URL for this page.
   GURL url(main_frame->url());
   if (url.is_empty())
     return;
 
-  // full text
+  // Retrieve the frame's full text.
   std::wstring contents;
   CaptureText(main_frame, &contents);
   if (contents.size()) {
-    // Send the text to the browser for indexing.
-    Send(new ViewHostMsg_PageContents(routing_id_, url, load_id, contents));
-  }
+    base::TimeTicks begin_time = base::TimeTicks::Now();
+    std::string language = DetermineTextLanguage(contents);
+    UMA_HISTOGRAM_MEDIUM_TIMES("Renderer4.LanguageDetection",
+                               base::TimeTicks::Now() - begin_time);
 
-  // Now that we have the contents, we can determine the language if necessary.
-  if (determine_page_text_after_loading_stops_) {
-    determine_page_text_after_loading_stops_ = false;
-    Send(new ViewHostMsg_PageLanguageDetermined(
-        routing_id_, DetermineTextLanguage(contents)));
+    // Send the text to the browser for indexing (the browser might decide not
+    // to index, if the URL is HTTPS for instance) and language discovery.
+    Send(new ViewHostMsg_PageContents(routing_id_, url, load_id, contents,
+                                      language));
   }
 
   // thumbnail
@@ -692,15 +690,6 @@ void RenderView::CaptureText(WebFrame* frame, std::wstring* contents) {
   if (!frame)
     return;
 
-  // Don't index any https pages. People generally don't want their bank
-  // accounts, etc. indexed on their computer, especially since some of these
-  // things are not marked cachable.
-  // TODO(brettw) we may want to consider more elaborate heuristics such as
-  // the cachability of the page. We may also want to consider subframes (this
-  // test will still index subframes if the subframe is SSL).
-  if (GURL(frame->url()).SchemeIsSecure())
-    return;
-
 #ifdef TIME_TEXT_RETRIEVAL
   double begin = time_util::GetHighResolutionTimeNow();
 #endif
@@ -3066,17 +3055,6 @@ void RenderView::OnFind(int request_id, const string16& search_text,
   }
 }
 
-void RenderView::OnDeterminePageLanguage() {
-  if (is_loading_) {
-    // Wait for the page to finish loading before trying to determine the
-    // language.
-    determine_page_text_after_loading_stops_ = true;
-    return;
-  }
-
-  Send(new ViewHostMsg_PageLanguageDetermined(routing_id_, DetectLanguage()));
-}
-
 std::string RenderView::DetectLanguage() {
   if (!webview() || is_loading_)
     return kUnknownLanguageCode;
diff --git a/chrome/renderer/render_view.h b/chrome/renderer/render_view.h
index 036cb0b..d1b4989 100644
--- a/chrome/renderer/render_view.h
+++ b/chrome/renderer/render_view.h
@@ -962,9 +962,6 @@ class RenderView : public RenderWidget,
   // Hopds a reference to the service which provides desktop notifications.
   scoped_ptr<NotificationProvider> notification_provider_;
 
-  // Set to true if request for capturing page text has been made.
-  bool determine_page_text_after_loading_stops_;
-
   // Holds state pertaining to a navigation that we initiated.  This is held by
   // the WebDataSource::ExtraData attribute.  We use pending_navigation_state_
   // as a temporary holder for the state until the WebDataSource corresponding
diff --git a/chrome/test/data/english_page.html b/chrome/test/data/english_page.html
new file mode 100644
index 0000000..539920d
--- /dev/null
+++ b/chrome/test/data/english_page.html
@@ -0,0 +1,6 @@
+<html>
+<head><title>This page is in English</title></head>
+<body>
+No joke! This is a page written in English. Awesome don't you think?
+</body>
+</html>
diff --git a/chrome/test/data/french_page.html b/chrome/test/data/french_page.html
new file mode 100644
index 0000000..7fcf684
--- /dev/null
+++ b/chrome/test/data/french_page.html
@@ -0,0 +1,6 @@
+<html>
+<head><title>Cette page est en Français</title></head>
+<body>
+Cette page a été rédigée en français. Saviez-vous que le Français est la langue officielle des jeux olympiques? Ça vous en bouche un coin, pas vrai?
+</body>
+</html>
diff --git a/chrome/test/ui_test_utils.cc b/chrome/test/ui_test_utils.cc
index db83689..4c04d20 100644
--- a/chrome/test/ui_test_utils.cc
+++ b/chrome/test/ui_test_utils.cc
@@ -257,6 +257,33 @@ class SimpleNotificationObserver : public NotificationObserver {
   DISALLOW_COPY_AND_ASSIGN(SimpleNotificationObserver);
 };
 
+class LanguageDetectionNotificationObserver : public NotificationObserver {
+ public:
+  explicit LanguageDetectionNotificationObserver(
+      RenderViewHost* render_view_host) {
+    registrar_.Add(this, NotificationType::TAB_LANGUAGE_DETERMINED,
+                  Source<RenderViewHost>(render_view_host));
+    ui_test_utils::RunMessageLoop();
+  }
+
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details) {
+    language_ = *(Details<std::string>(details).ptr());
+    MessageLoopForUI::current()->Quit();
+  }
+
+  std::string language() const {
+    return language_;
+  }
+
+ private:
+  NotificationRegistrar registrar_;
+  std::string language_;
+
+  DISALLOW_COPY_AND_ASSIGN(LanguageDetectionNotificationObserver);
+};
+
 class FindInPageNotificationObserver : public NotificationObserver {
  public:
   explicit FindInPageNotificationObserver(TabContents* parent_tab)
@@ -494,6 +521,11 @@ void WaitForFocusInBrowser(Browser* browser) {
       browser);
 }
 
+std::string WaitForLanguageDetection(TabContents* tab) {
+  LanguageDetectionNotificationObserver observer(tab->render_view_host());
+  return observer.language();
+}
+
 int FindInPage(TabContents* tab_contents, const string16& search_string,
                bool forward, bool match_case, int* ordinal) {
   tab_contents->StartFinding(search_string, forward, match_case);
diff --git a/chrome/test/ui_test_utils.h b/chrome/test/ui_test_utils.h
index 600aca0..d59d6c1 100644
--- a/chrome/test/ui_test_utils.h
+++ b/chrome/test/ui_test_utils.h
@@ -120,6 +120,10 @@ void WaitForFocusChange(RenderViewHost* rvh);
 // traversal).
 void WaitForFocusInBrowser(Browser* browser);
 
+// Waits for the language of the page to have been detected and returns it.
+// This should be called right after a navigation notification was received.
+std::string WaitForLanguageDetection(TabContents* tab_contents);
+
 // Performs a find in the page of the specified tab. Returns the number of
 // matches found.  |ordinal| is an optional parameter which is set to the index
 // of the current match.