Introduces an additional extension loader that load extra extensions based on per-extension json fi…

chrome/browser/extensions/external_filesystem_extension_loader_linux.cc

+// Copyright (c) 2012 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 "base/bind.h"
+#include "base/dir_reader_posix.h"
+#include "base/file_path.h"
+#include "base/file_util.h"
+#include "base/json/json_file_value_serializer.h"
+#include "base/json/json_string_value_serializer.h"
+#include "base/memory/scoped_handle.h"
+#include "base/path_service.h"
+#include "base/string_util.h"
+#include "base/utf_string_conversions.h"
+#include "base/values.h"
+#include "base/version.h"
+#include "chrome/browser/extensions/external_extension_provider_impl.h"
+#include "chrome/browser/extensions/external_extension_util.h"
+#include "chrome/browser/extensions/external_filesystem_extension_loader_linux.h"
+#include "content/public/browser/browser_thread.h"
+
+using content::BrowserThread;
+
+namespace {
+
+std::set<FilePath> GetPrefsCandidateFilesFromFolder(
+      const FilePath& external_extension_search_path) {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+
+  std::set<FilePath> external_extension_paths;
+
+  if (!file_util::PathExists(external_extension_search_path)) {
+    // Does not have to exist.
+    return external_extension_paths;
+  }
+
+  base::DirReaderPosix
+      reader(external_extension_search_path.value().c_str());
+
+  if (!reader.IsValid()) {
+    LOG(ERROR) << "Can not read external extensions path '"
+               << external_extension_search_path.LossyDisplayName()
+               << "' although it appears to exist.";
+    return external_extension_paths;
+  }
+
+  while (reader.Next()) {
+    const FilePath filename(reader.name());
+
+    if (filename.MatchesExtension(".json")) {
+      external_extension_paths.insert(filename);
+    } else {
+      DVLOG(1) << "Not considering: " << reader.name()
+               << " (does not have a .json extension)";
+    }
+  }
+
+  return external_extension_paths;
+}
+
+}  // namespace
+
+ExternalFilesystemExtensionLoader::ExternalFilesystemExtensionLoader(
+    int base_path_key)
+    : base_path_key_(base_path_key) {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+}
+
+ExternalFilesystemExtensionLoader::~ExternalFilesystemExtensionLoader()
+{}

[Finnur, 2012/04/25 13:33:45]

We prefer: 
func() {
}

over
 
func()
{}

Also, while I'm a fan of keeping the ctor and dtor together the style guide says
declaration order and definition order should match (this recently came up on
the chromium-dev mailing list), so this needs to move down below StartLoading
(line 79).

[Alexandre Abreu, 2012/04/25 15:31:05]

Done.

+
+void ExternalFilesystemExtensionLoader::StartLoading() {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  BrowserThread::PostTask(
+      BrowserThread::FILE, FROM_HERE,
+      base::Bind(&ExternalFilesystemExtensionLoader::LoadOnFileThread, this));
+}
+
+const FilePath ExternalFilesystemExtensionLoader::GetBaseCrxFilePath() {

[Finnur, 2012/04/25 13:33:45]

Um... where is this function used and why is it important that we access this
member only on the UI thread?

[Alexandre Abreu, 2012/04/25 15:31:05]

it is used in external_extension_provider_impl.cc, if for a given standalone
extension, the actual extension's crx path is a relative path. The 'base CRX
file path' is then used as a base path.
It is a 'required' semantic as specified by the ExternalExtensionLoader class:

// Instances are created on the UI thread and expect public method calls from
// the UI thread. Some subclasses introduce new methods that are executed on the
// FILE thread.

+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+
+  // |extension_base_path_| was set in LoadOnFileThread().
+  return extension_base_path_;
+}
+
+void ExternalFilesystemExtensionLoader::LoadOnFileThread() {
+  CHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
+  scoped_ptr<DictionaryValue> prefs(new DictionaryValue);
+
+  if (!PathService::Get(base_path_key_,
+                        &extension_base_path_)) {
+    DVLOG(1) << "Could not retrieve per-extension search path";
+    return;
+  }
+
+  // First list the potential .json candidates.
+  std::set<FilePath>
+      candidates = GetPrefsCandidateFilesFromFolder(extension_base_path_);
+  if (candidates.empty()) {
+    DVLOG(1) << "Extension candidates list empty";
+    return;
+  }
+
+  // For each file read the json description & build the proper
+  // associated prefs.
+  FilePath extension_search_path = extension_base_path_;
+  for (std::set<FilePath>::const_iterator it = candidates.begin();
+       it != candidates.end();
+       ++it) {
+    FilePath extension_candidate_path =
+        extension_search_path.Append(*it);
+
+    FilePath::StringType id =
+        extension_candidate_path.RemoveExtension().BaseName().value();
+
+    DVLOG(1) << "Reading json file: " << extension_candidate_path.value();
+
+    JSONFileValueSerializer serializer(extension_candidate_path);
+    DictionaryValue* json_file_content =
+        ExternalExtensionUtil::ExtractExtensionPrefs(extension_candidate_path,
+            &serializer);

[Finnur, 2012/04/25 13:33:45]

When splitting arguments between lines, we prefer:

function(foo,
         bar);

... or (depending on which is more readable) ...

function(
    foo, bar);

... over ...

function(foo,
    bar);

[Alexandre Abreu, 2012/04/25 15:31:05]

Done.

+
+    if (NULL != json_file_content) {
+      DVLOG(1) << "Adding extension with id: " << id;
+
+      prefs->Set(id, json_file_content);
+    }
+  }
+
+  prefs_.reset(prefs.release());
+  BrowserThread::PostTask(
+      BrowserThread::UI, FROM_HERE,
+      base::Bind(&ExternalFilesystemExtensionLoader::LoadFinished, this));
+}