0

Revert "Create display::Screen on demand on MAC"

This reverts commit c7f0333da8.

Reason for revert: The revert didn't fix the root cause of
crbug.com/1349955. Turns out that there is a conflict with
PartitionAlloc and macos11 and PA will be disabled on macos for now.

Original change's description:
> Create display::Screen on demand on MAC
>
> This reverts part of MAC changes in crrev.com/c/3685951 which
> creates Mac's screen at fixed point. This was causing a crash on
> macOS11 due to a bug in macOS11.
>
> Bug: 1349955,1350722
>
> Change-Id: I5dd11a4a7d5122a932c4fa199c2dc1d0a0363673
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3812323
> Reviewed-by: Avi Drissman <avi@chromium.org>
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Commit-Queue: Prudhvikumar Bommana <pbommana@google.com>
> Owners-Override: Avi Drissman <avi@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1032348}

Bug: 1349955,1350722
Change-Id: I43624c5f6a0c9dc09d43b1c9f6bd9a2526e49f88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3817743
Auto-Submit: Mitsuru Oshima <oshima@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Andrey Kosyakov <caseq@chromium.org>
Reviewed-by: Avi Drissman <avi@chromium.org>
Commit-Queue: Mitsuru Oshima <oshima@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032971}
This commit is contained in:
Mitsuru Oshima
2022-08-09 12:04:56 +00:00
committed by Chromium LUCI CQ
parent 82e81a4abb
commit c0a0320efb
15 changed files with 85 additions and 27 deletions

