[webkit-changes] [WebKit/WebKit] 53929a: REGRESSION (270199 at main): [iOS] Viewport units are...

Aditya Keerthi noreply at github.com
Fri Jan 5 00:45:45 PST 2024


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 53929a96966c93804cd8749b573b538503e1665c
      https://github.com/WebKit/WebKit/commit/53929a96966c93804cd8749b573b538503e1665c
  Author: Aditya Keerthi <akeerthi at apple.com>
  Date:   2024-01-05 (Fri, 05 Jan 2024)

  Changed paths:
    M Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
    M Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
    M Source/WebKit/UIProcess/API/ios/WKWebViewIOS.h
    M Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm
    M Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm
    M Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
    A Tools/TestWebKitAPI/Tests/ios/FullscreenOverriddenLayoutParameters.mm

  Log Message:
  -----------
  REGRESSION (270199 at main): [iOS] Viewport units are incorrect in Safari after entering and exiting fullscreen
https://bugs.webkit.org/show_bug.cgi?id=267102
rdar://120496571

Reviewed by Jer Noble.

Following 270199 at main, entering and exiting fullscreen results in the overridden
layout parameters (viewLayoutSize, minimumUnobscuredSize, and maximumUnobscuredSize)
getting set to garbage values. This breaks viewport units in Safari, which are
derived from those values.

The intention of 270199 at main was to reset these values as if they were never set
while in fullscreen. That change implemented the following methods on `WKWebView`:

```
- (std::optional<CGSize>)_minimumUnobscuredSizeOverride;
- (std::optional<CGSize>)_maximumUnobscuredSizeOverride;
```

However, those methods were already implemented as SPI in another `WKWebView`
category, with different return types:

```
- (CGSize)_minimumUnobscuredSizeOverride;
- (CGSize)_maximumUnobscuredSizeOverride;
```

The end result of implementing the same method twice with different returns
types, is that `std::optional<CGSize>` is treated as `CGSize`, giving garbage
data when these values are attempted to be restored.

Fix by removing the duplicate methods, and refactoring the code to make the
nature of these optionals less confusing. Previously the ivars were optionals,
but the exposed values were CGSize. Now, they are grouped into a single optional
struct.

* Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _recalculateViewportSizesWithMinimumViewportInset:maximumViewportInset:throwOnInvalidInput:]):
* Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h:

The key idea behind the refactoring is that `_viewLayoutSizeOverride`,
`_minimumUnobscuredSizeOverride`, and `_maximumUnobscuredSizeOverride` are all
either `std::nullopt` or none of them are `std::nullopt`. This allows for leaving
the underlying type as `CGSize`, with only a single optional struct wrapping
the overridden parameters.

Ultimately, the confusion between methods and ivars with the same name but
different underlying types, which existed before 270199 at main, is removed.

* Source/WebKit/UIProcess/API/ios/WKWebViewIOS.h:
* Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:
(-[WKWebView _updateScrollViewForTransaction:]):
(-[WKWebView activeViewLayoutSize:]):
(-[WKWebView _frameOrBoundsMayHaveChanged]):
(activeMinimumUnobscuredSize):
(activeMaximumUnobscuredSize):
(-[WKWebView _didStopDeferringGeometryUpdates]):
(-[WKWebView _setAvoidsUnsafeArea:]):
(-[WKWebView _hasOverriddenLayoutParameters]):
(-[WKWebView _minimumLayoutSizeOverride]):
(-[WKWebView _minimumUnobscuredSizeOverride]):
(-[WKWebView _maximumUnobscuredSizeOverride]):
(-[WKWebView _beginAnimatedResizeWithUpdates:]):
(-[WKWebView _overrideLayoutParametersWithMinimumLayoutSize:minimumUnobscuredSizeOverride:maximumUnobscuredSizeOverride:]):
(-[WKWebView _clearOverrideLayoutParameters]):
(-[WKWebView _viewLayoutSizeOverride]): Deleted.
(-[WKWebView _setViewLayoutSizeOverride:]): Deleted.
(-[WKWebView _setMinimumUnobscuredSizeOverride:]): Deleted.
(-[WKWebView _setMaximumUnobscuredSizeOverride:]): Deleted.
* Source/WebKit/UIProcess/ios/fullscreen/WKFullScreenWindowControllerIOS.mm:
(WebKit::WKWebViewState::applyTo):

The saved values do not need to be optionals, since there is already a check for
`hasOverriddenLayoutParameters`.

* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/ios/FullscreenOverriddenLayoutParameters.mm: Added.
(swizzledPresentViewController):
(-[FullscreenOverriddenLayoutParametersWebView initWithFrame:configuration:]):
(-[FullscreenOverriddenLayoutParametersWebView enterFullscreen]):
(-[FullscreenOverriddenLayoutParametersWebView exitFullscreen]):
(-[FullscreenOverriddenLayoutParametersWebView didChangeValueForKey:]):
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/272679@main




More information about the webkit-changes mailing list