Make it possible to read CLD data from a file

chrome/browser/translate/translate_tab_helper.cc

 // Copyright 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #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"
 #include "chrome/browser/translate/translate_infobar_delegate.h"
 #include "chrome/browser/translate/translate_manager.h"
 #include "chrome/browser/translate/translate_service.h"
 #include "chrome/browser/ui/browser.h"
 #include "chrome/browser/ui/browser_finder.h"
 #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)) {}
 
 TranslateTabHelper::~TranslateTabHelper() {
 }
 
 LanguageState& TranslateTabHelper::GetLanguageState() {
   return translate_driver_.language_state();
 }
 
@@ skipping 64 matching lines @@
                                    error_type,
                                    original_profile->GetPrefs());
 }
 
 bool TranslateTabHelper::OnMessageReceived(const IPC::Message& message) {
   bool handled = true;
   IPC_BEGIN_MESSAGE_MAP(TranslateTabHelper, 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()
 
   return handled;
 }
 
 void TranslateTabHelper::DidNavigateAnyFrame(
     const content::LoadCommittedDetails& details,
     const content::FrameNavigateParams& params) {
   // Let the LanguageState clear its state.
   translate_driver_.DidNavigate(details);
 }
 
 void TranslateTabHelper::WebContentsDestroyed(
     content::WebContents* web_contents) {
   // Translation process can be interrupted.
   // Destroying the TranslateManager now guarantees that it never has to deal
   // with NULL WebContents.
   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) {
   translate_driver_.language_state().LanguageDetermined(
       details.adopted_language, page_needs_translation);
 
   content::NotificationService::current()->Notify(
       chrome::NOTIFICATION_TAB_LANGUAGE_DETERMINED,
       content::Source<content::WebContents>(web_contents()),
       content::Details<const LanguageDetectionDetails>(&details));
@@ skipping 48 matching lines @@
     if (GetLanguageState().InTranslateNavigation())
       return;
   }
 
   TranslateBubbleFactory::Show(
       browser->window(), web_contents(), step, error_type);
 #else
   NOTREACHED();
 #endif
 }