Commit ba3ae098 authored by Tim Schumann's avatar Tim Schumann Committed by Commit Bot

Revert "[Paint Preview] Add font access for Linux"

This reverts commit 4951143a.

Reason for revert: PaintPreviewCompositorCollectionBrowserTest.TestInitializationSuccess fails on linux-chromeos-google-rel

CI: https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-google-rel?limit=100
Instance: https://ci.chromium.org/p/chrome/builders/ci/linux-chromeos-google-rel/9587

example logs:
[ RUN      ] PaintPreviewCompositorCollectionBrowserTest.TestInitializationSuccess
[1:1:1123/144534.806495:WARNING:hugepage_text.cc(182)] The ordering seems incorrect, fall back to small page
[17651:17651:1123/144534.888293:WARNING:audio_manager_linux.cc(52)] Falling back to ALSA for audio output. PulseAudio is not available or could not be initialized.
[17651:17651:1123/144534.982920:INFO:profile_network_context_service.cc(206)] Using built-in cert verifier.
[17651:17651:1123/144534.988049:ERROR:proximity_auth_profile_pref_manager.cc(189)] Failed to find local state prefs for current user.
[17651:17651:1123/144535.011423:INFO:profile_network_context_service.cc(206)] Using built-in cert verifier.
[17651:17651:1123/144535.017906:INFO:profile_network_context_service.cc(206)] Using built-in cert verifier.
[17651:17651:1123/144535.019873:INFO:remote_commands_service.cc(40)] Fetching remote commands.
[17651:17651:1123/144535.019945:WARNING:remote_commands_service.cc(42)] Client is not registered.
[17651:17651:1123/144535.019973:INFO:remote_commands_invalidator.cc(34)] Initialize RemoteCommandsInvalidator.
[17651:17651:1123/144535.019993:INFO:remote_commands_invalidator.cc(59)] Starting RemoteCommandsInvalidator.
[17651:17651:1123/144535.020014:INFO:remote_commands_invalidator.cc(130)] RemoteCommandsInvalidator ReloadPolicyData.
[17651:17651:1123/144535.020034:INFO:remote_commands_invalidator.cc(172)] Unregister RemoteCommandsInvalidator.
[17651:17651:1123/144535.022879:WARNING:wallpaper_controller_client.cc(370)] Cannot get wallpaper files id in RemovePolicyWallpaper. This should never happen under normal circumstances.
[17651:17651:1123/144535.064656:ERROR:account_manager_migrator.cc(248)] Could not find a refresh token for the Device Account.
[17651:17733:1123/144535.158896:WARNING:google_brand_chromeos.cc(39)] Brand code file missing: /opt/oem/etc/BRAND_CODE
../../content/public/test/browser_test_base.cc:605: Failure
Failed
RunLoop::Run() timed out.
Stack trace:
#0 0x55f14751ff09 (/b/s/w/ir/out/Release/browser_tests+0x70a8f08)

[17651:17651:1123/144605.264003:INFO:remote_commands_invalidator.cc(47)] Shutdown RemoteCommandsInvalidator.
[17651:17651:1123/144605.264078:INFO:remote_commands_invalidator.cc(70)] Stopping RemoteCommandsInvalidator.
[17651:17651:1123/144605.264095:INFO:remote_commands_invalidator.cc(172)] Unregister RemoteCommandsInvalidator.
[17651:17651:1123/144605.271554:WARNING:pref_notifier_impl.cc(40)] Pref observer for native_printing.external_print_servers_whitelist found at shutdown.
[17651:17651:1123/144605.307536:WARNING:pref_notifier_impl.cc(40)] Pref observer for media_router.cast_allow_all_ips found at shutdown.
[17651:17651:1123/144605.307567:WARNING:pref_notifier_impl.cc(40)] Pref observer for EulaAccepted found at shutdown.
[17651:17651:1123/144605.307579:WARNING:pref_notifier_impl.cc(40)] Pref observer for browser.relaunch_heads_up_period found at shutdown.
[17651:17651:1123/144605.307587:WARNING:pref_notifier_impl.cc(40)] Pref observer for browser.relaunch_notification_period found at shutdown.
[17651:17651:1123/144605.307595:WARNING:pref_notifier_impl.cc(40)] Pref observer for browser.relaunch_notification found at shutdown.
[  FAILED  ] PaintPreviewCompositorCollectionBrowserTest.TestInitializationSuccess, where TypeParam =  and GetParam() =  (30599 ms)

