Make it possible to read CLD data from a file

chrome/browser/translate/translate_tab_helper.cc

Index: chrome/browser/translate/translate_tab_helper.cc
diff --git a/chrome/browser/translate/translate_tab_helper.cc b/chrome/browser/translate/translate_tab_helper.cc
index bae7a5984c1eafd9be8b712c9d2192326930bed0..62cb6ea51d08c479ef4330029999cee6f83c6743 100644
--- a/chrome/browser/translate/translate_tab_helper.cc
+++ b/chrome/browser/translate/translate_tab_helper.cc
@@ -4,6 +4,10 @@
 
 #include "chrome/browser/translate/translate_tab_helper.h"
 
+#include "base/path_service.h"
+#include "base/platform_file.h"
+#include "base/synchronization/lock.h"
+#include "base/task_runner.h"
 #include "chrome/browser/chrome_notification_types.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/translate/translate_accept_languages_factory.h"
@@ -15,19 +19,28 @@
 #include "chrome/browser/ui/browser_window.h"
 #include "chrome/browser/ui/tabs/tab_strip_model.h"
 #include "chrome/browser/ui/translate/translate_bubble_factory.h"
+#include "chrome/common/chrome_paths.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/render_messages.h"
 #include "components/translate/core/browser/page_translated_details.h"
 #include "components/translate/core/browser/translate_accept_languages.h"
 #include "components/translate/core/browser/translate_prefs.h"
 #include "components/translate/core/common/language_detection_details.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 
 DEFINE_WEB_CONTENTS_USER_DATA_KEY(TranslateTabHelper);
 
+// Statics defined in the .h file:
+base::PlatformFile TranslateTabHelper::s_cached_platform_file_ = 0;
+base::Lock TranslateTabHelper::s_file_lock_;
+
 TranslateTabHelper::TranslateTabHelper(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
+      weak_pointer_factory_(this),
       translate_driver_(&web_contents->GetController()),
       translate_manager_(new TranslateManager(this)) {}
 
@@ -112,6 +125,7 @@ bool TranslateTabHelper::OnMessageReceived(const IPC::Message& message) {
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_TranslateLanguageDetermined,
                         OnLanguageDetermined)
     IPC_MESSAGE_HANDLER(ChromeViewHostMsg_PageTranslated, OnPageTranslated)
+    IPC_MESSAGE_HANDLER(ChromeViewHostMsg_NeedCLDData, OnCLDDataRequested)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
 
@@ -133,6 +147,85 @@ void TranslateTabHelper::WebContentsDestroyed(
   translate_manager_.reset();
 }
 
+void TranslateTabHelper::OnCLDDataRequested() {
+  DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+  // Quickly try to read s_cached_platform_file_. If it's non-zero, we have
+  // cached the CLD data file and can answer immediately; else, we'll make
+  // a request to the blocking pool to look up the file and cache it.
+  // The value of s_cached_platform_file_ becomes non-zero is only set once,
+  // so when it becomes non-zero we are guaranteed it will not change again.
+  if (!s_file_lock_.Try()) return; // We'll get another request later, whatevs!

[palmer, 2014/03/13 17:59:50]

Nit: Style for C++ is to put "return;" on its own line.

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

Done.

+  const base::PlatformFile handle = s_cached_platform_file_;
+  s_file_lock_.Release();
+
+  if (handle == 0) {
+    // We don't have the data file yet. Queue an asynchronous caching attempt.
+    // The caching attempt happens in the blocking pool because it may involve
+    // arbitrary filesystem access.
+    content::BrowserThread::PostBlockingPoolTask(
+        FROM_HERE,
+        base::Bind(&TranslateTabHelper::HandleCLDDataRequest,
+                   weak_pointer_factory_.GetWeakPtr()));
+  } else {
+    // We do have the data available. Respond to the request.
+    SendCLDDataAvailable(handle);
+  }
+}
+
+void TranslateTabHelper::SendCLDDataAvailable(const base::PlatformFile handle) {
+  // WARNING: This method can be invoked from either the IO thread or the
+  // blocking pool. Do not use the synchronous invocation style.
+
+  // We have the data available, respond to the request.
+  IPC::PlatformFileForTransit ipc_platform_file =
+      IPC::GetFileHandleForProcess(
+          handle,
+          GetWebContents()->GetRenderViewHost()->GetProcess()->GetHandle(),
+          false);
+  Send(new ChromeViewMsg_CLDDataAvailable(
+      GetWebContents()->GetRenderViewHost()->GetRoutingID(),
+      ipc_platform_file));
+}
+
+void TranslateTabHelper::HandleCLDDataRequest() {
+  // Because this function involves arbitrary file system access, it must run
+  // on the blocking pool.
+  DCHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::IO));
+  DCHECK(!content::BrowserThread::CurrentlyOn(content::BrowserThread::UI));
+
+  base::AutoLock lock(s_file_lock_); // will release when we return

[palmer, 2014/03/13 17:59:50]

This comment is superfluous.

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

Done.

+  if (s_cached_platform_file_ != 0) return; // already done, duplicate request

[palmer, 2014/03/13 17:59:50]

Style: "return;" on its own line.

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

Done.

+
+  base::FilePath path;
+  if (!PathService::Get(chrome::DIR_USER_DATA, &path))
+    return; // Chrome isn't properly installed.

[palmer, 2014/03/13 17:59:50]

DCHECK? CHECK or Warn the user in release builds?

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

Done. I took the DCHECK out because of other comments, but a WARN is
appropriate.

+
+  // If the file exists, we can send an IPC-safe construct back to the
+  // renderer process immediately. If not (e.g., the file is being downloaded
+  // right now), then we will do nothing. It is up to the renderer process

[palmer, 2014/03/13 17:59:50]

Rather than repeat the comments to this effect, I would just document this
"caller must retry" protocol in the .h file once.

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

Done.

+  // to call us again later.
+  path = path.Append(chrome::kCLDDataFilename);
+  if (!base::PathExists(path))
+    return; // File doesn't exist

[palmer, 2014/03/13 17:59:50]

This comment is superfluous.

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

Done.

+
+  // Attempt to open the file for reading. It exists, so we should succeed.
+  bool created = false;
+  base::PlatformFileError error;
+  s_cached_platform_file_ = base::CreatePlatformFile(
+      path,
+      base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ,
+      &created, &error);
+  DCHECK(!created);
+  if (error != base::PLATFORM_FILE_OK) {
+    s_cached_platform_file_ = 0;
+    return; // File exists, but we can't open it.

[palmer, 2014/03/13 17:59:50]

This comment is superfluous.

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

Done.

+  }
+
+  // If we get this far, we've just cached the file. Respond immediately.
+  lock.~AutoLock();

[palmer, 2014/03/13 17:59:50]

Not a problem, I don't think, but it "feels" a little odd to explicitly call
this. I wonder if it perhaps indicates a "bump" in the API.

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

I wanted to release the lock as quickly as possible, rather than holding it
while we send the response. Since the code on the other side uses a non-blocking
Try(), it's not a big deal; if there is a strong desire to avoid the destructor
manual invocation, that's cool. I'll add a comment here noting this so its
clear, let me know if you want it changed.

+  SendCLDDataAvailable(s_cached_platform_file_);
+}
+
 void TranslateTabHelper::OnLanguageDetermined(
     const LanguageDetectionDetails& details,
     bool page_needs_translation) {