[webkit-changes] [WebKit/WebKit] d19167: REGRESSION (260451 at main): Opening any PDF in WebKi...

Tyler Wilcock noreply at github.com
Fri Mar 24 22:23:04 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: d191671333e174c82b39ae2a71fc878e5026be12
      https://github.com/WebKit/WebKit/commit/d191671333e174c82b39ae2a71fc878e5026be12
  Author: Tyler Wilcock <tyler_w at apple.com>
  Date:   2023-03-24 (Fri, 24 Mar 2023)

  Changed paths:
    A LayoutTests/platform/mac-wk2/plugins/pdf-plugin-initial-scroll-position-expected.txt
    A LayoutTests/platform/mac-wk2/plugins/pdf-plugin-initial-scroll-position.html
    M Source/WebCore/plugins/PluginViewBase.h
    M Source/WebCore/testing/Internals.cpp
    M Source/WebCore/testing/Internals.h
    M Source/WebCore/testing/Internals.idl
    M Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h
    M Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm
    M Source/WebKit/WebProcess/Plugins/PluginView.cpp
    M Source/WebKit/WebProcess/Plugins/PluginView.h

  Log Message:
  -----------
  REGRESSION (260451 at main): Opening any PDF in WebKit opens it halfway down the first page
https://bugs.webkit.org/show_bug.cgi?id=254374
rdar://106880773

Reviewed by Tim Horton.

Before 260451 at main, the sequence for updating PDF scroll position on and shortly after load was this:

1. PDF plugin is initialized, scroll position is set to (0, 0)
2. PDFKit does `-[PDFLayerController _updateAutoScale]` and `-[PDFLayerController magnifyWithMagnification:atPoint:immediately:]`,
   and informs the `WKPDFLayerControllerDelegate` to update the scroll position to some greater than zero value (despite no scroll actually happening)
3. A call to `PDFPlugin::receivedNonLinearizedPDFSentinel()` is handled. Scroll position is set to (0, 0)

After 260451 at main, the sequence became:

1. PDF plugin is initialized, scroll position is set to (0, 0)
2. PDFKit does magnification as described above, updates scroll position to (0, 190 (or other >0 value)). This is dispatched to the main runloop to be handled asynchronously.
3. A call to `PDFPlugin::receivedNonLinearizedPDFSentinel()` is handled. Scroll position is set to (0, 0)
4. The async dispatch from step 2 is handled by the main runloop, overwriting the (0,0) value

This patch addresses the bug by using `callOnMainRunLoopAndWait` instead of `callOnMainRunLoop` inside `-[PDFPlugin updateScrollPosition:]`
to prevent incorrect re-ordering of scroll position changes.

* LayoutTests/platform/mac-wk2/plugins/pdf-plugin-initial-scroll-position-expected.txt: Added.
* LayoutTests/platform/mac-wk2/plugins/pdf-plugin-initial-scroll-position.html: Added.
* Source/WebCore/plugins/PluginViewBase.h:
(WebCore::PluginViewBase::scrollPositionForTesting const):
* Source/WebCore/testing/Internals.cpp:
(WebCore::scrollPositionForPlugin):
(WebCore::Internals::pluginScrollPositionX):
(WebCore::Internals::pluginScrollPositionY):
* Source/WebCore/testing/Internals.h:
* Source/WebCore/testing/Internals.idl:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.h:
* Source/WebKit/WebProcess/Plugins/PDF/PDFPlugin.mm:
(-[WKPDFLayerControllerDelegate updateScrollPosition:]):
* Source/WebKit/WebProcess/Plugins/PluginView.cpp:
(WebKit::PluginView::scrollPositionForTesting const):
* Source/WebKit/WebProcess/Plugins/PluginView.h:

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




More information about the webkit-changes mailing list