Original change's description:
> [Paint Preview] Add font access for Linux
> 
> Testing on Cluster Telemetry with Linux found an issue where SkFontMgr
> wasn't initialized properly resulting in inability to start the
> compositor. This is due to the lack of font access since we didn't
> call EnsureBlinkInitializedWithSandbox. However, the compositor only
> needs a fraction of the features that come with that call. This CL
> starts the font_service which enables the SkFontMgr to initialize. Note
> that we don't require WebSandboxSupport to be initialized (at least for
> Linux) as all the fonts are stored in the provided SkPictures.
> 
> Test coverage for this will be added as part of the end-to-end tests
> for the player. That should catch any issues with other platforms.
> 
> Bug: 1023377
> Change-Id: I95a224f6f3759b1b8f8df2989222757ebbd4f12e
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1908062
> Commit-Queue: Calder Kitagawa <ckitagawa@chromium.org>
> Reviewed-by: Jochen Eisinger <jochen@chromium.org>
> Reviewed-by: Dominik Röttsches <drott@chromium.org>
> Reviewed-by: Ian Vollick <vollick@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#718149}

TBR=vollick@chromium.org,drott@chromium.org,jochen@chromium.org,ckitagawa@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 1023377
Change-Id: I0de7093d6612f2836ff8bfce30db5758d06da258
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1932797Reviewed-by: default avatarTim Schumann <tschumann@chromium.org>
Commit-Queue: Tim Schumann <tschumann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#718460}
parent 6e95466a
include_rules = [
"+components/services/paint_preview_compositor",
]
// Copyright 2019 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 <utility>
#include <vector>
#include "base/bind.h"
#include "base/no_destructor.h"
#include "base/run_loop.h"
#include "base/strings/utf_string_conversions.h"
#include "base/task/post_task.h"
#include "base/unguessable_token.h"
#include "chrome/test/base/in_process_browser_test.h"
#include "components/services/paint_preview_compositor/paint_preview_compositor_collection_impl.h"
#include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h"
#include "content/public/browser/service_process_host.h"
#include "mojo/public/cpp/bindings/remote.h"
#include "mojo/public/cpp/bindings/service_factory.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace paint_preview {
class PaintPreviewCompositorCollectionBrowserTest
: public InProcessBrowserTest {
public:
PaintPreviewCompositorCollectionBrowserTest() = default;
~PaintPreviewCompositorCollectionBrowserTest() override = default;
protected:
mojo::Remote<mojom::PaintPreviewCompositorCollection> StartUtilityProcess() {
return content::ServiceProcessHost::Launch<
mojom::PaintPreviewCompositorCollection>(
content::ServiceProcessHost::Options()
.WithSandboxType(service_manager::SANDBOX_TYPE_PDF_COMPOSITOR)
.Pass());
}
};
IN_PROC_BROWSER_TEST_F(PaintPreviewCompositorCollectionBrowserTest,
TestInitializationSuccess) {
base::RunLoop loop;
mojo::Remote<mojom::PaintPreviewCompositorCollection> compositor_collection =
StartUtilityProcess();
EXPECT_TRUE(compositor_collection.is_bound());
EXPECT_TRUE(compositor_collection.is_connected());
// If the compositor_collection hasn't crashed during initialization due to
// lacking font support then this call should succeed.
compositor_collection->ListCompositors(base::BindOnce(
[](base::OnceClosure quit,
const std::vector<base::UnguessableToken>& ids) {
EXPECT_EQ(ids.size(), 0U);
std::move(quit).Run();
},
loop.QuitClosure()));
loop.Run();
}
} // namespace paint_preview
...@@ -696,7 +696,6 @@ if (!is_android) { ...@@ -696,7 +696,6 @@ if (!is_android) {
"//components/resources", "//components/resources",
"//components/safe_browsing:buildflags", "//components/safe_browsing:buildflags",
"//components/safe_browsing/db:test_database_manager", "//components/safe_browsing/db:test_database_manager",
"//components/services/paint_preview_compositor/public/mojom",
"//components/services/patch/public/mojom", "//components/services/patch/public/mojom",
"//components/services/quarantine:test_support", "//components/services/quarantine:test_support",
"//components/signin/core/browser", "//components/signin/core/browser",
...@@ -1041,7 +1040,6 @@ if (!is_android) { ...@@ -1041,7 +1040,6 @@ if (!is_android) {
"../browser/page_load_metrics/observers/subresource_loading_page_load_metrics_observer_browsertest.cc", "../browser/page_load_metrics/observers/subresource_loading_page_load_metrics_observer_browsertest.cc",
"../browser/page_load_metrics/observers/third_party_metrics_observer_browsertest.cc", "../browser/page_load_metrics/observers/third_party_metrics_observer_browsertest.cc",
"../browser/page_load_metrics/page_load_metrics_browsertest.cc", "../browser/page_load_metrics/page_load_metrics_browsertest.cc",
"../browser/paint_preview/paint_preview_compositor_browsertest.cc",
"../browser/password_manager/credential_manager_browsertest.cc", "../browser/password_manager/credential_manager_browsertest.cc",
"../browser/password_manager/password_manager_browsertest.cc", "../browser/password_manager/password_manager_browsertest.cc",
"../browser/pdf/pdf_extension_test.cc", "../browser/pdf/pdf_extension_test.cc",
......
...@@ -21,17 +21,12 @@ static_library("paint_preview_compositor") { ...@@ -21,17 +21,12 @@ static_library("paint_preview_compositor") {
"//components/discardable_memory/client", "//components/discardable_memory/client",
"//components/paint_preview/common", "//components/paint_preview/common",
"//components/paint_preview/common/proto", "//components/paint_preview/common/proto",
"//content/public/utility",
"//mojo/public/cpp/bindings", "//mojo/public/cpp/bindings",
"//skia", "//skia",
"//ui/gfx/geometry", "//ui/gfx/geometry",
"//url", "//url",
] ]
if (is_linux) {
deps += [ "//components/services/font/public/cpp" ]
}
if (is_win) { if (is_win) {
deps += [ "//content/public/child" ] deps += [ "//content/public/child" ]
} }
......
include_rules = [ include_rules = [
"+components/discardable_memory/client", "+components/discardable_memory/client",
"+components/paint_preview", "+components/paint_preview",
"+components/services/font/public",
"+content/public/child", # Windows direct write proxy access. "+content/public/child", # Windows direct write proxy access.
"+content/public/utility",
"+mojo/public/cpp", "+mojo/public/cpp",
"+third_party/skia/include", "+third_party/skia/include/core",
"+ui/gfx/geometry", "+ui/gfx/geometry",
] ]
...@@ -9,14 +9,10 @@ ...@@ -9,14 +9,10 @@
#include "base/memory/discardable_memory.h" #include "base/memory/discardable_memory.h"
#include "base/memory/discardable_memory_allocator.h" #include "base/memory/discardable_memory_allocator.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "content/public/utility/utility_thread.h"
#include "third_party/skia/include/core/SkFontMgr.h" #include "third_party/skia/include/core/SkFontMgr.h"
#include "third_party/skia/include/ports/SkFontConfigInterface.h"
#if defined(OS_WIN) #if defined(OS_WIN)
#include "content/public/child/dwrite_font_proxy_init_win.h" #include "content/public/child/dwrite_font_proxy_init_win.h"
#elif defined(OS_LINUX)
#include "components/services/font/public/cpp/font_loader.h"
#endif #endif
namespace paint_preview { namespace paint_preview {
...@@ -32,21 +28,10 @@ PaintPreviewCompositorCollectionImpl::PaintPreviewCompositorCollectionImpl( ...@@ -32,21 +28,10 @@ PaintPreviewCompositorCollectionImpl::PaintPreviewCompositorCollectionImpl(
if (!initialize_environment) if (!initialize_environment)
return; return;
// Initialize font access for Skia.
#if defined(OS_WIN) #if defined(OS_WIN)
// Initialize direct write font proxy so skia can use it.
content::InitializeDWriteFontProxy(); content::InitializeDWriteFontProxy();
#elif defined(OS_LINUX)
mojo::PendingRemote<font_service::mojom::FontService> font_service;
content::UtilityThread::Get()->BindHostReceiver(
font_service.InitWithNewPipeAndPassReceiver());
font_loader_ = sk_make_sp<font_service::FontLoader>(std::move(font_service));
SkFontConfigInterface::SetGlobal(font_loader_);
#endif #endif
// TODO(crbug/1023377): Determine if EnsureBlinkInitialized*() does any other
// initialization we require. Possibly for other platforms (e.g. MacOS,
// Android). In theory, WebSandboxSupport isn't required since we subset and
// load all required fonts into the Skia Pictures for portability so they are
// all local; however, this may be required for initialization on MacOS?
// TODO(crbug/1013585): PDF compositor initializes Blink to leverage some // TODO(crbug/1013585): PDF compositor initializes Blink to leverage some
// codecs for images. This is a huge overhead and shouldn't be necessary for // codecs for images. This is a huge overhead and shouldn't be necessary for
...@@ -54,15 +39,7 @@ PaintPreviewCompositorCollectionImpl::PaintPreviewCompositorCollectionImpl( ...@@ -54,15 +39,7 @@ PaintPreviewCompositorCollectionImpl::PaintPreviewCompositorCollectionImpl(
// encoding to PNG or we could provide our own codec implementations. // encoding to PNG or we could provide our own codec implementations.
// Sanity check that fonts are working. // Sanity check that fonts are working.
#if defined(OS_LINUX)
// No WebSandbox is provided on Linux so the local fonts aren't accessible.
// This is fine since since the subsetted fonts are provided in the SkPicture.
// However, we still need to check that the SkFontMgr starts as it is used by
// Skia when handling the SkPicture.
DCHECK(SkFontMgr::RefDefault());
#else
DCHECK(SkFontMgr::RefDefault()->countFamilies()); DCHECK(SkFontMgr::RefDefault()->countFamilies());
#endif
} }
PaintPreviewCompositorCollectionImpl::~PaintPreviewCompositorCollectionImpl() { PaintPreviewCompositorCollectionImpl::~PaintPreviewCompositorCollectionImpl() {
......
...@@ -12,18 +12,12 @@ ...@@ -12,18 +12,12 @@
#include "base/memory/scoped_refptr.h" #include "base/memory/scoped_refptr.h"
#include "base/single_thread_task_runner.h" #include "base/single_thread_task_runner.h"
#include "base/unguessable_token.h" #include "base/unguessable_token.h"
#include "build/build_config.h"
#include "components/discardable_memory/client/client_discardable_shared_memory_manager.h" #include "components/discardable_memory/client/client_discardable_shared_memory_manager.h"
#include "components/services/paint_preview_compositor/paint_preview_compositor_impl.h" #include "components/services/paint_preview_compositor/paint_preview_compositor_impl.h"
#include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h" #include "components/services/paint_preview_compositor/public/mojom/paint_preview_compositor.mojom.h"
#include "mojo/public/cpp/bindings/pending_receiver.h" #include "mojo/public/cpp/bindings/pending_receiver.h"
#include "mojo/public/cpp/bindings/receiver.h" #include "mojo/public/cpp/bindings/receiver.h"
#if defined(OS_LINUX)
#include "components/services/font/public/cpp/font_loader.h"
#include "third_party/skia/include/core/SkRefCnt.h"
#endif
namespace discardable_memory { namespace discardable_memory {
class ClientDiscardableSharedMemoryManager; class ClientDiscardableSharedMemoryManager;
} }
...@@ -68,10 +62,6 @@ class PaintPreviewCompositorCollectionImpl ...@@ -68,10 +62,6 @@ class PaintPreviewCompositorCollectionImpl
std::unique_ptr<PaintPreviewCompositorImpl>> std::unique_ptr<PaintPreviewCompositorImpl>>
compositors_; compositors_;
#if defined(OS_LINUX)
sk_sp<font_service::FontLoader> font_loader_;
#endif
PaintPreviewCompositorCollectionImpl( PaintPreviewCompositorCollectionImpl(
const PaintPreviewCompositorCollectionImpl&) = delete; const PaintPreviewCompositorCollectionImpl&) = delete;
PaintPreviewCompositorCollectionImpl& operator=( PaintPreviewCompositorCollectionImpl& operator=(
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment