From a8a5c8fe6a2b8692f9b9d12f50b40155abfa8819 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 16 Nov 2022 15:52:07 +0000 Subject: [PATCH] Bug 1798213 - For widget-less pages keep defaulting to primary screen scale factor. r=tnikkel,layout-reviewers,extension-reviewers,robwu This restores the previous behavior in a somewhat more principled way. The extensions code is still broken in multi-monitor cases, but that's a more complicated fix. Differential Revision: https://phabricator.services.mozilla.com/D161997 --- dom/ipc/BrowserParent.cpp | 49 ++++++++++--------- .../test/mochitest/mochitest-common.ini | 1 + .../test_ext_background_page_dpi.html | 46 +++++++++++++++++ widget/PuppetWidget.h | 4 +- widget/Screen.cpp | 20 +++++--- widget/Screen.h | 3 ++ widget/nsBaseWidget.cpp | 12 +++++ widget/nsIWidget.h | 10 ++++ 8 files changed, 114 insertions(+), 31 deletions(-) create mode 100644 toolkit/components/extensions/test/mochitest/test_ext_background_page_dpi.html diff --git a/dom/ipc/BrowserParent.cpp b/dom/ipc/BrowserParent.cpp index 567a8abfec9f..d16a45f42a98 100644 --- a/dom/ipc/BrowserParent.cpp +++ b/dom/ipc/BrowserParent.cpp @@ -473,10 +473,10 @@ ParentShowInfo BrowserParent::GetShowInfo() { TryCacheDPIAndScale(); if (mFrameElement) { nsAutoString name; - mFrameElement->GetAttr(kNameSpaceID_None, nsGkAtoms::name, name); + mFrameElement->GetAttr(nsGkAtoms::name, name); bool isTransparent = nsContentUtils::IsChromeDoc(mFrameElement->OwnerDoc()) && - mFrameElement->HasAttr(kNameSpaceID_None, nsGkAtoms::transparent); + mFrameElement->HasAttr(nsGkAtoms::transparent); return ParentShowInfo(name, false, isTransparent, mDPI, mRounding, mDefaultScale.scale); } @@ -546,6 +546,10 @@ void BrowserParent::SetOwnerElement(Element* aElement) { #endif AddWindowListeners(); + + // The DPI depends on our frame element's widget, so invalidate now in case + // we've tried to cache it already. + mDPI = -1; TryCacheDPIAndScale(); if (mRemoteLayerTreeOwner.IsInitialized()) { @@ -3427,17 +3431,17 @@ void BrowserParent::TryCacheDPIAndScale() { return; } + const auto oldDefaultScale = mDefaultScale; nsCOMPtr widget = GetWidget(); + mDPI = widget ? widget->GetDPI() : nsIWidget::GetFallbackDPI(); + mRounding = widget ? widget->RoundsWidgetCoordinatesTo() : 1; + mDefaultScale = + widget ? widget->GetDefaultScale() : nsIWidget::GetFallbackDefaultScale(); - if (widget) { - mDPI = widget->GetDPI(); - mRounding = widget->RoundsWidgetCoordinatesTo(); - if (mDefaultScale != widget->GetDefaultScale()) { - // The change of the default scale factor will affect the child dimensions - // so we need to invalidate it. - mUpdatedDimensions = false; - } - mDefaultScale = widget->GetDefaultScale(); + if (mDefaultScale != oldDefaultScale) { + // The change of the default scale factor will affect the child dimensions + // so we need to invalidate it. + mUpdatedDimensions = false; } } @@ -3582,18 +3586,19 @@ void BrowserParent::PreserveLayers(bool aPreserveLayers) { } void BrowserParent::NotifyResolutionChanged() { - if (!mIsDestroyed) { - // TryCacheDPIAndScale()'s cache is keyed off of - // mDPI being greater than 0, so this invalidates it. - mDPI = -1; - TryCacheDPIAndScale(); - // If mDPI was set to -1 to invalidate it and then TryCacheDPIAndScale - // fails to cache the values, then mDefaultScale.scale might be invalid. - // We don't want to send that value to content. Just send -1 for it too in - // that case. - Unused << SendUIResolutionChanged(mDPI, mRounding, - mDPI < 0 ? -1.0 : mDefaultScale.scale); + if (mIsDestroyed) { + return; } + // TryCacheDPIAndScale()'s cache is keyed off of + // mDPI being greater than 0, so this invalidates it. + mDPI = -1; + TryCacheDPIAndScale(); + // If mDPI was set to -1 to invalidate it and then TryCacheDPIAndScale + // fails to cache the values, then mDefaultScale.scale might be invalid. + // We don't want to send that value to content. Just send -1 for it too in + // that case. + Unused << SendUIResolutionChanged(mDPI, mRounding, + mDPI < 0 ? -1.0 : mDefaultScale.scale); } bool BrowserParent::CanCancelContentJS( diff --git a/toolkit/components/extensions/test/mochitest/mochitest-common.ini b/toolkit/components/extensions/test/mochitest/mochitest-common.ini index 805d79b0b558..77a8631bcb0c 100644 --- a/toolkit/components/extensions/test/mochitest/mochitest-common.ini +++ b/toolkit/components/extensions/test/mochitest/mochitest-common.ini @@ -87,6 +87,7 @@ skip-if = toolkit == 'android' || tsan # near-permafail after landing bug 127005 [test_ext_background_canvas.html] [test_ext_background_page.html] skip-if = (toolkit == 'android') # android doesn't have devtools +[test_ext_background_page_dpi.html] [test_ext_browserAction_openPopup.html] [test_ext_browserAction_openPopup_incognito_window.html] skip-if = os == "android" # cannot open private windows - bug 1372178 diff --git a/toolkit/components/extensions/test/mochitest/test_ext_background_page_dpi.html b/toolkit/components/extensions/test/mochitest/test_ext_background_page_dpi.html new file mode 100644 index 000000000000..40772402b143 --- /dev/null +++ b/toolkit/components/extensions/test/mochitest/test_ext_background_page_dpi.html @@ -0,0 +1,46 @@ + + +DPI of background page + + + + + diff --git a/widget/PuppetWidget.h b/widget/PuppetWidget.h index d78c35950c12..27b32c2bde11 100644 --- a/widget/PuppetWidget.h +++ b/widget/PuppetWidget.h @@ -382,9 +382,9 @@ class PuppetWidget : public nsBaseWidget, ContentCacheInChild mContentCache; // The DPI of the parent widget containing this widget. - float mDPI = 96; + float mDPI = GetFallbackDPI(); int32_t mRounding = 1; - double mDefaultScale = 1.0f; + double mDefaultScale = GetFallbackDefaultScale().scale; ScreenIntMargin mSafeAreaInsets; diff --git a/widget/Screen.cpp b/widget/Screen.cpp index 4962d1c9bac7..19a95acba522 100644 --- a/widget/Screen.cpp +++ b/widget/Screen.cpp @@ -129,15 +129,21 @@ Screen::GetContentsScaleFactor(double* aOutScale) { return NS_OK; } +CSSToLayoutDeviceScale Screen::GetCSSToLayoutDeviceScale( + IncludeOSZoom aIncludeOSZoom) const { + auto scale = CSSToLayoutDeviceScale(StaticPrefs::layout_css_devPixelsPerPx()); + if (scale.scale <= 0.0) { + scale = mDefaultCssScale; + } + if (bool(aIncludeOSZoom)) { + scale.scale *= LookAndFeel::SystemZoomSettings().mFullZoom; + } + return scale; +} + NS_IMETHODIMP Screen::GetDefaultCSSScaleFactor(double* aOutScale) { - double scale = StaticPrefs::layout_css_devPixelsPerPx(); - if (scale > 0.0) { - *aOutScale = scale; - } else { - *aOutScale = mDefaultCssScale.scale; - } - *aOutScale *= LookAndFeel::SystemZoomSettings().mFullZoom; + *aOutScale = GetCSSToLayoutDeviceScale(IncludeOSZoom::Yes).scale; return NS_OK; } diff --git a/widget/Screen.h b/widget/Screen.h index ac890e8ab26f..056a45a51d7f 100644 --- a/widget/Screen.h +++ b/widget/Screen.h @@ -51,6 +51,9 @@ class Screen final : public nsIScreen { return mContentsScale; } + enum class IncludeOSZoom : bool { No, Yes }; + CSSToLayoutDeviceScale GetCSSToLayoutDeviceScale(IncludeOSZoom) const; + private: virtual ~Screen() = default; diff --git a/widget/nsBaseWidget.cpp b/widget/nsBaseWidget.cpp index f3b47d625f29..c5756a2fee7b 100644 --- a/widget/nsBaseWidget.cpp +++ b/widget/nsBaseWidget.cpp @@ -59,6 +59,7 @@ #include "mozilla/layers/InputAPZContext.h" #include "mozilla/layers/WebRenderLayerManager.h" #include "mozilla/webrender/WebRenderTypes.h" +#include "mozilla/widget/ScreenManager.h" #include "nsAppDirectoryServiceDefs.h" #include "nsCOMPtr.h" #include "nsContentUtils.h" @@ -2105,6 +2106,17 @@ void nsIWidget::OnLongTapTimerCallback(nsITimer* aTimer, void* aClosure) { self->mLongTapTouchPoint = nullptr; } +float nsIWidget::GetFallbackDPI() { + RefPtr primaryScreen = + ScreenManager::GetSingleton().GetPrimaryScreen(); + return primaryScreen->GetDPI(); +} + +CSSToLayoutDeviceScale nsIWidget::GetFallbackDefaultScale() { + RefPtr s = ScreenManager::GetSingleton().GetPrimaryScreen(); + return s->GetCSSToLayoutDeviceScale(Screen::IncludeOSZoom::No); +} + nsresult nsIWidget::ClearNativeTouchSequence(nsIObserver* aObserver) { AutoObserverNotifier notifier(aObserver, "cleartouch"); diff --git a/widget/nsIWidget.h b/widget/nsIWidget.h index 3c5e39a9c2d7..b8db3c0973d6 100644 --- a/widget/nsIWidget.h +++ b/widget/nsIWidget.h @@ -594,6 +594,11 @@ class nsIWidget : public nsISupports { */ virtual float GetDPI() = 0; + /** + * Fallback DPI for when there's no widget available. + */ + static float GetFallbackDPI(); + /** * Return the scaling factor between device pixels and the platform- * dependent "desktop pixels" used to manage window positions on a @@ -618,6 +623,11 @@ class nsIWidget : public nsISupports { */ mozilla::CSSToLayoutDeviceScale GetDefaultScale(); + /** + * Fallback default scale for when there's no widget available. + */ + static mozilla::CSSToLayoutDeviceScale GetFallbackDefaultScale(); + /** * Return the first child of this widget. Will return null if * there are no children.