[fuchsia] Expose configurations to the Fuchsia Hub.

* Add a read-only file, "config-default.json", to the Hub debug
  directory for the WebEngine component containing the default
  configuration.
* Add a read/write file "config-override.json" to the Hub debug
  directory for the WebEngine component allowing to override the
  configuration for all subsequently created Contexts.
* Add "blink-settings" as an allowed command-line argument to
  configurations.

Bug: 1014670
Change-Id: I4be28996bcfdd598836aa64c1fc2c59a0ac4432a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2032425Reviewed-by: David Dorwin <ddorwin@chromium.org>
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Fabrice de Gans-Riberi <fdegans@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744411}

fuchsia/base/BUILD.gn

@@ -15,12 +15,14 @@ source_set("base") {
     "config_reader.cc",
     "init_logging.cc",
     "mem_buffer_util.cc",
+    "scoped_pseudo_file_publisher.cc",
     "string_util.cc",
   ]
   public = [
     "config_reader.h",
     "init_logging.h",
     "mem_buffer_util.h",
+    "scoped_pseudo_file_publisher.h",
     "string_util.h",
   ]
   deps = [

fuchsia/base/scoped_pseudo_file_publisher.cc

+// Copyright 2020 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 "fuchsia/base/scoped_pseudo_file_publisher.h"
+
+#include <lib/vfs/cpp/pseudo_dir.h>
+#include <lib/vfs/cpp/pseudo_file.h>
+
+#include "base/fuchsia/fuchsia_logging.h"
+#include "base/logging.h"
+
+namespace cr_fuchsia {
+
+ScopedPseudoFilePublisher::ScopedPseudoFilePublisher() = default;
+
+ScopedPseudoFilePublisher::ScopedPseudoFilePublisher(
+    vfs::PseudoDir* pseudo_dir,
+    base::StringPiece filename,
+    std::unique_ptr<vfs::PseudoFile> pseudo_file)
+    : pseudo_dir_(pseudo_dir), filename_(filename) {
+  DCHECK(pseudo_dir_);
+
+  zx_status_t status = pseudo_dir_->AddEntry(filename_, std::move(pseudo_file));
+  ZX_DCHECK(status == ZX_OK, status);
+}
+
+ScopedPseudoFilePublisher::~ScopedPseudoFilePublisher() {
+  if (pseudo_dir_) {
+    zx_status_t status = pseudo_dir_->RemoveEntry(filename_);
+    ZX_DCHECK(status == ZX_OK, status);
+  }
+}
+
+ScopedPseudoFilePublisher::ScopedPseudoFilePublisher(
+    ScopedPseudoFilePublisher&& other)
+    : pseudo_dir_(other.pseudo_dir_), filename_(std::move(other.filename_)) {
+  other.pseudo_dir_ = nullptr;
+}
+
+ScopedPseudoFilePublisher& ScopedPseudoFilePublisher::operator=(
+    ScopedPseudoFilePublisher&& other) {
+  // |tmp| destructor will be called when it gets out of scope, ensuring |this|
+  // stops publishing |filename_| if needs be.
+  ScopedPseudoFilePublisher tmp(std::move(other));
+  std::swap(tmp.pseudo_dir_, pseudo_dir_);
+  std::swap(tmp.filename_, filename_);
+  return *this;
+}
+
+}  // namespace cr_fuchsia

fuchsia/base/scoped_pseudo_file_publisher.h

+// Copyright 2020 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.
+
+#ifndef FUCHSIA_BASE_SCOPED_PSEUDO_FILE_PUBLISHER_H_
+#define FUCHSIA_BASE_SCOPED_PSEUDO_FILE_PUBLISHER_H_
+
+#include <memory>
+#include <string>
+
+#include "base/strings/string_piece.h"
+
+namespace vfs {
+class PseudoDir;
+class PseudoFile;
+}  // namespace vfs
+
+namespace cr_fuchsia {
+
+// Links |pseudo_file| at the specified |filename| under the specified
+// |pseudo_dir|, and unlinks it when going out of scope.
+class ScopedPseudoFilePublisher {
+ public:
+  ScopedPseudoFilePublisher();
+  ScopedPseudoFilePublisher(vfs::PseudoDir* pseudo_dir,
+                            base::StringPiece filename,
+                            std::unique_ptr<vfs::PseudoFile> pseudo_file);
+  ~ScopedPseudoFilePublisher();
+
+  ScopedPseudoFilePublisher(ScopedPseudoFilePublisher&&);
+  ScopedPseudoFilePublisher& operator=(ScopedPseudoFilePublisher&&);
+
+  ScopedPseudoFilePublisher(const ScopedPseudoFilePublisher&) = delete;
+  ScopedPseudoFilePublisher& operator=(const ScopedPseudoFilePublisher&) =
+      delete;
+
+ private:
+  vfs::PseudoDir* pseudo_dir_ = nullptr;
+  std::string filename_;
+};
+
+}  // namespace cr_fuchsia
+
+#endif  // FUCHSIA_BASE_SCOPED_PSEUDO_FILE_PUBLISHER_H_

fuchsia/engine/context_provider_impl.cc

@@ -9,6 +9,8 @@
 #include <lib/fdio/directory.h>
 #include <lib/fdio/fd.h>
 #include <lib/fdio/io.h>
+#include <lib/sys/cpp/component_context.h>
+#include <lib/vfs/cpp/pseudo_file.h>
 #include <lib/zx/job.h>
 #include <stdio.h>
 #include <sys/stat.h>
@@ -27,9 +29,11 @@
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/files/scoped_file.h"
+#include "base/fuchsia/default_context.h"
 #include "base/fuchsia/default_job.h"
 #include "base/fuchsia/fuchsia_logging.h"
 #include "base/json/json_reader.h"
+#include "base/json/json_writer.h"
 #include "base/logging.h"
 #include "base/path_service.h"
 #include "base/process/launch.h"
@@ -43,6 +47,7 @@
 #include "components/viz/common/features.h"
 #include "content/public/common/content_switches.h"
 #include "fuchsia/base/config_reader.h"
+#include "fuchsia/base/string_util.h"
 #include "fuchsia/engine/common/web_engine_content_client.h"
 #include "fuchsia/engine/switches.h"
 #include "gpu/command_buffer/service/gpu_switches.h"
@@ -116,17 +121,6 @@ bool SetContentDirectoriesInCommandLine(
   return true;
 }
 
-base::Value LoadConfig() {
-  base::Optional<base::Value> config = cr_fuchsia::LoadPackageConfig();
-  if (!config) {
-    DLOG(WARNING) << "Configuration data not found. Using default "
-                     "WebEngine configuration.";
-    return base::Value(base::Value::Type::DICTIONARY);
-  }
-
-  return std::move(*config);
-}
-
 void AppendFeature(base::StringPiece features_flag,
                    base::StringPiece feature_string,
                    base::CommandLine* command_line) {
@@ -153,6 +147,8 @@ bool MaybeAddCommandLineArgsFromConfig(const base::Value& config,
 
   static const base::StringPiece kAllowedArgs[] = {
       switches::kAcceleratedCanvas2dMSAASampleCount,
+      // TODO(crbug.com/1054589): Remove the "blink-settings" argument.
+      switches::kBlinkSettings,
       switches::kDisableFeatures,
       switches::kDisableGpuWatchdog,
       switches::kEnableFeatures,
@@ -206,12 +202,82 @@ bool IsFuchsiaCdmSupported() {
 #endif
 }
 
+// Callback for vfs::PseudoFile read.
+zx_status_t OnReadConfig(const base::Value& config,
+                         std::vector<uint8_t>* output,
+                         size_t max_bytes) {
+  base::Value empty_config(base::Value::Type::DICTIONARY);
+  std::string json;
+
+  // This CHECK can be triggered if |config| has a depth higher than 200.
+  // However, for this use case, it should not happen.
+  CHECK(
+      base::JSONWriter::Write(config.is_none() ? empty_config : config, &json));
+
+  output->assign(json.begin(), json.end());
+  return ZX_OK;
+}
+
+// Callback for vfs::PseudoFile write.
+zx_status_t OnWriteConfig(base::Value* config, std::vector<uint8_t> input) {
+  base::Optional<base::Value> parsed =
+      base::JSONReader::Read(cr_fuchsia::BytesAsString(input));
+  if (!parsed || !parsed->is_dict())
+    return ZX_ERR_IO_REFUSED;
+
+  *config = std::move(parsed.value());
+  return ZX_OK;
+}
+
+cr_fuchsia::ScopedPseudoFilePublisher CreateAndPublishDebugFile(
+    base::StringPiece filename,
+    vfs::PseudoFile::ReadHandler read_fn,
+    vfs::PseudoFile::WriteHandler write_fn) {
+  // 32k configuration size ought to be enough for anyone.
+  constexpr size_t kMaxConfigSize = 32768;
+
+  vfs::PseudoDir* debug_dir = base::fuchsia::ComponentContextForCurrentProcess()
+                                  ->outgoing()
+                                  ->debug_dir();
+  std::unique_ptr<vfs::PseudoFile> pseudo_file =
+      std::make_unique<vfs::PseudoFile>(kMaxConfigSize, std::move(read_fn),
+                                        std::move(write_fn));
+  return cr_fuchsia::ScopedPseudoFilePublisher(debug_dir, filename,
+                                               std::move(pseudo_file));
+}
+
+constexpr char kConfigDefaultFileName[] = "config-default.json";
+constexpr char kConfigOverrideFileName[] = "config-override.json";
+
 }  // namespace
 
 const uint32_t ContextProviderImpl::kContextRequestHandleId =
     PA_HND(PA_USER0, 0);
 
-ContextProviderImpl::ContextProviderImpl() = default;
+ContextProviderImpl::ContextProviderImpl() {
+  base::Optional<base::Value> default_config = cr_fuchsia::LoadPackageConfig();
+  if (default_config) {
+    config_default_ = std::move(default_config.value());
+  } else {
+    config_default_ = base::Value(base::Value::Type::DICTIONARY);
+  }
+
+  config_default_file_ = CreateAndPublishDebugFile(
+      kConfigDefaultFileName,
+      [this](std::vector<uint8_t>* output, size_t max_bytes) {
+        return OnReadConfig(config_default_, output, max_bytes);
+      },
+      nullptr /* write_handler */);
+
+  config_override_file_ = CreateAndPublishDebugFile(
+      kConfigOverrideFileName,
+      [this](std::vector<uint8_t>* output, size_t max_bytes) {
+        return OnReadConfig(config_override_, output, max_bytes);
+      },
+      [this](std::vector<uint8_t> input) {
+        return OnWriteConfig(&config_override_, input);
+      });
+}
 
 ContextProviderImpl::~ContextProviderImpl() = default;
 
@@ -283,8 +349,7 @@ void ContextProviderImpl::Create(
   base::CommandLine launch_command = *base::CommandLine::ForCurrentProcess();
   std::vector<zx::channel> devtools_listener_channels;
 
-  base::Value web_engine_config =
-      config_for_test_.is_none() ? LoadConfig() : std::move(config_for_test_);
+  base::Value web_engine_config = LoadConfig();
   if (!MaybeAddCommandLineArgsFromConfig(web_engine_config, &launch_command)) {
     context_request.Close(ZX_ERR_INTERNAL);
     return;
@@ -505,6 +570,20 @@ void ContextProviderImpl::SetLaunchCallbackForTest(
   launch_for_test_ = std::move(launch);
 }
 
+base::Value ContextProviderImpl::LoadConfig() {
+  if (!config_override_.is_none())
+    return config_override_.Clone();
+
+  base::Optional<base::Value> config = cr_fuchsia::LoadPackageConfig();
+  if (!config) {
+    DLOG(WARNING) << "Configuration data not found. Using default "
+                     "WebEngine configuration.";
+    return base::Value(base::Value::Type::DICTIONARY);
+  }
+
+  return std::move(*config);
+}
+
 void ContextProviderImpl::EnableDevTools(
     fidl::InterfaceHandle<fuchsia::web::DevToolsListener> listener,
     EnableDevToolsCallback callback) {

fuchsia/engine/context_provider_impl.h

@@ -13,6 +13,7 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/values.h"
+#include "fuchsia/base/scoped_pseudo_file_publisher.h"
 #include "fuchsia/engine/web_engine_export.h"
 
 namespace base {
@@ -45,11 +46,14 @@ class WEB_ENGINE_EXPORT ContextProviderImpl
   void SetLaunchCallbackForTest(LaunchCallbackForTest launch);
 
   // Sets a config to use for the test, instead of looking for the config file.
-  void set_config_for_test(base::Value config) {
-    config_for_test_ = std::move(config);
+  void set_config_override_for_test(base::Value config) {
+    config_override_ = std::move(config);
   }
 
  private:
+  // Load the appropriate configuration.
+  base::Value LoadConfig();
+
   // fuchsia::web::Debug implementation.
   void EnableDevTools(
       fidl::InterfaceHandle<fuchsia::web::DevToolsListener> listener,
@@ -59,8 +63,14 @@ class WEB_ENGINE_EXPORT ContextProviderImpl
   // fake Context process to be launched.
   LaunchCallbackForTest launch_for_test_;
 
-  // Set by configuration tests.
-  base::Value config_for_test_;
+  // Default configuration value.
+  base::Value config_default_;
+  cr_fuchsia::ScopedPseudoFilePublisher config_default_file_;
+
+  // Configuration override to replace the default configuration, if non-empty.
+  // Setting this value affects all subsequently created Contexts.
+  base::Value config_override_;
+  cr_fuchsia::ScopedPseudoFilePublisher config_override_file_;
 
   // The DevToolsListeners registered via the Debug interface.
   fidl::InterfacePtrSet<fuchsia::web::DevToolsListener> devtools_listeners_;

fuchsia/engine/context_provider_impl_unittest.cc

@@ -373,7 +373,7 @@ TEST(ContextProviderImplConfigTest, WithConfigWithCommandLineArgs) {
 
   base::RunLoop loop;
   ContextProviderImpl context_provider;
-  context_provider.set_config_for_test(std::move(config_dict));
+  context_provider.set_config_override_for_test(std::move(config_dict));
   context_provider.SetLaunchCallbackForTest(
       base::BindLambdaForTesting([&loop](const base::CommandLine& command,
                                          const base::LaunchOptions& options) {
@@ -406,7 +406,7 @@ TEST(ContextProviderImplConfigTest, WithConfigWithDisallowedCommandLineArgs) {
 
   base::RunLoop loop;
   ContextProviderImpl context_provider;
-  context_provider.set_config_for_test(std::move(config_dict));
+  context_provider.set_config_override_for_test(std::move(config_dict));
   context_provider.SetLaunchCallbackForTest(
       base::BindLambdaForTesting([&loop](const base::CommandLine& command,
                                          const base::LaunchOptions& options) {
@@ -439,7 +439,7 @@ TEST(ContextProviderImplConfigTest, WithConfigWithWronglyTypedCommandLineArgs) {
 
   base::RunLoop loop;
   ContextProviderImpl context_provider;
-  context_provider.set_config_for_test(std::move(config_dict));
+  context_provider.set_config_override_for_test(std::move(config_dict));
   context_provider.SetLaunchCallbackForTest(
       base::BindLambdaForTesting([&](const base::CommandLine& command,
                                      const base::LaunchOptions& options) {

fuchsia/engine/web_engine_debug_integration_test.cc

@@ -9,13 +9,16 @@
 #include <lib/sys/cpp/component_context.h>
 
 #include "base/files/file_enumerator.h"
+#include "base/files/file_util.h"
 #include "base/fuchsia/default_context.h"
 #include "base/fuchsia/file_utils.h"
+#include "base/json/json_reader.h"
 #include "base/macros.h"
 #include "base/test/task_environment.h"
 #include "fuchsia/base/context_provider_test_connector.h"
 #include "fuchsia/base/fit_adapter.h"
 #include "fuchsia/base/frame_test_util.h"
+#include "fuchsia/base/mem_buffer_util.h"
 #include "fuchsia/base/result_receiver.h"
 #include "fuchsia/base/test_devtools_list_fetcher.h"
 #include "fuchsia/base/test_navigation_listener.h"
@@ -27,6 +30,27 @@ namespace {
 
 const char kTestServerRoot[] = FILE_PATH_LITERAL("fuchsia/engine/test/data");
 
+base::Optional<base::Value> ReadConfigFile(base::FilePath config_file_path) {
+  char data[4096];
+  int length = base::ReadFile(config_file_path, data, 4096);
+  if (length == -1)
+    return base::nullopt;
+
+  base::StringPiece json(data, length);
+  return base::JSONReader::Read(base::StringPiece(json));
+}
+
+bool WriteConfigFile(base::FilePath config_file_path,
+                     const base::StringPiece& config) {
+  base::File config_file(config_file_path,
+                         base::File::FLAG_OPEN | base::File::FLAG_WRITE);
+  if (config_file.error_details() != base::File::FILE_OK)
+    return false;
+
+  int result = config_file.Write(0, config.data(), config.size());
+  return result != -1;
+}
+
 }  // namespace
 
 class WebEngineDebugIntegrationTest : public testing::Test {
@@ -48,14 +72,14 @@ class WebEngineDebugIntegrationTest : public testing::Test {
     base::FileEnumerator file_enum(
         base::FilePath("/hub/c/context_provider.cmx"), false,
         base::FileEnumerator::DIRECTORIES);
-    base::FilePath web_engine_path = file_enum.Next();
-    ASSERT_FALSE(web_engine_path.empty());
+    web_engine_path_ = file_enum.Next();
+    ASSERT_FALSE(web_engine_path_.empty());
 
     // There should only be one instance of WebEngine in the realm.
     ASSERT_TRUE(file_enum.Next().empty());
 
     debug_dir_ = std::make_unique<sys::ServiceDirectory>(
-        base::fuchsia::OpenDirectory(web_engine_path.Append("out/debug")));
+        base::fuchsia::OpenDirectory(web_engine_path_.Append("out/debug")));
     debug_dir_->Connect(debug_.NewRequest());
 
     // Attach the DevToolsListener. EnableDevTools has an acknowledgement
@@ -102,6 +126,7 @@ class WebEngineDebugIntegrationTest : public testing::Test {
 
   TestDebugListener dev_tools_listener_;
   fidl::Binding<fuchsia::web::DevToolsListener> dev_tools_listener_binding_;
+  base::FilePath web_engine_path_;
   std::unique_ptr<sys::ServiceDirectory> debug_dir_;
   fuchsia::web::ContextProviderPtr web_context_provider_;
   fidl::InterfaceHandle<fuchsia::sys::ComponentController>
@@ -292,3 +317,94 @@ TEST_F(WebEngineDebugIntegrationTest, DebugAndUserService) {
   frame_data.context.Unbind();
   dev_tools_listener_.RunUntilNumberOfPortsIs(0);
 }
+
+// Checks the default and override configuration files in the debug directory
+// behave properly.
+// Due to potential side effects with other integration tests, these checks need
+// to be kept in a single test.
+TEST_F(WebEngineDebugIntegrationTest, ConfigOverride) {
+  base::FilePath default_config_path =
+      web_engine_path_.Append("out/debug/config-default.json");
+  base::FilePath config_override_path =
+      web_engine_path_.Append("out/debug/config-override.json");
+  const std::string kInvalidValue = "[foo";
+  const std::string kNonDictionayJson = "[\"foo\"]";
+  const std::string kDisableJavaScriptConfig =
+      "{\"command-line-args\":{\"blink-settings\":\"scriptEnabled=false\"}}";
+
+  {
+    // Read the default configuration. It should be a valid JSON dictionary.
+    base::Optional<base::Value> default_config =
+        ReadConfigFile(default_config_path);
+    ASSERT_TRUE(default_config);
+    EXPECT_TRUE(default_config->is_dict());
+
+    // Check the default configuration is read-only.
+    EXPECT_FALSE(WriteConfigFile(default_config_path, "foo"));
+  }
+
+  {
+    // Read the original config override, it should be an empty dictionary.
+    base::Optional<base::Value> config_override =
+        ReadConfigFile(config_override_path);
+    ASSERT_TRUE(config_override);
+    EXPECT_TRUE(config_override->is_dict());
+    EXPECT_TRUE(config_override->DictEmpty());
+  }
+
+  {
+    // Attempt to write a non-JSON value to the config override and read the
+    // value again, it should still be an empty dictionary.
+    ASSERT_TRUE(WriteConfigFile(config_override_path, kInvalidValue));
+    base::Optional<base::Value> config_override =
+        ReadConfigFile(config_override_path);
+    ASSERT_TRUE(config_override);
+    EXPECT_TRUE(config_override->is_dict());
+    EXPECT_TRUE(config_override->DictEmpty());
+  }
+
+  {
+    // Attempt to write a non-dictionary JSON value to the config override and
+    // read the value again, it should still be an empty dictionary.
+    ASSERT_TRUE(WriteConfigFile(config_override_path, kNonDictionayJson));
+    base::Optional<base::Value> config_override =
+        ReadConfigFile(config_override_path);
+    ASSERT_TRUE(config_override);
+    EXPECT_TRUE(config_override->is_dict());
+    EXPECT_TRUE(config_override->DictEmpty());
+  }
+
+  {
+    // Write a dictionary JSON value to the config override. The file should
+    // have been updated.
+    ASSERT_TRUE(
+        WriteConfigFile(config_override_path, kDisableJavaScriptConfig));
+    base::Optional<base::Value> config_override =
+        ReadConfigFile(config_override_path);
+    ASSERT_TRUE(config_override);
+    EXPECT_TRUE(config_override->is_dict());
+    EXPECT_FALSE(config_override->DictEmpty());
+
+    // Load a page and attempt to execute JavaScript. JavaScript execution
+    // should fail and return a "null" value.
+    std::string url = test_server_.GetURL("/title1.html").spec();
+    TestContextAndFrame frame_data(web_context_provider_.get(),
+                                   UserModeDebugging::kDisabled, url);
+
+    base::Optional<base::Value> value =
+        cr_fuchsia::ExecuteJavaScript(frame_data.frame.get(), "42;");
+    ASSERT_TRUE(value);
+    EXPECT_TRUE(value->is_none());
+  }
+
+  {
+    // Clear the config override and read the value again, it should be an empty
+    // dictionary.
+    ASSERT_TRUE(WriteConfigFile(config_override_path, "{}"));
+    base::Optional<base::Value> config_override =
+        ReadConfigFile(config_override_path);
+    ASSERT_TRUE(config_override);
+    EXPECT_TRUE(config_override->is_dict());
+    EXPECT_TRUE(config_override->DictEmpty());
+  }
+}