Media Galleries API Picasa: Put INI indexing step into sandboxed utility process.

chrome/browser/media_galleries/fileapi/safe_picasa_albums_indexer.cc

+// Copyright 2013 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/media_galleries/fileapi/safe_picasa_albums_indexer.h"
+
+#include "base/file_util.h"
+#include "chrome/browser/media_galleries/fileapi/media_file_system_backend.h"
+#include "chrome/common/chrome_utility_messages.h"
+#include "content/public/browser/browser_thread.h"
+#include "content/public/browser/child_process_data.h"
+#include "content/public/browser/utility_process_host.h"
+
+using chrome::MediaFileSystemBackend;
+using content::BrowserThread;
+using content::UtilityProcessHost;
+
+namespace picasa {
+
+namespace {
+
+// Picasa INI files are named "picasa.ini" on Picasa for Windows before version
+// 71.18. Later versions and Picasa for Mac uses ".picasa.ini".
+// See: https://support.google.com/picasa/answer/11257?hl=en
+const char kPicasaINIFilename[]       = ".picasa.ini";
+const char kPicasaINIFilenameLegacy[] = "picasa.ini";
+
+// Arbitrarily chosen to be a decent size but not block thread too much.
+const int kPicasaINIReadBatchSize = 10;
+
+}
+
+SafePicasaAlbumsIndexer::SafePicasaAlbumsIndexer(
+    const AlbumMap& albums,
+    const AlbumMap& folders,
+    const ParserCallback& callback)
+    : callback_(callback),
+      parser_state_(INITIAL_STATE) {
+  DCHECK(MediaFileSystemBackend::CurrentlyOnMediaTaskRunnerThread());
+  DCHECK(!callback_.is_null());
+
+  for (AlbumMap::const_iterator it = albums.begin(); it != albums.end(); ++it) {
+    album_uids_.insert(it->second.uid);
+  }
+
+  for (AlbumMap::const_iterator it = folders.begin(); it != folders.end();
+       ++it) {
+    folders_queue.push(it->second.path);
+  }
+}
+
+void SafePicasaAlbumsIndexer::Start() {
+  DCHECK(MediaFileSystemBackend::CurrentlyOnMediaTaskRunnerThread());
+  DCHECK_EQ(INITIAL_STATE, parser_state_);
+
+  parser_state_ = STARTED_READING_INI_FILES_STATE;
+
+  ProcessFoldersBatch();
+}
+
+SafePicasaAlbumsIndexer::~SafePicasaAlbumsIndexer() {
+}
+
+void SafePicasaAlbumsIndexer::ProcessFoldersBatch() {

[tommycli, 2013/07/09 21:52:57]

This callback is posted repeatedly until done.

+  DCHECK(MediaFileSystemBackend::CurrentlyOnMediaTaskRunnerThread());
+  DCHECK_EQ(STARTED_READING_INI_FILES_STATE, parser_state_);
+
+  for (int i = 0; i < kPicasaINIReadBatchSize && !folders_queue.empty(); ++i) {
+    base::FilePath folder_path = folders_queue.front();
+    folders_queue.pop();
+
+    FolderINIContents folder_ini;
+    folder_ini.folder_path = folder_path;
+
+    // See kPicasaINIFilename declaration for details.
+    if (file_util::ReadFileToString(
+            folder_path.AppendASCII(kPicasaINIFilename),
+            &folder_ini.ini_contents) ||
+        file_util::ReadFileToString(
+            folder_path.AppendASCII(kPicasaINIFilenameLegacy),
+            &folder_ini.ini_contents)) {
+      folders_inis_.push_back(folder_ini);
+    }
+  }
+
+  if (!folders_queue.empty()) {

[vandebo (ex-Chrome), 2013/07/10 17:43:52]

Should this be folder_inis_?

[tommycli, 2013/07/10 18:43:32]

No. While queue is not empty, post itself onto the task runner again.

[vandebo (ex-Chrome), 2013/07/10 19:54:03]

Hmm, right.

[tommycli, 2013/07/10 22:22:27]

Done.

+    MediaFileSystemBackend::MediaTaskRunner()->PostTask(
+        FROM_HERE,
+        base::Bind(SafePicasaAlbumsIndexer::ProcessFoldersBatch,
+                   weak_factory_.GetWeakPtr()));
+  } else {
+    parser_state_ = FINISHED_READING_INI_FILES_STATE;

[vandebo (ex-Chrome), 2013/07/10 17:43:52]

This seems ok for now.  It still has the disadvantage that you're reading all of
the inis into memory at once.  If you do the utility process in batch mode you
could send kPicasaINIReadBatchSize worth of data at a time.

[tommycli, 2013/07/10 18:43:32]

Yeah I agree we should keep that idea in our back pocket. Lei said that for his
8000 photos, he had 330 .picasa.ini files that totaled 427KB.

+    BrowserThread::PostTask(
+        BrowserThread::IO,
+        FROM_HERE,
+        base::Bind(&SafePicasaAlbumsIndexer::StartWorkOnIOThread, this));
+  }
+}
+
+void SafePicasaAlbumsIndexer::StartWorkOnIOThread() {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  DCHECK_EQ(FINISHED_READING_INI_FILES_STATE, parser_state_);
+
+  std::vector<picasa::FolderINIContents> folder_inis;
+
+  UtilityProcessHost* host =
+      UtilityProcessHost::Create(this, base::MessageLoopProxy::current());
+  host->EnableZygote();
+  host->Send(new ChromeUtilityMsg_IndexPicasaAlbumsContents(album_uids_,
+                                                            folder_inis));
+  parser_state_ = STARTED_PARSING_STATE;
+}
+
+void SafePicasaAlbumsIndexer::OnIndexPicasaAlbumsContentsFinished(
+    const AlbumImagesMap& albums_images) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  if (parser_state_ != STARTED_PARSING_STATE)
+    return;
+
+  MediaFileSystemBackend::MediaTaskRunner()->PostTask(
+      FROM_HERE,
+      base::Bind(callback_, albums_images));
+  parser_state_ = FINISHED_PARSING_STATE;
+}
+
+void SafePicasaAlbumsIndexer::OnProcessCrashed(int exit_code) {
+  OnIndexPicasaAlbumsContentsFinished(AlbumImagesMap());
+}
+
+bool SafePicasaAlbumsIndexer::OnMessageReceived(
+    const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(SafePicasaAlbumsIndexer, message)
+    IPC_MESSAGE_HANDLER(
+        ChromeUtilityHostMsg_IndexPicasaAlbumsContents_Finished,
+        OnIndexPicasaAlbumsContentsFinished)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+  return handled;
+}
+
+}  // namespace picasa