@ -24,6 +24,10 @@ namespace apps {
class MachBootstrapAcceptorTest; class MachBootstrapAcceptorTest;
} }
namespace display {
class ScopedNativeScreen;
}
@class AppShimDelegate; @class AppShimDelegate;
@class ProfileMenuTarget; @class ProfileMenuTarget;
@class ApplicationDockMenuTarget; @class ApplicationDockMenuTarget;
@ -189,6 +193,9 @@ class AppShimController : public chrome::mojom::AppShim {
base::scoped_nsobject<ApplicationDockMenuTarget> base::scoped_nsobject<ApplicationDockMenuTarget>
application_dock_menu_target_; application_dock_menu_target_;
// The screen object used in the app sim.
std::unique_ptr<display::ScopedNativeScreen> screen_;
// The items in the profile menu. // The items in the profile menu.
std::vector<chrome::mojom::ProfileMenuItemPtr> profile_menu_items_; std::vector<chrome::mojom::ProfileMenuItemPtr> profile_menu_items_;

@ -43,6 +43,7 @@
#include "mojo/public/cpp/platform/platform_channel.h" #include "mojo/public/cpp/platform/platform_channel.h"
#include "ui/accelerated_widget_mac/window_resize_helper_mac.h" #include "ui/accelerated_widget_mac/window_resize_helper_mac.h"
#include "ui/base/l10n/l10n_util.h" #include "ui/base/l10n/l10n_util.h"
#include "ui/display/screen.h"
#include "ui/gfx/image/image.h" #include "ui/gfx/image/image.h"
// The ProfileMenuTarget bridges between Objective C (as the target for the // The ProfileMenuTarget bridges between Objective C (as the target for the
@ -130,8 +131,9 @@ AppShimController::AppShimController(const Params& params)
profile_menu_target_([[ProfileMenuTarget alloc] initWithController:this]), profile_menu_target_([[ProfileMenuTarget alloc] initWithController:this]),
application_dock_menu_target_( application_dock_menu_target_(
[[ApplicationDockMenuTarget alloc] initWithController:this]) { [[ApplicationDockMenuTarget alloc] initWithController:this]) {
// Since AppShimController is created before the main message loop starts, screen_ = std::make_unique<display::ScopedNativeScreen>();
// NSApp will not be set, so use sharedApplication. // Since AppShimController is created before the main message loop starts,
// NSApp will not be set, so use sharedApplication.
NSApplication* sharedApplication = [NSApplication sharedApplication]; NSApplication* sharedApplication = [NSApplication sharedApplication];
[sharedApplication setDelegate:delegate_]; [sharedApplication setDelegate:delegate_];
} }

@ -6093,6 +6093,8 @@ static_library("browser") {
"mac/auth_session_request.mm", "mac/auth_session_request.mm",
"mac/bluetooth_utility.h", "mac/bluetooth_utility.h",
"mac/bluetooth_utility.mm", "mac/bluetooth_utility.mm",
"mac/chrome_browser_main_extra_parts_mac.h",
"mac/chrome_browser_main_extra_parts_mac.mm",
"mac/dock.h", "mac/dock.h",
"mac/dock.mm", "mac/dock.mm",
"mac/exception_processor.h", "mac/exception_processor.h",

@ -344,6 +344,7 @@
#include "chrome/browser/browser_process_platform_part_mac.h" #include "chrome/browser/browser_process_platform_part_mac.h"
#include "chrome/browser/chrome_browser_main_mac.h" #include "chrome/browser/chrome_browser_main_mac.h"
#include "chrome/browser/mac/auth_session_request.h" #include "chrome/browser/mac/auth_session_request.h"
#include "chrome/browser/mac/chrome_browser_main_extra_parts_mac.h"
#include "components/soda/constants.h" #include "components/soda/constants.h"
#include "sandbox/mac/seatbelt_exec.h" #include "sandbox/mac/seatbelt_exec.h"
#include "sandbox/policy/mac/params.h" #include "sandbox/policy/mac/params.h"
@ -1525,6 +1526,10 @@ ChromeContentBrowserClient::CreateBrowserMainParts(bool is_integration_test) {
#endif #endif
#endif #endif
#if BUILDFLAG(IS_MAC)
main_parts->AddParts(std::make_unique<ChromeBrowserMainExtraPartsMac>());
#endif
#if BUILDFLAG(IS_CHROMEOS_ASH) #if BUILDFLAG(IS_CHROMEOS_ASH)
// TODO(jamescook): Combine with `ChromeBrowserMainPartsAsh`. // TODO(jamescook): Combine with `ChromeBrowserMainPartsAsh`.
main_parts->AddParts(std::make_unique<ChromeBrowserMainExtraPartsAsh>()); main_parts->AddParts(std::make_unique<ChromeBrowserMainExtraPartsAsh>());

@ -0,0 +1,32 @@
// Copyright 2022 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 CHROME_BROWSER_MAC_CHROME_BROWSER_MAIN_EXTRA_PARTS_MAC_H_
#define CHROME_BROWSER_MAC_CHROME_BROWSER_MAIN_EXTRA_PARTS_MAC_H_
#include <memory>
#include "chrome/browser/chrome_browser_main_extra_parts.h"
namespace display {
class ScopedNativeScreen;
}
class ChromeBrowserMainExtraPartsMac : public ChromeBrowserMainExtraParts {
public:
ChromeBrowserMainExtraPartsMac();
ChromeBrowserMainExtraPartsMac(const ChromeBrowserMainExtraPartsMac&) =
delete;
ChromeBrowserMainExtraPartsMac& operator=(
const ChromeBrowserMainExtraPartsMac&) = delete;
~ChromeBrowserMainExtraPartsMac() override;
// ChromeBrowserMainExtraParts:
void PreEarlyInitialization() override;
private:
std::unique_ptr<display::ScopedNativeScreen> screen_;
};
#endif // CHROME_BROWSER_MAC_CHROME_BROWSER_MAIN_EXTRA_PARTS_MAC_H_

@ -0,0 +1,14 @@
// Copyright 2022 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/mac/chrome_browser_main_extra_parts_mac.h"
#include "ui/display/screen.h"
ChromeBrowserMainExtraPartsMac::ChromeBrowserMainExtraPartsMac() = default;
ChromeBrowserMainExtraPartsMac::~ChromeBrowserMainExtraPartsMac() = default;
void ChromeBrowserMainExtraPartsMac::PreEarlyInitialization() {
screen_ = std::make_unique<display::ScopedNativeScreen>();
}

@ -27,10 +27,7 @@ namespace {
class TestScreen : public display::ScreenBase { class TestScreen : public display::ScreenBase {
public: public:
TestScreen() TestScreen() : previous_screen_(display::Screen::GetScreen()) {
: previous_screen_(display::Screen::HasScreen()
? display::Screen::GetScreen()
: nullptr) {
display::Screen::SetScreenInstance(this); display::Screen::SetScreenInstance(this);
} }
@ -50,7 +47,6 @@ class TestScreen : public display::ScreenBase {
} }
private: private:
// TODO(crbug.com/1350722): Remove this.
raw_ptr<display::Screen> previous_screen_; raw_ptr<display::Screen> previous_screen_;
}; };

@ -15,6 +15,7 @@
#if BUILDFLAG(IS_MAC) #if BUILDFLAG(IS_MAC)
#include "content/public/browser/native_web_keyboard_event.h" #include "content/public/browser/native_web_keyboard_event.h"
#include "ui/display/screen.h"
#endif #endif
class GURL; class GURL;
@ -139,6 +140,9 @@ class ShellPlatformDelegate {
#endif #endif
private: private:
#if BUILDFLAG(IS_MAC)
std::unique_ptr<display::ScopedNativeScreen> screen_;
#endif
// Data held for each Shell instance, since there is one ShellPlatformDelegate // Data held for each Shell instance, since there is one ShellPlatformDelegate
// for the whole browser process (shared across Shells). This is defined for // for the whole browser process (shared across Shells). This is defined for
// each platform implementation. // each platform implementation.

@ -134,7 +134,9 @@ struct ShellPlatformDelegate::PlatformData {};
ShellPlatformDelegate::ShellPlatformDelegate() = default; ShellPlatformDelegate::ShellPlatformDelegate() = default;
ShellPlatformDelegate::~ShellPlatformDelegate() = default; ShellPlatformDelegate::~ShellPlatformDelegate() = default;
void ShellPlatformDelegate::Initialize(const gfx::Size& default_window_size) {} void ShellPlatformDelegate::Initialize(const gfx::Size& default_window_size) {
screen_ = std::make_unique<display::ScopedNativeScreen>();
}
void ShellPlatformDelegate::CreatePlatformWindow( void ShellPlatformDelegate::CreatePlatformWindow(
Shell* shell, Shell* shell,

@ -29,6 +29,10 @@ class PolicyService;
} // namespace policy } // namespace policy
#endif #endif
#if BUILDFLAG(IS_MAC)
#include "ui/display/screen.h"
#endif
namespace ui { namespace ui {
class Compositor; class Compositor;
} // namespace ui } // namespace ui
@ -118,6 +122,10 @@ class HEADLESS_EXPORT HeadlessBrowserImpl : public HeadlessBrowser,
#endif #endif
protected: protected:
#if BUILDFLAG(IS_MAC)
std::unique_ptr<display::ScopedNativeScreen> screen_;
#endif
base::OnceCallback<void(HeadlessBrowser*)> on_start_callback_; base::OnceCallback<void(HeadlessBrowser*)> on_start_callback_;
HeadlessBrowser::Options options_; HeadlessBrowser::Options options_;
raw_ptr<HeadlessBrowserMainParts> browser_main_parts_; // Not owned. raw_ptr<HeadlessBrowserMainParts> browser_main_parts_; // Not owned.

@ -65,6 +65,7 @@ const NSActivityOptions kActivityOptions =
} // namespace } // namespace
void HeadlessBrowserImpl::PlatformInitialize() { void HeadlessBrowserImpl::PlatformInitialize() {
screen_ = std::make_unique<display::ScopedNativeScreen>();
HeadlessPopUpMethods::Init(); HeadlessPopUpMethods::Init();
} }

@ -31,14 +31,7 @@ Screen::~Screen() = default;
// static // static
Screen* Screen::GetScreen() { Screen* Screen::GetScreen() {
// Create on the fly on iOS and Mac. #if BUILDFLAG(IS_IOS)
//
// iOS: iOS's screen is initialized using static functions and difficult to
// reset. Just create once.
//
// macOS: Creating a screen too early during startup can cause a crash on
// macos11. https:://crbug.com/1349955.
#if BUILDFLAG(IS_IOS) || BUILDFLAG(IS_MAC)
if (!g_screen) if (!g_screen)
g_screen = CreateNativeScreen(); g_screen = CreateNativeScreen();
#endif #endif
@ -236,7 +229,7 @@ ScopedNativeScreen::~ScopedNativeScreen() {
void ScopedNativeScreen::MaybeInit(const base::Location& location) { void ScopedNativeScreen::MaybeInit(const base::Location& location) {
maybe_init_called_ = true; maybe_init_called_ = true;
if (!Screen::HasScreen()) { if (!Screen::HasScreen()) {
#if BUILDFLAG(IS_IOS) || BUILDFLAG(IS_MAC) #if BUILDFLAG(IS_IOS)
Screen::GetScreen(); Screen::GetScreen();
#else #else
screen_ = base::WrapUnique(CreateScreen()); screen_ = base::WrapUnique(CreateScreen());

@ -10,9 +10,7 @@ namespace display {
namespace test { namespace test {
ScopedScreenOverride::ScopedScreenOverride(Screen* screen) ScopedScreenOverride::ScopedScreenOverride(Screen* screen)
: original_screen_(display::Screen::HasScreen() : original_screen_(display::Screen::GetScreen()) {
? display::Screen::GetScreen()
: nullptr) {
display::Screen::SetScreenInstance(screen); display::Screen::SetScreenInstance(screen);
} }

@ -14,7 +14,6 @@ class Screen;
namespace test { namespace test {
// [Deprecated] Do not use this in new code. // [Deprecated] Do not use this in new code.
// TODO(crbug.com/1350722): Remove this class.
// //
// This class represents a RAII wrapper for global screen overriding. An object // This class represents a RAII wrapper for global screen overriding. An object
// of this class restores original display::Screen instance when it goes out of // of this class restores original display::Screen instance when it goes out of

@ -34,12 +34,7 @@ TestScreen::~TestScreen() {
// static // static
TestScreen* TestScreen::Get() { TestScreen* TestScreen::Get() {
#if DCHECK_IS_ON() DCHECK_EQ(Screen::GetScreen(), test_screen);
if (Screen::HasScreen())
DCHECK_EQ(Screen::GetScreen(), test_screen);
else
DCHECK(!test_screen);
#endif
return test_screen; return test_screen;
} }