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 "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/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);
 
 TranslateTabHelper::TranslateTabHelper(content::WebContents* web_contents)
     : content::WebContentsObserver(web_contents),
       translate_driver_(&web_contents->GetController()),
       translate_manager_(new TranslateManager(this)) {}
 
 TranslateTabHelper::~TranslateTabHelper() {
@@ skipping 70 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();
 }
 
+// TODO(andrewhayden): It's annoying that during download of the data file,
+// multiple renderers may be calling this method repeatedly, but there's not
+// an obviously-better choice from a complexity standpoint: we have no idea
+// if or when the file will become available, and the check is cheap.
+void TranslateTabHelper::OnCLDDataRequested() {
+  static base::PlatformFile cached_platform_file = 0;

[bulach, 2014/03/11 18:56:10]

nit: avoid using function-scoped static, make it either a class-level or in a
namespace.

[Andrew Hayden (chromium.org), 2014/03/13 16:33:33]

Done.

+  const int routing_id = GetWebContents()->GetRenderViewHost()->GetRoutingID();
+
+  // If we have cached the platform file, we reuse it. Else, we cache it now.
+  if (cached_platform_file == 0) {
+    // First, grab a reference to the user data directory
+    base::FilePath path;
+    if (!PathService::Get(chrome::DIR_USER_DATA, &path)) {
+        NOTREACHED();

[bulach, 2014/03/11 18:56:10]

nit: unindent.
also, in some other places in this file doesn't use {} for single-line
statement, probably best to keep consistency.

[Andrew Hayden (chromium.org), 2014/03/13 16:33:33]

Done.

+    }
+
+    // Now derive the name of the CLD data file that we are looking for.
+    path = path.Append(chrome::kCLDDataFilename);
+
+    // 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
+    // to call us again later.
+    if (!base::PathExists(path)) {

[bulach, 2014/03/11 18:56:10]

ditto for {}

[Andrew Hayden (chromium.org), 2014/03/13 16:33:33]

Done.

+      return;
+    }
+
+    // Attempt to open the file for reading.
+    int flags = base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ;

[bulach, 2014/03/11 18:56:10]

nit: probably simpler to just pass this inline rather than keeping a local?

[Andrew Hayden (chromium.org), 2014/03/13 16:33:33]

Done.

+    bool created = false;
+    base::PlatformFileError error;
+    cached_platform_file = base::CreatePlatformFile(

[bulach, 2014/03/11 18:56:10]

I think this is arriving at the IO thread, we should open the file in the FILE
thread.

[Andrew Hayden (chromium.org), 2014/03/13 16:33:33]

I have moved this to the blocking pool. It adds a bit of complexity, but it
seems to work fine! Thanks for the suggestion. I added some comments to make it
clear what is going on, as we do need to bring in locks at this point. I believe
the resulting code is as non-blocking as it can be, and should perform well.

+        path, flags, &created, &error);
+    DCHECK(created == false);
+    if (error != base::PLATFORM_FILE_OK) {
+      cached_platform_file = 0;
+      DCHECK(false) << "Failed to open CLD data file, error code=" << error;
+      return;
+    }
+  }
+
+  // Get a handle to the renderer process for IPC.
+  base::ProcessHandle rendererProcessHandle =
+      GetWebContents()->GetRenderViewHost()->GetProcess()->GetHandle();
+  // Prepare a file handle for transit to that renderer process.
+  IPC::PlatformFileForTransit ipcPlatformFile = IPC::GetFileHandleForProcess(

[bulach, 2014/03/11 18:56:10]

nit: s/ipcPlatformFile/ipc_platform_file/

[Andrew Hayden (chromium.org), 2014/03/13 16:33:33]

Done.

+      cached_platform_file,
+      rendererProcessHandle,
+      false);
+  // Send the file handle to the renderer via IPC. We're done!
+  Send(new ChromeViewMsg_CLDDataAvailable(routing_id, ipcPlatformFile));

[bulach, 2014/03/11 18:56:10]

I suppose if this is in the IO thread, we wouldn't want to send the reply from
within the same message.
depending on how 173 is solved, it may be ok though (if this function ends up
being executed in the FILE thread).

[Andrew Hayden (chromium.org), 2014/03/13 16:33:33]

We still respond from the IO thread, but I see no danger here. We're talking to
a different process and we're not sending messages synchronously. I'm leaving
this as-is unless there's a strong reason to object. I've added a comment noting
this for maintainers.

+}
+
 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
 }