diff options
author | jcampan@chromium.org <jcampan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-01-15 17:05:46 +0000 |
---|---|---|
committer | jcampan@chromium.org <jcampan@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-01-15 17:05:46 +0000 |
commit | ba9daa6c087fb6320f32f47561b2d06a4c04c41b (patch) | |
tree | 6a6abd8e2ba2e0f91a1fe0f0656ed7c2639eb140 /chrome/browser | |
parent | 01938c95315cec08d8bfed833e02374626ee9961 (diff) | |
download | chromium_src-ba9daa6c087fb6320f32f47561b2d06a4c04c41b.zip chromium_src-ba9daa6c087fb6320f32f47561b2d06a4c04c41b.tar.gz chromium_src-ba9daa6c087fb6320f32f47561b2d06a4c04c41b.tar.bz2 |
3rd attempt at landing the language detection on page load.
A memory error has been fixed in the CLD library in the meantime.
This should hopefully fixes the crashers in the reliability tests.
Note that this version is actually simpler than the original review since the detection is now performed in the renderer. (So the CLD code runs sandboxed.)
Original review:
http://codereview.chromium.org/492024/show
BUG=30662
TEST=Run the unit-tests.
Review URL: http://codereview.chromium.org/518075
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@36362 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser')
-rw-r--r-- | chrome/browser/browser.cc | 4 | ||||
-rw-r--r-- | chrome/browser/browser_browsertest.cc | 33 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_tabs_module.cc | 43 | ||||
-rw-r--r-- | chrome/browser/extensions/extension_tabs_module.h | 1 | ||||
-rw-r--r-- | chrome/browser/renderer_host/browser_render_process_host.cc | 13 | ||||
-rw-r--r-- | chrome/browser/renderer_host/browser_render_process_host.h | 2 | ||||
-rw-r--r-- | chrome/browser/renderer_host/render_view_host.cc | 27 | ||||
-rw-r--r-- | chrome/browser/renderer_host/render_view_host.h | 21 | ||||
-rw-r--r-- | chrome/browser/renderer_host/render_view_host_delegate.h | 7 | ||||
-rw-r--r-- | chrome/browser/tab_contents/navigation_entry.h | 10 | ||||
-rw-r--r-- | chrome/browser/tab_contents/tab_contents.cc | 37 | ||||
-rw-r--r-- | chrome/browser/tab_contents/tab_contents.h | 8 |
12 files changed, 146 insertions, 60 deletions
diff --git a/chrome/browser/browser.cc b/chrome/browser/browser.cc index 0cfbe13..a3c4ab5 100644 --- a/chrome/browser/browser.cc +++ b/chrome/browser/browser.cc @@ -1210,8 +1210,8 @@ void Browser::OpenCreateShortcutsDialog() { DCHECK(pending_web_app_action_ == NONE); pending_web_app_action_ = CREATE_SHORTCUT; - // Start fetching web app info for CreateApplicatoinShortcut dialog and - // show the dialog when the data is available in OnDidGetApplicationInfo. + // Start fetching web app info for CreateApplicationShortcut dialog and show + // the dialog when the data is available in OnDidGetApplicationInfo. current_tab->render_view_host()->GetApplicationInfo(entry->page_id()); #else NOTIMPLEMENTED(); diff --git a/chrome/browser/browser_browsertest.cc b/chrome/browser/browser_browsertest.cc index 756b7dc..a948947 100644 --- a/chrome/browser/browser_browsertest.cc +++ b/chrome/browser/browser_browsertest.cc @@ -288,3 +288,36 @@ IN_PROC_BROWSER_TEST_F(BrowserTest, FaviconOfOnloadRedirectToAnchorPage) { controller().GetActiveEntry(); EXPECT_EQ(expected_favicon_url.spec(), entry->favicon().url().spec()); } + +// The CLD library only works on Windows at this point. +#if defined(OS_WIN) +// 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()); +} +#endif diff --git a/chrome/browser/extensions/extension_tabs_module.cc b/chrome/browser/extensions/extension_tabs_module.cc index 19d2687..fca5820 100644 --- a/chrome/browser/extensions/extension_tabs_module.cc +++ b/chrome/browser/extensions/extension_tabs_module.cc @@ -860,26 +860,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/browser_render_process_host.cc b/chrome/browser/renderer_host/browser_render_process_host.cc index 32c141c..dadeb53 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.cc +++ b/chrome/browser/renderer_host/browser_render_process_host.cc @@ -738,7 +738,6 @@ void BrowserRenderProcessHost::OnMessageReceived(const IPC::Message& msg) { // Dispatch control messages. bool msg_is_ok = true; IPC_BEGIN_MESSAGE_MAP_EX(BrowserRenderProcessHost, msg, msg_is_ok) - IPC_MESSAGE_HANDLER(ViewHostMsg_PageContents, OnPageContents) IPC_MESSAGE_HANDLER(ViewHostMsg_UpdatedCacheStats, OnUpdatedCacheStats) IPC_MESSAGE_HANDLER(ViewHostMsg_SuddenTerminationChanged, @@ -836,18 +835,6 @@ void BrowserRenderProcessHost::OnChannelError() { // TODO(darin): clean this up } -void BrowserRenderProcessHost::OnPageContents(const GURL& url, - int32 page_id, - const std::wstring& contents) { - Profile* p = profile(); - if (!p || p->IsOffTheRecord()) - return; - - HistoryService* hs = p->GetHistoryService(Profile::IMPLICIT_ACCESS); - if (hs) - hs->SetPageContents(url, contents); -} - void BrowserRenderProcessHost::OnUpdatedCacheStats( const WebCache::UsageStats& stats) { WebCacheManager::GetInstance()->ObserveStats(id(), stats); diff --git a/chrome/browser/renderer_host/browser_render_process_host.h b/chrome/browser/renderer_host/browser_render_process_host.h index 3a6e83b..0fd51ea 100644 --- a/chrome/browser/renderer_host/browser_render_process_host.h +++ b/chrome/browser/renderer_host/browser_render_process_host.h @@ -104,8 +104,6 @@ class BrowserRenderProcessHost : public RenderProcessHost, friend class VisitRelayingRenderProcessHost; // Control message handlers. - void OnPageContents(const GURL& url, int32 page_id, - const std::wstring& contents); void OnUpdatedCacheStats(const WebKit::WebCache::UsageStats& stats); void SuddenTerminationChanged(bool enabled); void OnExtensionAddListener(const std::string& event_name); diff --git a/chrome/browser/renderer_host/render_view_host.cc b/chrome/browser/renderer_host/render_view_host.cc index 842e7b4..0af786b 100644 --- a/chrome/browser/renderer_host/render_view_host.cc +++ b/chrome/browser/renderer_host/render_view_host.cc @@ -428,10 +428,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)); } @@ -769,8 +765,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) @@ -854,6 +848,7 @@ void RenderViewHost::OnMessageReceived(const IPC::Message& msg) { IPC_MESSAGE_HANDLER(ViewHostMsg_AccessibilityFocusChange, OnAccessibilityFocusChange) IPC_MESSAGE_HANDLER(ViewHostMsg_OnCSSInserted, OnCSSInserted) + IPC_MESSAGE_HANDLER(ViewHostMsg_PageContents, OnPageContents) // Have the super handle all other messages. IPC_MESSAGE_UNHANDLED(RenderWidgetHost::OnMessageReceived(msg)) IPC_END_MESSAGE_MAP_EX() @@ -1167,14 +1162,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( @@ -1795,3 +1782,15 @@ void RenderViewHost::OnAccessibilityFocusChange(int acc_obj_id) { void RenderViewHost::OnCSSInserted() { delegate_->DidInsertCSS(); } + +void RenderViewHost::OnPageContents(const GURL& url, + int32 page_id, + 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, + language); +} diff --git a/chrome/browser/renderer_host/render_view_host.h b/chrome/browser/renderer_host/render_view_host.h index 831bd80..3eca11d 100644 --- a/chrome/browser/renderer_host/render_view_host.h +++ b/chrome/browser/renderer_host/render_view_host.h @@ -213,12 +213,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); @@ -427,22 +421,22 @@ class RenderViewHost : public RenderWidgetHost, // Creates a new RenderWidget with the given route id. void CreateNewWidget(int route_id, bool activatable); - // Send the response to an extension api call. + // Sends the response to an extension api call. void SendExtensionResponse(int request_id, bool success, const std::string& response, const std::string& error); - // Send a response to an extension api call that it was blocked for lack of + // Sends a response to an extension api call that it was blocked for lack of // permission. void BlockExtensionRequest(int request_id); - // Notify the renderer that its view type has changed. + // Notifies the renderer that its view type has changed. void ViewTypeChanged(ViewType::Type type); - // Tell renderer which browser window it is being attached to. + // Tells the renderer which browser window it is being attached to. void UpdateBrowserWindowId(int window_id); - // Tell render view that custom context action has been selected. + // Tells the render view that a custom context action has been selected. void PerformCustomContextMenuAction(unsigned action); protected: @@ -500,7 +494,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, @@ -598,6 +591,10 @@ class RenderViewHost : public RenderWidgetHost, void OnExtensionPostMessage(int port_id, const std::string& message); void OnAccessibilityFocusChange(int acc_obj_id); void OnCSSInserted(); + void OnPageContents(const GURL& url, + int32 page_id, + 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 c3df62d..eecc54f 100644 --- a/chrome/browser/renderer_host/render_view_host_delegate.h +++ b/chrome/browser/renderer_host/render_view_host_delegate.h @@ -217,6 +217,13 @@ class RenderViewHostDelegate { virtual void OnDidGetApplicationInfo( int32 page_id, const webkit_glue::WebApplicationInfo& app_info) = 0; + + // Notification that the contents of the page has been loaded. + virtual void OnPageContents(const GURL& url, + int renderer_process_id, + int32 page_id, + 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 10efa22..bf0bd56 100644 --- a/chrome/browser/tab_contents/tab_contents.cc +++ b/chrome/browser/tab_contents/tab_contents.cc @@ -1076,10 +1076,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())) { @@ -1735,6 +1731,39 @@ 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, + 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. + // 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); + } + } + + 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( 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 112675d..6333b59 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. @@ -814,6 +811,11 @@ 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, + const std::string& language); // RenderViewHostDelegate::Resource implementation. virtual void DidStartProvisionalLoadForFrame(RenderViewHost* render_view_host, |