diff options
author | jcampan@chromium.org <jcampan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-01-07 23:25:52 +0000 |
---|---|---|
committer | jcampan@chromium.org <jcampan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-01-07 23:25:52 +0000 |
commit | 7893a989c3e8cfa8f9cc25547025d541c1205ef4 (patch) | |
tree | 2ae568e48c773ec1c1bf614ca5ba326f9327d0d7 /chrome/browser/tab_contents | |
parent | 59a10b95b43a0b78ec3153ed21f2b1181a4a947e (diff) | |
download | chromium_src-7893a989c3e8cfa8f9cc25547025d541c1205ef4.zip chromium_src-7893a989c3e8cfa8f9cc25547025d541c1205ef4.tar.gz chromium_src-7893a989c3e8cfa8f9cc25547025d541c1205ef4.tar.bz2 |
Revert 35735 - Relanding the language detection code.
Still causes redness on the reliability bot.
The code would crash if multiple PageContents notifications
were received rapidly. The CLDHelper now notifies the
TabContents directly and the TabContents ensures only one
language detection can be performed at a time.
Added unittests to validate these cases in web_contents_unittest.cc.
Note that patch set 1 is the original patch, patch set 2 contains the new changes.
Original review:
http://codereview.chromium.org/492024/show
BUG=30662
TEST=Run the unittests.
Review URL: http://codereview.chromium.org/504051
TBR=jcampan@chromium.org
Review URL: http://codereview.chromium.org/523149
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@35753 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/tab_contents')
-rw-r--r-- | chrome/browser/tab_contents/navigation_entry.h | 10 | ||||
-rw-r--r-- | chrome/browser/tab_contents/tab_contents.cc | 57 | ||||
-rw-r--r-- | chrome/browser/tab_contents/tab_contents.h | 14 | ||||
-rw-r--r-- | chrome/browser/tab_contents/web_contents_unittest.cc | 104 |
4 files changed, 8 insertions, 177 deletions
diff --git a/chrome/browser/tab_contents/navigation_entry.h b/chrome/browser/tab_contents/navigation_entry.h index df6ad06..e679c10 100644 --- a/chrome/browser/tab_contents/navigation_entry.h +++ b/chrome/browser/tab_contents/navigation_entry.h @@ -389,15 +389,6 @@ 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 @@ -422,7 +413,6 @@ 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 e8275ce..10efa22 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -369,9 +369,6 @@ TabContents::~TabContents() { UMA_HISTOGRAM_TIMES("Tab.Close", base::TimeTicks::Now() - tab_close_start_time_); } - - if (cld_helper_.get()) - cld_helper_->CancelLanguageDetection(); } // static @@ -640,24 +637,6 @@ void TabContents::Activate() { delegate_->ActivateContents(this); } -void TabContents::PageLanguageDetected() { - DCHECK(cld_helper_.get()); - - NavigationEntry* entry = controller_.GetActiveEntry(); - if (process()->id() == cld_helper_->renderer_process_id() && - entry && entry->page_id() == cld_helper_->page_id()) { - entry->set_language(cld_helper_->language()); - } - - std::string lang = cld_helper_->language(); - NotificationService::current()->Notify( - NotificationType::TAB_LANGUAGE_DETERMINED, - Source<RenderViewHost>(render_view_host()), - Details<std::string>(&lang)); - - cld_helper_ = NULL; // Release the CLD helper. -} - void TabContents::ShowContents() { if (render_widget_host_view()) render_widget_host_view()->DidBecomeSelected(); @@ -1097,6 +1076,10 @@ 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())) { @@ -1752,38 +1735,6 @@ void TabContents::OnDidGetApplicationInfo( delegate()->OnDidGetApplicationInfo(this, page_id); } -void TabContents::OnPageContents(const GURL& url, - int renderer_process_id, - int32 page_id, - const std::wstring& contents) { - // 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 (!url.SchemeIsSecure()) { - Profile* p = profile(); - if (p && !p->IsOffTheRecord()) { - HistoryService* hs = p->GetHistoryService(Profile::IMPLICIT_ACCESS); - if (hs) - hs->SetPageContents(url, contents); - } - } - - // Detect the page language. The detection happens on the file thread. - // PageLanguageDetected() is called when the language has been detected. - if (cld_helper_.get()) { - // There is already a language detection in flight, cancel it to avoid - // having multiple PageLanguageDetected() notifications on this tab. (They - // would cause a crasher as cld_helper_ would be NULLed on the 1st - // notification). - cld_helper_->CancelLanguageDetection(); - } - cld_helper_ = new CLDHelper(this, renderer_process_id, page_id, contents); - cld_helper_->DetectLanguage(); -} - void TabContents::DidStartProvisionalLoadForFrame( RenderViewHost* render_view_host, bool is_main_frame, diff --git a/chrome/browser/tab_contents/tab_contents.h b/chrome/browser/tab_contents/tab_contents.h index 4b921ce..112675d 100644 --- a/chrome/browser/tab_contents/tab_contents.h +++ b/chrome/browser/tab_contents/tab_contents.h @@ -18,7 +18,6 @@ #include "base/scoped_ptr.h" #include "chrome/browser/autocomplete/autocomplete_edit.h" #include "chrome/browser/cancelable_request.h" -#include "chrome/browser/cld_helper.h" #include "chrome/browser/dom_ui/dom_ui_factory.h" #include "chrome/browser/download/save_package.h" #include "chrome/browser/fav_icon_helper.h" @@ -289,9 +288,6 @@ class TabContents : public PageNavigator, // to the foreground if necessary. void Activate(); - // Called by CLDHelper to notify the language of a page has been detected. - void PageLanguageDetected(); - // TODO(brettw) document these. virtual void ShowContents(); virtual void HideContents(); @@ -542,6 +538,9 @@ 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. @@ -815,10 +814,6 @@ class TabContents : public PageNavigator, virtual void OnDidGetApplicationInfo( int32 page_id, const webkit_glue::WebApplicationInfo& info); - virtual void OnPageContents(const GURL& url, - int renderer_process_id, - int32 page_id, - const std::wstring& contents); // RenderViewHostDelegate::Resource implementation. virtual void DidStartProvisionalLoadForFrame(RenderViewHost* render_view_host, @@ -1183,9 +1178,6 @@ class TabContents : public PageNavigator, // profile scoped_refptr<URLRequestContextGetter> request_context_; - // Used to retrieve the language of the current page. - scoped_refptr<CLDHelper> cld_helper_; - // --------------------------------------------------------------------------- DISALLOW_COPY_AND_ASSIGN(TabContents); diff --git a/chrome/browser/tab_contents/web_contents_unittest.cc b/chrome/browser/tab_contents/web_contents_unittest.cc index 8254001..9b59406 100644 --- a/chrome/browser/tab_contents/web_contents_unittest.cc +++ b/chrome/browser/tab_contents/web_contents_unittest.cc @@ -17,7 +17,6 @@ #include "chrome/common/render_messages.h" #include "chrome/common/url_constants.h" #include "chrome/test/testing_profile.h" -#include "chrome/test/ui_test_utils.h" #include "ipc/ipc_channel.h" #include "testing/gtest/include/gtest/gtest.h" @@ -195,9 +194,7 @@ class TabContentsTest : public RenderViewHostTestHarness { public: TabContentsTest() : RenderViewHostTestHarness(), - ui_thread_(ChromeThread::UI, &message_loop_), - file_thread_(ChromeThread::FILE) { - file_thread_.Start(); + ui_thread_(ChromeThread::UI, &message_loop_) { } private: @@ -209,7 +206,6 @@ class TabContentsTest : public RenderViewHostTestHarness { } ChromeThread ui_thread_; - ChromeThread file_thread_; }; // Test to make sure that title updates get stripped of whitespace. @@ -1431,101 +1427,3 @@ TEST_F(TabContentsTest, NoJSMessageOnInterstitials) { &did_suppress_message); EXPECT_TRUE(did_suppress_message); } - -// TODO(port): The compact language detection library works only for Windows. -#if defined(OS_WIN) -// Tests the normal case of language detection when a page loads. -TEST_F(TabContentsTest, NormalLanguageDetection) { - const GURL kURL("http://www.google.com"); - const std::wstring kPageContent(L"C'est le contenu de la page. Fantastique!"); - - // Navigate to a page. - NavigateAndCommit(kURL); - - NavigationEntry* entry = contents()->controller().GetActiveEntry(); - ASSERT_TRUE(entry); - - // Simulate the PageContents message that triggers the language detection. - rvh()->TestOnMessageReceived(ViewHostMsg_PageContents(0, - kURL, entry->page_id(), - kPageContent)); - std::string language = ui_test_utils::WaitForLanguageDetection(contents()); - EXPECT_EQ("fr", language); -} - -// Tests the case of language detection when several page loads happen for the -// same page. -TEST_F(TabContentsTest, LanguageDetectionMultipleLoads) { - const GURL kURL("http://www.google.com"); - const std::wstring kPageContent(L"The page contents. It is fantastic!"); - - // Navigate to a page. - NavigateAndCommit(kURL); - - NavigationEntry* entry = contents()->controller().GetActiveEntry(); - ASSERT_TRUE(entry); - - // Simulate 2 PageContents messages that triggers the language detection. - rvh()->TestOnMessageReceived(ViewHostMsg_PageContents(0, - kURL, entry->page_id(), - kPageContent)); - rvh()->TestOnMessageReceived(ViewHostMsg_PageContents(0, - kURL, entry->page_id(), - kPageContent)); - std::string language = ui_test_utils::WaitForLanguageDetection(contents()); - EXPECT_EQ("en", language); -} - -// Tests the case of language detection when several page loads happen. -TEST_F(TabContentsTest, LanguageDetectionMultipleNavigations) { - const GURL kURL1("http://www.google.com"); - const GURL kURL2("http://www.yahoo.com"); - const std::wstring kPageContent1(L"The page contents. It is fantastic!"); - const std::wstring kPageContent2(L"C'est le contenu de la page.Fantastique!"); - // Navigate to the first page. - NavigateAndCommit(kURL1); - NavigationEntry* entry = contents()->controller().GetActiveEntry(); - ASSERT_TRUE(entry); - - // Simulate the PageContents message that triggers the language detection. - rvh()->TestOnMessageReceived(ViewHostMsg_PageContents(0, - kURL1, entry->page_id(), - kPageContent1)); - - // Before the language notification is back to the UI thread, navigate to a - // new page. - NavigateAndCommit(kURL2); - entry = contents()->controller().GetActiveEntry(); - ASSERT_TRUE(entry); - - // Simulate the PageContents message that triggers the language detection. - rvh()->TestOnMessageReceived(ViewHostMsg_PageContents(0, - kURL2, entry->page_id(), - kPageContent2)); - - std::string language = ui_test_utils::WaitForLanguageDetection(contents()); - EXPECT_EQ("fr", language); -} -#endif - -// Tests the case of language detection when the tab is closed before the -// detection has been performed. -TEST_F(TabContentsTest, LanguageDetectionClosedTab) { - const GURL kURL("http://www.google.com"); - const std::wstring kPageContent(L"The page contents. It is fantastic!"); - // Navigate to the page. - NavigateAndCommit(kURL); - NavigationEntry* entry = contents()->controller().GetActiveEntry(); - ASSERT_TRUE(entry); - - // Simulate the PageContents message that triggers the language detection. - rvh()->TestOnMessageReceived(ViewHostMsg_PageContents(0, - kURL, entry->page_id(), - kPageContent)); - - // Now close the tab. - DeleteContents(); - - // Run all pending message to make sure nothing crashes. - MessageLoop::current()->RunAllPending(); -} |