Modularize Compact Language Detector 2 (CLD2) data sources

chrome/renderer/translate/translate_helper.cc

Index: chrome/renderer/translate/translate_helper.cc
diff --git a/chrome/renderer/translate/translate_helper.cc b/chrome/renderer/translate/translate_helper.cc
index f29b31ff9007e331b0e7d08ed0c442219559ffdf..20c080483a05986699663e67a5d80f2d1cd7fde4 100644
--- a/chrome/renderer/translate/translate_helper.cc
+++ b/chrome/renderer/translate/translate_helper.cc
@@ -4,15 +4,8 @@
 
 #include "chrome/renderer/translate/translate_helper.h"
 
-#if defined(CLD2_DYNAMIC_MODE)
-#include <stdint.h>
-#endif
-
 #include "base/bind.h"
 #include "base/compiler_specific.h"
-#if defined(CLD2_DYNAMIC_MODE)
-#include "base/files/memory_mapped_file.h"
-#endif
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/strings/string16.h"
@@ -28,10 +21,7 @@
 #include "extensions/common/constants.h"
 #include "extensions/renderer/extension_groups.h"
 #include "ipc/ipc_platform_file.h"
-#if defined(CLD2_DYNAMIC_MODE)
 #include "content/public/common/url_constants.h"
-#include "third_party/cld_2/src/public/compact_lang_det.h"
-#endif
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebElement.h"
 #include "third_party/WebKit/public/web/WebFrame.h"
@@ -75,14 +65,11 @@ const char kAutoDetectionLanguage[] = "auto";
 // Isolated world sets following content-security-policy.
 const char kContentSecurityPolicy[] = "script-src 'self' 'unsafe-eval'";
 
+// Whether or not we have set the CLD callback yet.
+bool cld_callback_set = false;

[Takashi Toyoshima, 2014/06/23 08:50:01]

g_cld_callback_set

[Andrew Hayden (chromium.org), 2014/06/23 13:20:36]

Done.

+
 }  // namespace
 
-#if defined(CLD2_DYNAMIC_MODE)
-// The mmap for the CLD2 data must be held forever once it is available in the
-// process. This is declared static in the translate_helper.h.
-base::LazyInstance<TranslateHelper::CLDMmapWrapper>::Leaky
-  TranslateHelper::s_cld_mmap_ = LAZY_INSTANCE_INITIALIZER;
-#endif
 
 ////////////////////////////////////////////////////////////////////////////////
 // TranslateHelper, public:
@@ -91,29 +78,24 @@ TranslateHelper::TranslateHelper(content::RenderView* render_view)
     : content::RenderViewObserver(render_view),
       page_id_(-1),
       translation_pending_(false),
-      weak_method_factory_(this)
-#if defined(CLD2_DYNAMIC_MODE)
-      ,cld2_data_file_polling_started_(false),
-      cld2_data_file_polling_canceled_(false),
+      weak_method_factory_(this),
+      cld_data_provider_(translate::CreateRendererCldDataProviderFor(this)),
+      cld_data_polling_started_(false),
+      cld_data_polling_canceled_(false),
       deferred_page_capture_(false),
       deferred_page_id_(-1),
