[fuchsia] Remove single-Context assumption from WebEngine.

Allow multiple ContextImpls to co-exist in a single WebEngine instance, by moving their ownership to a BrowserMainParts-owned BindingSet<>. Each ContextImpl now also owns its own BrowserContext. Bug: 1163073 Change-Id: I92fe51baf750cde07069f3be919c618bb7afd85d Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2780472 Commit-Queue: Sergey Ulanov <sergeyu@chromium.org> Commit-Queue: David Dorwin <ddorwin@chromium.org> Reviewed-by: David Dorwin <ddorwin@chromium.org> Auto-Submit: Wez <wez@chromium.org> Cr-Commit-Position: refs/heads/master@{#868337}
chromium · Apr 1, 2021 · 5f3d773 · 5f3d773
1 parent be58450
commit 5f3d773
Show file tree

Hide file tree

Showing 9 changed files with 106 additions and 90 deletions.
diff --git a/fuchsia/engine/browser/context_impl.cc b/fuchsia/engine/browser/context_impl.cc
@@ -22,14 +22,13 @@
 #include "third_party/blink/public/common/web_preferences/web_preferences.h"
 #include "third_party/blink/public/mojom/webpreferences/web_preferences.mojom.h"
 
-ContextImpl::ContextImpl(content::BrowserContext* browser_context,
-                         WebEngineDevToolsController* devtools_controller)
-    : browser_context_(browser_context),
+ContextImpl::ContextImpl(
+    std::unique_ptr<content::BrowserContext> browser_context,
+    WebEngineDevToolsController* devtools_controller)
+    : browser_context_(std::move(browser_context)),
       devtools_controller_(devtools_controller),
-      cookie_manager_(base::BindRepeating(
-          &content::StoragePartition::GetNetworkContext,
-          base::Unretained(content::BrowserContext::GetDefaultStoragePartition(
-              browser_context_)))) {
+      cookie_manager_(base::BindRepeating(&ContextImpl::GetNetworkContext,
+                                          base::Unretained(this))) {
   DCHECK(browser_context_);
   DCHECK(devtools_controller_);
   devtools_controller_->OnContextCreated();
@@ -73,7 +72,8 @@ void ContextImpl::CreateFrameWithParams(
   }
 
   // Create a WebContents to host the new Frame.
-  content::WebContents::CreateParams create_params(browser_context_, nullptr);
+  content::WebContents::CreateParams create_params(browser_context_.get(),
+                                                   nullptr);
   create_params.initially_hidden = true;
   auto web_contents = content::WebContents::Create(create_params);
 
@@ -211,6 +211,7 @@ FrameImpl* ContextImpl::GetFrameImplForTest(
 
 network::mojom::NetworkContext* ContextImpl::GetNetworkContext() {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  return content::BrowserContext::GetDefaultStoragePartition(browser_context_)
+  return content::BrowserContext::GetDefaultStoragePartition(
+             browser_context_.get())
       ->GetNetworkContext();
 }
diff --git a/fuchsia/engine/browser/context_impl.h b/fuchsia/engine/browser/context_impl.h
@@ -34,8 +34,8 @@ class WebEngineDevToolsController;
 // All created Frames are owned by this object.
 class WEB_ENGINE_EXPORT ContextImpl : public fuchsia::web::Context {
  public:
-  // |browser_context| and |devtools_controller| must outlive ContextImpl.
-  ContextImpl(content::BrowserContext* browser_context,
+  // |devtools_controller| must outlive ContextImpl.
+  ContextImpl(std::unique_ptr<content::BrowserContext> browser_context,
               WebEngineDevToolsController* devtools_controller);
 
   // Tears down the Context, destroying any active Frames in the process.
@@ -76,16 +76,16 @@ class WEB_ENGINE_EXPORT ContextImpl : public fuchsia::web::Context {
   // |frame_ptr| client.
   FrameImpl* GetFrameImplForTest(fuchsia::web::FramePtr* frame_ptr) const;
 
-  content::BrowserContext* browser_context_for_test() const {
-    return browser_context_;
+  content::BrowserContext* browser_context() const {
+    return browser_context_.get();
   }
 
  private:
   // Returns the NetworkContext from the default StoragePartition.
   network::mojom::NetworkContext* GetNetworkContext();
 
   // Reference to the browser implementation for this Context.
-  content::BrowserContext* const browser_context_;
+  std::unique_ptr<content::BrowserContext> const browser_context_;
 
   // Reference to the class managing the DevTools remote debugging service.
   WebEngineDevToolsController* const devtools_controller_;

diff --git a/fuchsia/engine/browser/explicit_sites_filter_browsertest.cc b/fuchsia/engine/browser/explicit_sites_filter_browsertest.cc
@@ -50,7 +50,7 @@ class ExplicitSitesFilterTest : public FrameImplTestBaseWithServer {
     FrameImplTestBaseWithServer::SetUpOnMainThread();
 
     SafeSearchFactory::GetInstance()
-        ->GetForBrowserContext(context_impl()->browser_context_for_test())
+        ->GetForBrowserContext(context_impl()->browser_context())
         ->SetSafeSearchURLCheckerForTest(
             stub_url_checker_.BuildURLChecker(kUrlCheckerCacheSize));
   }

diff --git a/fuchsia/engine/browser/web_engine_browser_context.cc b/fuchsia/engine/browser/web_engine_browser_context.cc
@@ -11,13 +11,11 @@
 #include "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
-#include "base/i18n/rtl.h"
 #include "base/path_service.h"
 #include "base/system/sys_info.h"
 #include "components/keyed_service/content/browser_context_dependency_manager.h"
 #include "components/keyed_service/core/simple_key_map.h"
 #include "components/site_isolation/site_isolation_policy.h"
-#include "components/strings/grit/components_locale_settings.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/resource_context.h"
@@ -27,7 +25,6 @@
 #include "media/capabilities/in_memory_video_decode_stats_db_impl.h"
 #include "media/mojo/services/video_decode_perf_history.h"
 #include "services/network/public/cpp/network_switches.h"
-#include "ui/base/l10n/l10n_util.h"
 
 namespace {
 
@@ -199,10 +196,6 @@ WebEngineBrowserContext::GetVideoDecodePerfHistory() {
   return BrowserContext::GetVideoDecodePerfHistory();
 }
 
-std::string WebEngineBrowserContext::GetPreferredLanguages() const {
-  return l10n_util::GetStringUTF8(IDS_ACCEPT_LANGUAGES);
-}
-
 media::VideoDecodePerfHistory*
 WebEngineBrowserContext::GetInMemoryVideoDecodePerfHistory() {
   constexpr char kUserDataKeyName[] = "video-decode-perf-history";

diff --git a/fuchsia/engine/browser/web_engine_browser_context.h b/fuchsia/engine/browser/web_engine_browser_context.h
@@ -44,12 +44,6 @@ class WebEngineBrowserContext : public content::BrowserContext {
       override;
   media::VideoDecodePerfHistory* GetVideoDecodePerfHistory() override;
 
-  // Returns a comma-separated list of language codes, in preference order.
-  // This is suitable for direct use setting the "sec-ch-lang" header, or
-  // passed to net::HttpUtil::GenerateAcceptLanguageHeader() to generate a
-  // legacy "accept-language" header value.
-  std::string GetPreferredLanguages() const;
-
  private:
   // Contains URLRequestContextGetter required for resource loading.
   class ResourceContext;

diff --git a/fuchsia/engine/browser/web_engine_browser_main_parts.cc b/fuchsia/engine/browser/web_engine_browser_main_parts.cc
@@ -15,6 +15,7 @@
 #include "base/i18n/rtl.h"
 #include "base/logging.h"
 #include "base/threading/thread_task_runner_handle.h"
+#include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/gpu_data_manager.h"
 #include "content/public/browser/histogram_fetcher.h"
 #include "content/public/browser/render_frame_host.h"
@@ -57,16 +58,28 @@ void FetchHistogramsFromChildProcesses(
 }  // namespace
 
 WebEngineBrowserMainParts::WebEngineBrowserMainParts(
+    content::ContentBrowserClient* browser_client,
     const content::MainFunctionParams& parameters,
     fidl::InterfaceRequest<fuchsia::web::Context> request)
-    : parameters_(parameters), request_(std::move(request)) {
+    : browser_client_(browser_client),
+      parameters_(parameters),
+      request_(std::move(request)) {
   DCHECK(request_);
 }
 
 WebEngineBrowserMainParts::~WebEngineBrowserMainParts() {
   display::Screen::SetScreenInstance(nullptr);
 }
 
+std::vector<content::BrowserContext*>
+WebEngineBrowserMainParts::browser_contexts() const {
+  std::vector<content::BrowserContext*> contexts;
+  contexts.reserve(context_bindings_.size());
+  for (auto& binding : context_bindings_.bindings())
+    contexts.push_back(binding->impl()->browser_context());
+  return contexts;
+}
+
 void WebEngineBrowserMainParts::PostEarlyInitialization() {
   base::ImportantFileWriterCleaner::GetInstance().Initialize();
 }
@@ -93,16 +106,27 @@ int WebEngineBrowserMainParts::PreMainMessageLoopRun() {
     gpu_data_manager->DisableHardwareAcceleration();
   }
 
-  DCHECK(!browser_context_);
-  browser_context_ = std::make_unique<WebEngineBrowserContext>(
+  DCHECK_EQ(context_bindings_.size(), 0u);
+  auto browser_context = std::make_unique<WebEngineBrowserContext>(
       base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kIncognito));
 
   devtools_controller_ = WebEngineDevToolsController::CreateFromCommandLine(
       *base::CommandLine::ForCurrentProcess());
-  context_service_ = std::make_unique<ContextImpl>(browser_context_.get(),
-                                                   devtools_controller_.get());
-  context_binding_ = std::make_unique<fidl::Binding<fuchsia::web::Context>>(
-      context_service_.get(), std::move(request_));
+
+  // TODO(crbug.com/1163073): Instead of creating a single ContextImpl instance
+  // the code below should public fuchsia.web.Context to the outgoing directory.
+  // Also it shouldn't quit main loop after Context disconnects.
+  context_bindings_.AddBinding(
+      std::make_unique<ContextImpl>(std::move(browser_context),
+                                    devtools_controller_.get()),
+      std::move(request_), /* dispatcher */ nullptr,
+      // Quit the browser main loop when the Context connection is dropped.
+      [this](zx_status_t status) {
+        ZX_LOG_IF(ERROR, status != ZX_ERR_PEER_CLOSED, status)
+            << " Context disconnected.";
+        // context_service_.reset();
+        std::move(quit_closure_).Run();
+      });
 
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
           switches::kUseLegacyMetricsService)) {
@@ -123,22 +147,13 @@ int WebEngineBrowserMainParts::PreMainMessageLoopRun() {
   media_resource_provider_service_ =
       std::make_unique<MediaResourceProviderService>();
 
-  // Quit the browser main loop when the Context connection is dropped.
-  context_binding_->set_error_handler([this](zx_status_t status) {
-    ZX_LOG_IF(ERROR, status != ZX_ERR_PEER_CLOSED, status)
-        << " Context disconnected.";
-    context_service_.reset();
-    std::move(quit_closure_).Run();
-  });
-
   // Disable RenderFrameHost's Javascript injection restrictions so that the
   // Context and Frames can implement their own JS injection policy at a higher
   // level.
   content::RenderFrameHost::AllowInjectingJavaScript();
 
   if (parameters_.ui_task) {
-    // Since the main loop won't run, there is nothing to quit in the
-    // |context_binding_| error handler.
+    // Since the main loop won't run, there is nothing to quit.
     quit_closure_ = base::DoNothing::Once();
 
     std::move(*parameters_.ui_task).Run();
@@ -161,23 +176,24 @@ void WebEngineBrowserMainParts::WillRunMainMessageLoop(
 }
 
 void WebEngineBrowserMainParts::PostMainMessageLoopRun() {
-  // The service and its binding should have already been released by the error
-  // handler.
-  DCHECK(!context_service_);
-  DCHECK(!context_binding_->is_bound());
+  // Main loop should quit only after all Context instances have been destroyed.
+  DCHECK_EQ(context_bindings_.size(), 0u);
 
   // These resources must be freed while a MessageLoop is still available, so
   // that they may post cleanup tasks during teardown.
-  // NOTE: Please destroy objects in the reverse order of their creation.
+  // NOTE: Objects are destroyed in the reverse order of their creation.
   legacy_metrics_client_.reset();
-  context_binding_.reset();
-  browser_context_.reset();
   screen_.reset();
   intl_profile_watcher_.reset();
 
   base::ImportantFileWriterCleaner::GetInstance().Stop();
 }
 
+ContextImpl* WebEngineBrowserMainParts::context_for_test() const {
+  DCHECK_EQ(context_bindings_.size(), 1u);
+  return context_bindings_.bindings().front()->impl().get();
+}
+
 void WebEngineBrowserMainParts::OnIntlProfileChanged(
     const fuchsia::intl::Profile& profile) {
   // Configure the ICU library in this process with the new primary locale.
@@ -191,9 +207,14 @@ void WebEngineBrowserMainParts::OnIntlProfileChanged(
           base::i18n::GetConfiguredLocale());
   VLOG(1) << "Reloaded locale resources: " << loaded_locale;
 
-  // Reconfigure the network process.
-  content::BrowserContext::GetDefaultStoragePartition(browser_context_.get())
-      ->GetNetworkContext()
-      ->SetAcceptLanguage(net::HttpUtil::GenerateAcceptLanguageHeader(
-          browser_context_->GetPreferredLanguages()));
+  // Reconfigure each web.Context's NetworkContext with the new setting.
+  for (auto& binding : context_bindings_.bindings()) {
+    content::BrowserContext* const browser_context =
+        binding->impl()->browser_context();
+    std::string accept_language = net::HttpUtil::GenerateAcceptLanguageHeader(
+        browser_client_->GetAcceptLangs(browser_context));
+    content::BrowserContext::GetDefaultStoragePartition(browser_context)
+        ->GetNetworkContext()
+        ->SetAcceptLanguage(accept_language);
+  }
 }
diff --git a/fuchsia/engine/browser/web_engine_browser_main_parts.h b/fuchsia/engine/browser/web_engine_browser_main_parts.h
@@ -15,6 +15,7 @@
 #include "content/public/browser/browser_main_parts.h"
 #include "fuchsia/engine/browser/context_impl.h"
 #include "fuchsia/engine/browser/web_engine_browser_context.h"
+#include "fuchsia/engine/web_engine_export.h"
 
 namespace base {
 class FuchsiaIntlProfileWatcher;
@@ -25,6 +26,7 @@ class Screen;
 }
 
 namespace content {
+class ContentBrowserClient;
 struct MainFunctionParams;
 }
 
@@ -34,16 +36,16 @@ class LegacyMetricsClient;
 
 class MediaResourceProviderService;
 
-class WebEngineBrowserMainParts : public content::BrowserMainParts {
+class WEB_ENGINE_EXPORT WebEngineBrowserMainParts
+    : public content::BrowserMainParts {
  public:
   explicit WebEngineBrowserMainParts(
+      content::ContentBrowserClient* browser_client,
       const content::MainFunctionParams& parameters,
       fidl::InterfaceRequest<fuchsia::web::Context> request);
   ~WebEngineBrowserMainParts() override;
 
-  content::BrowserContext* browser_context() const {
-    return browser_context_.get();
-  }
+  std::vector<content::BrowserContext*> browser_contexts() const;
   WebEngineDevToolsController* devtools_controller() const {
     return devtools_controller_.get();
   }
@@ -58,19 +60,19 @@ class WebEngineBrowserMainParts : public content::BrowserMainParts {
       std::unique_ptr<base::RunLoop>& run_loop) override;
   void PostMainMessageLoopRun() override;
 
-  ContextImpl* context_for_test() const { return context_service_.get(); }
+  ContextImpl* context_for_test() const;
 
  private:
   void OnIntlProfileChanged(const fuchsia::intl::Profile& profile);
 
+  content::ContentBrowserClient* const browser_client_;
   const content::MainFunctionParams& parameters_;
 
   fidl::InterfaceRequest<fuchsia::web::Context> request_;
 
   std::unique_ptr<display::Screen> screen_;
-  std::unique_ptr<WebEngineBrowserContext> browser_context_;
-  std::unique_ptr<ContextImpl> context_service_;
-  std::unique_ptr<fidl::Binding<fuchsia::web::Context>> context_binding_;
+  fidl::BindingSet<fuchsia::web::Context, std::unique_ptr<ContextImpl>>
+      context_bindings_;
   std::unique_ptr<WebEngineDevToolsController> devtools_controller_;
   std::unique_ptr<cr_fuchsia::LegacyMetricsClient> legacy_metrics_client_;
   std::unique_ptr<MediaResourceProviderService>