-      deferred_contents_(ASCIIToUTF16(""))
-#endif
-  {
+      deferred_contents_(ASCIIToUTF16("")) {
 }
 
 TranslateHelper::~TranslateHelper() {
   CancelPendingTranslation();
-#if defined(CLD2_DYNAMIC_MODE)
-  CancelCLD2DataFilePolling();
-#endif
+  CancelCldDataPolling();
 }
 
 void TranslateHelper::PrepareForUrl(const GURL& url) {
-#if defined(CLD2_DYNAMIC_MODE)
   deferred_page_capture_ = false;
   deferred_contents_.clear();
-  if (cld2_data_file_polling_started_)
+  if (cld_data_polling_started_)
     return;
 
   // TODO(andrewhayden): Refactor translate_manager.cc's IsTranslatableURL to

[Andrew Hayden (chromium.org), 2014/06/21 07:40:28]

This is biting us: the chromeos bots are failing with this message:
error: 'file_manager' has not been declared

I'll add a temporary workaround for now, but as soon as this refactor is landed
we should get this take care of.

[droger, 2014/06/23 08:31:31]

I don't think this can go in components/translate/core/:
- it depends on content/, and core/ cannot depend on content/
- I don't know if components can depend on extensions, but in any case core/
cannot.

If you want to move it I'd think this should be better in
chrome/common/translate/, or maybe in components/translate/content/common, if
it's ok to depend on extensions there.

[Andrew Hayden (chromium.org), 2014/06/23 13:20:36]

Talked with Takashi about this, see below.

@@ -135,19 +117,16 @@ void TranslateHelper::PrepareForUrl(const GURL& url) {
 #endif
 
   // Start polling for CLD data.
-  cld2_data_file_polling_started_ = true;
-  TranslateHelper::SendCLD2DataFileRequest(0, 1000);
-#endif
+  cld_data_polling_started_ = true;
+  TranslateHelper::SendCldDataRequest(0, 1000);
 }
 
-#if defined(CLD2_DYNAMIC_MODE)
 void TranslateHelper::DeferPageCaptured(const int page_id,
                                         const base::string16& contents) {
   deferred_page_capture_ = true;
   deferred_page_id_ = page_id;
   deferred_contents_ = contents;
 }
-#endif
 
 void TranslateHelper::PageCaptured(int page_id,
                                    const base::string16& contents) {
@@ -166,14 +145,13 @@ void TranslateHelper::PageCaptured(int page_id,
 
   // TODO(andrewhayden): UMA insertion point here: Track if data is available.
   // TODO(andrewhayden): Retry insertion point here, retry till data available.
-#if defined(CLD2_DYNAMIC_MODE)
-  if (!CLD2::isDataLoaded()) {
+  if (!cld_data_provider_->IsCldDataAvailable()) {
     // We're in dynamic mode and CLD data isn't loaded. Retry when CLD data
     // is loaded, if ever.
     TranslateHelper::DeferPageCaptured(page_id, contents);
     return;
   }
-#endif
+
   page_id_ = page_id;
   WebDocument document = main_frame->document();
   std::string content_language = document.contentLanguage().utf8();
@@ -218,9 +196,7 @@ void TranslateHelper::CancelPendingTranslation() {
   translation_pending_ = false;
   source_lang_.clear();
   target_lang_.clear();
-#if defined(CLD2_DYNAMIC_MODE)
-  CancelCLD2DataFilePolling();
-#endif
+  CancelCldDataPolling();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -395,11 +371,17 @@ bool TranslateHelper::OnMessageReceived(const IPC::Message& message) {
   IPC_BEGIN_MESSAGE_MAP(TranslateHelper, message)
     IPC_MESSAGE_HANDLER(ChromeViewMsg_TranslatePage, OnTranslatePage)
     IPC_MESSAGE_HANDLER(ChromeViewMsg_RevertTranslation, OnRevertTranslation)
-#if defined(CLD2_DYNAMIC_MODE)
-    IPC_MESSAGE_HANDLER(ChromeViewMsg_CLDDataAvailable, OnCLDDataAvailable);
-#endif
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
+  if (!handled) {
+    if (!cld_callback_set) {

[Takashi Toyoshima, 2014/06/23 08:50:01]

Hum.... setting a callback inside this message handler looks a little tricky.
Can you set it just before calling cld_data_provider_->SendCldDataRequest()?

[Andrew Hayden (chromium.org), 2014/06/23 13:20:36]

Done.

+      cld_callback_set = true;
+      cld_data_provider_->SetCldAvailableCallback(
+          base::Bind(&TranslateHelper::OnCldDataAvailable,
+                     weak_method_factory_.GetWeakPtr()));
+    }
+    handled = cld_data_provider_->OnMessageReceived(message);
+  }
   return handled;
 }
 
@@ -588,23 +570,22 @@ WebFrame* TranslateHelper::GetMainFrame() {
   return web_view->mainFrame();
 }
 
-#if defined(CLD2_DYNAMIC_MODE)
-void TranslateHelper::CancelCLD2DataFilePolling() {
-  cld2_data_file_polling_canceled_ = true;
+void TranslateHelper::CancelCldDataPolling() {
+  cld_data_polling_canceled_ = true;
 }
 
-void TranslateHelper::SendCLD2DataFileRequest(const int delay_millis,
-                                              const int next_delay_millis) {
+void TranslateHelper::SendCldDataRequest(const int delay_millis,
+                                         const int next_delay_millis) {
   // Terminate immediately if told to stop polling.
-  if (cld2_data_file_polling_canceled_)
+  if (cld_data_polling_canceled_)
     return;
 
   // Terminate immediately if data is already loaded.
-  if (CLD2::isDataLoaded())
+  if (cld_data_provider_->IsCldDataAvailable())
     return;
 
-  // Else, send the IPC message to the browser process requesting the data...
-  Send(new ChromeViewHostMsg_NeedCLDData(routing_id()));
+  // Else, make an asynchronous request to get the data we need.

[Takashi Toyoshima, 2014/06/23 08:50:01]

As a commented above, you may be able to set the callback here. It would be
readable for me.

[Andrew Hayden (chromium.org), 2014/06/23 13:20:36]

Done.

+  cld_data_provider_->SendCldDataRequest();
 
   // ... and enqueue another delayed task to call again. This will start a
   // chain of polling that will last until the pointer stops being NULL,
@@ -616,68 +597,17 @@ void TranslateHelper::SendCLD2DataFileRequest(const int delay_millis,
   // Use a weak pointer to avoid keeping this helper object around forever.
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
-      base::Bind(&TranslateHelper::SendCLD2DataFileRequest,
+      base::Bind(&TranslateHelper::SendCldDataRequest,
                  weak_method_factory_.GetWeakPtr(),
                  next_delay_millis, next_delay_millis),
       base::TimeDelta::FromMilliseconds(delay_millis));
 }
 
-void TranslateHelper::OnCLDDataAvailable(
-    const IPC::PlatformFileForTransit ipc_file_handle,
-    const uint64 data_offset,
-    const uint64 data_length) {
-  LoadCLDDData(IPC::PlatformFileForTransitToFile(ipc_file_handle), data_offset,
-               data_length);
-  if (deferred_page_capture_ && CLD2::isDataLoaded()) {
+void TranslateHelper::OnCldDataAvailable() {
+  if (deferred_page_capture_) {
     deferred_page_capture_ = false; // Don't do this a second time.
     PageCaptured(deferred_page_id_, deferred_contents_);
     deferred_page_id_ = -1; // Clean up for sanity
     deferred_contents_.clear(); // Clean up for sanity
   }
 }
-
-void TranslateHelper::LoadCLDDData(
-    base::File file,
-    const uint64 data_offset,
-    const uint64 data_length) {
-  // Terminate immediately if told to stop polling.
-  if (cld2_data_file_polling_canceled_)
-    return;
-
-  // Terminate immediately if data is already loaded.
-  if (CLD2::isDataLoaded())
-    return;
-
-  if (!file.IsValid()) {
-    LOG(ERROR) << "Can't find the CLD data file.";
-    return;
-  }
-
-  // mmap the file
-  s_cld_mmap_.Get().value = new base::MemoryMappedFile();
-  bool initialized = s_cld_mmap_.Get().value->Initialize(file.Pass());
-  if (!initialized) {
-    LOG(ERROR) << "mmap initialization failed";
-    delete s_cld_mmap_.Get().value;
-    s_cld_mmap_.Get().value = NULL;
-    return;
-  }
-
-  // Sanity checks
-  uint64 max_int32 = std::numeric_limits<int32>::max();
-  if (data_length + data_offset > s_cld_mmap_.Get().value->length()
-      || data_length > max_int32) { // max signed 32 bit integer
-    LOG(ERROR) << "Illegal mmap config: data_offset="
-        << data_offset << ", data_length=" << data_length
-        << ", mmap->length()=" << s_cld_mmap_.Get().value->length();
-    delete s_cld_mmap_.Get().value;
-    s_cld_mmap_.Get().value = NULL;
-    return;
-  }
-
-  // Initialize the CLD subsystem... and it's all done!
-  const uint8* data_ptr = s_cld_mmap_.Get().value->data() + data_offset;
-  CLD2::loadDataFromRawAddress(data_ptr, data_length);
-  DCHECK(CLD2::isDataLoaded()) << "Failed to load CLD data from mmap";
-}
-#endif