⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 249697 in webkit


Ignore:
Timestamp:
Sep 9, 2019, 8:19:56 PM (7 years ago)
Author:
Alan Coon
Message:

Cherry-pick r249584. rdar://problem/55202935

Marking up a note on iOS results in a PDF with no contents
​https://bugs.webkit.org/show_bug.cgi?id=201530
<rdar://problem/53686019>

Reviewed by Andy Estes.

Source/WebKit:

  • Platform/IPC/Connection.cpp: (IPC::Connection::Connection): (IPC::Connection::waitForMessage): (IPC::Connection::connectionDidClose):
  • Platform/IPC/Connection.h: If the main thread is blocked when the Web Content process dies, and something eventually calls waitForAndDispatchImmediately without returning control to the main run loop, we will wait for the full timeout, because a) the code to mark the connection invalid is dispatched to the main thread, and b) the secondary thread that is informed of the Web Content process dying did not yet have a "waiting for" message to mark as interrupted (because it wasn't waiting yet).

Fix this race by adding a bit that is set under the waitForMessage lock
on the secondary thread when the connection is invalidated, identically
to m_shouldWaitForSyncReplies, which solves the same problem for sync
messages.

Read the new bit when we are about to start waiting, and bail if it is set.
It's OK to not read it inside the loop because we are guaranteed to have
waitForMessage set at that point, so the normal interruption bit will work.

  • UIProcess/ios/WKContentView.mm: (-[WKContentView _processDidExit]): Reset _isPrintingToPDF; the Web Content process is never going to get back to us if it crashes.

(-[WKContentView _wk_pageCountForPrintFormatter:]):
Do not bail from starting a printing operation if one is already occurring.
This fixes the original bug, because Markup ends up invalidating the page
count at least one extra time before asking for the printed document.
Instead of maintaining the fragile requirement that you cannot recompute
the page count while printing, just let it happen. In order to make this
work safely, synchronously wait for the previous printed result before
continuing with the next print.

We could do more coalescing here if need be, but calls to -_recalcPageCount
are not high in volume.

Tools:

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/WebKitCocoa/WKWebViewPrintFormatter.mm: Add some tests for WKWebViewPrintFormatter; specifically that it is possible to _recalcPageCount twice in quick succession, and that we don't hang if we start painting the printed content immediately after a Web Content process crash.

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@249584 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:
branches/safari-608-branch
Files:
1 added
6 edited

Legend:

Unmodified
Added
Removed

  • branches/safari-608-branch/Source/WebKit/ChangeLog

    r249693 r249697  
     12019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r249584. rdar://problem/55202935
     4
     5    Marking up a note on iOS results in a PDF with no contents
     6    https://bugs.webkit.org/show_bug.cgi?id=201530
     7    <rdar://problem/53686019>
     8   
     9    Reviewed by Andy Estes.
     10   
     11    Source/WebKit:
     12   
     13    * Platform/IPC/Connection.cpp:
     14    (IPC::Connection::Connection):
     15    (IPC::Connection::waitForMessage):
     16    (IPC::Connection::connectionDidClose):
     17    * Platform/IPC/Connection.h:
     18    If the main thread is blocked when the Web Content process dies, and
     19    something eventually calls waitForAndDispatchImmediately without
     20    returning control to the main run loop, we will wait for the full timeout,
     21    because a) the code to mark the connection invalid is dispatched
     22    to the main thread, and b) the secondary thread that is informed of
     23    the Web Content process dying did not yet have a "waiting for" message
     24    to mark as interrupted (because it wasn't waiting yet).
     25   
     26    Fix this race by adding a bit that is set under the waitForMessage lock
     27    on the secondary thread when the connection is invalidated, identically
     28    to m_shouldWaitForSyncReplies, which solves the same problem for sync
     29    messages.
     30   
     31    Read the new bit when we are about to start waiting, and bail if it is set.
     32    It's OK to not read it inside the loop because we are guaranteed to have
     33    waitForMessage set at that point, so the normal interruption bit will work.
     34   
     35    * UIProcess/ios/WKContentView.mm:
     36    (-[WKContentView _processDidExit]):
     37    Reset _isPrintingToPDF; the Web Content process is never going to get
     38    back to us if it crashes.
     39   
     40    (-[WKContentView _wk_pageCountForPrintFormatter:]):
     41    Do not bail from starting a printing operation if one is already occurring.
     42    This fixes the original bug, because Markup ends up invalidating the page
     43    count at least one extra time before asking for the printed document.
     44    Instead of maintaining the fragile requirement that you cannot recompute
     45    the page count while printing, just let it happen. In order to make this
     46    work safely, synchronously wait for the previous printed result before
     47    continuing with the next print.
     48   
     49    We could do more coalescing here if need be, but calls to -_recalcPageCount
     50    are not high in volume.
     51   
     52    Tools:
     53   
     54    * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     55    * TestWebKitAPI/Tests/WebKitCocoa/WKWebViewPrintFormatter.mm:
     56    Add some tests for WKWebViewPrintFormatter; specifically that it is
     57    possible to _recalcPageCount twice in quick succession, and that
     58    we don't hang if we start painting the printed content immediately
     59    after a Web Content process crash.
     60   
     61    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249584 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     62
     63    2019-09-06  Tim Horton  <timothy_horton@apple.com>
     64
     65            Marking up a note on iOS results in a PDF with no contents
     66            https://bugs.webkit.org/show_bug.cgi?id=201530
     67            <rdar://problem/53686019>
     68
     69            Reviewed by Andy Estes.
     70
     71            * Platform/IPC/Connection.cpp:
     72            (IPC::Connection::Connection):
     73            (IPC::Connection::waitForMessage):
     74            (IPC::Connection::connectionDidClose):
     75            * Platform/IPC/Connection.h:
     76            If the main thread is blocked when the Web Content process dies, and
     77            something eventually calls waitForAndDispatchImmediately without
     78            returning control to the main run loop, we will wait for the full timeout,
     79            because a) the code to mark the connection invalid is dispatched
     80            to the main thread, and b) the secondary thread that is informed of
     81            the Web Content process dying did not yet have a "waiting for" message
     82            to mark as interrupted (because it wasn't waiting yet).
     83
     84            Fix this race by adding a bit that is set under the waitForMessage lock
     85            on the secondary thread when the connection is invalidated, identically
     86            to m_shouldWaitForSyncReplies, which solves the same problem for sync
     87            messages.
     88
     89            Read the new bit when we are about to start waiting, and bail if it is set.
     90            It's OK to not read it inside the loop because we are guaranteed to have
     91            waitForMessage set at that point, so the normal interruption bit will work.
     92
     93            * UIProcess/ios/WKContentView.mm:
     94            (-[WKContentView _processDidExit]):
     95            Reset _isPrintingToPDF; the Web Content process is never going to get
     96            back to us if it crashes.
     97
     98            (-[WKContentView _wk_pageCountForPrintFormatter:]):
     99            Do not bail from starting a printing operation if one is already occurring.
     100            This fixes the original bug, because Markup ends up invalidating the page
     101            count at least one extra time before asking for the printed document.
     102            Instead of maintaining the fragile requirement that you cannot recompute
     103            the page count while printing, just let it happen. In order to make this
     104            work safely, synchronously wait for the previous printed result before
     105            continuing with the next print.
     106
     107            We could do more coalescing here if need be, but calls to -_recalcPageCount
     108            are not high in volume.
     109
    11102019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
    2111

  • branches/safari-608-branch/Source/WebKit/Platform/IPC/Connection.cpp

    r247992 r249697  
    273273    , m_didReceiveInvalidMessage(false)
    274274    , m_shouldWaitForSyncReplies(true)
     275    , m_shouldWaitForMessages(true)
    275276{
    276277    ASSERT(RunLoop::isMain());
    … …  
    522523        ASSERT(!m_waitingForMessage);
    523524        if (m_waitingForMessage)
     525            return nullptr;
     526
     527        // If the connection is already invalidated, don't even start waiting.
     528        // Once m_waitingForMessage is set, messageWaitingInterrupted will cover this instead.
     529        if (!m_shouldWaitForMessages)
    524530            return nullptr;
    525531
    … …  
    846852    {
    847853        std::lock_guard<Lock> lock(m_waitForMessageMutex);
     854
     855        ASSERT(m_shouldWaitForMessages);
     856        m_shouldWaitForMessages = false;
     857
    848858        if (m_waitingForMessage)
    849859            m_waitingForMessage->messageWaitingInterrupted = true;

  • branches/safari-608-branch/Source/WebKit/Platform/IPC/Connection.h

    r247992 r249697  
    366366    Lock m_syncReplyStateMutex;
    367367    bool m_shouldWaitForSyncReplies;
     368    bool m_shouldWaitForMessages;
    368369    struct PendingSyncReply;
    369370    Vector<PendingSyncReply> m_pendingSyncReplies;

  • branches/safari-608-branch/Source/WebKit/UIProcess/ios/WKContentView.mm

    r247954 r249697  
    558558    [self _removeVisibilityPropagationView];
    559559#endif
     560
     561    _isPrintingToPDF = NO;
    560562}
    561563
    … …  
    703705{
    704706    if (_isPrintingToPDF)
    705         return 0;
     707        [self _waitForDrawToPDFCallback];
    706708
    707709    uint64_t frameID;
    … …  
    746748}
    747749
     750- (BOOL)_waitForDrawToPDFCallback
     751{
     752    if (!_page->process().connection()->waitForAndDispatchImmediately<Messages::WebPageProxy::DrawToPDFCallback>(_page->webPageID(), Seconds::infinity())) {
     753        ASSERT_NOT_REACHED();
     754        return false;
     755    }
     756    ASSERT(!_isPrintingToPDF);
     757    return true;
     758}
     759
    748760- (CGPDFDocumentRef)_wk_printedDocument
    749761{
    750762    if (_isPrintingToPDF) {
    751         if (!_page->process().connection()->waitForAndDispatchImmediately<Messages::WebPageProxy::DrawToPDFCallback>(_page->pageID(), Seconds::infinity())) {
    752             ASSERT_NOT_REACHED();
     763        if (![self _waitForDrawToPDFCallback])
    753764            return nullptr;
    754         }
    755         ASSERT(!_isPrintingToPDF);
    756765    }
    757766

  • branches/safari-608-branch/Tools/ChangeLog

    r249693 r249697  
     12019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
     2
     3        Cherry-pick r249584. rdar://problem/55202935
     4
     5    Marking up a note on iOS results in a PDF with no contents
     6    https://bugs.webkit.org/show_bug.cgi?id=201530
     7    <rdar://problem/53686019>
     8   
     9    Reviewed by Andy Estes.
     10   
     11    Source/WebKit:
     12   
     13    * Platform/IPC/Connection.cpp:
     14    (IPC::Connection::Connection):
     15    (IPC::Connection::waitForMessage):
     16    (IPC::Connection::connectionDidClose):
     17    * Platform/IPC/Connection.h:
     18    If the main thread is blocked when the Web Content process dies, and
     19    something eventually calls waitForAndDispatchImmediately without
     20    returning control to the main run loop, we will wait for the full timeout,
     21    because a) the code to mark the connection invalid is dispatched
     22    to the main thread, and b) the secondary thread that is informed of
     23    the Web Content process dying did not yet have a "waiting for" message
     24    to mark as interrupted (because it wasn't waiting yet).
     25   
     26    Fix this race by adding a bit that is set under the waitForMessage lock
     27    on the secondary thread when the connection is invalidated, identically
     28    to m_shouldWaitForSyncReplies, which solves the same problem for sync
     29    messages.
     30   
     31    Read the new bit when we are about to start waiting, and bail if it is set.
     32    It's OK to not read it inside the loop because we are guaranteed to have
     33    waitForMessage set at that point, so the normal interruption bit will work.
     34   
     35    * UIProcess/ios/WKContentView.mm:
     36    (-[WKContentView _processDidExit]):
     37    Reset _isPrintingToPDF; the Web Content process is never going to get
     38    back to us if it crashes.
     39   
     40    (-[WKContentView _wk_pageCountForPrintFormatter:]):
     41    Do not bail from starting a printing operation if one is already occurring.
     42    This fixes the original bug, because Markup ends up invalidating the page
     43    count at least one extra time before asking for the printed document.
     44    Instead of maintaining the fragile requirement that you cannot recompute
     45    the page count while printing, just let it happen. In order to make this
     46    work safely, synchronously wait for the previous printed result before
     47    continuing with the next print.
     48   
     49    We could do more coalescing here if need be, but calls to -_recalcPageCount
     50    are not high in volume.
     51   
     52    Tools:
     53   
     54    * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     55    * TestWebKitAPI/Tests/WebKitCocoa/WKWebViewPrintFormatter.mm:
     56    Add some tests for WKWebViewPrintFormatter; specifically that it is
     57    possible to _recalcPageCount twice in quick succession, and that
     58    we don't hang if we start painting the printed content immediately
     59    after a Web Content process crash.
     60   
     61    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@249584 268f45cc-cd09-0410-ab3c-d52691b4dbfc
     62
     63    2019-09-06  Tim Horton  <timothy_horton@apple.com>
     64
     65            Marking up a note on iOS results in a PDF with no contents
     66            https://bugs.webkit.org/show_bug.cgi?id=201530
     67            <rdar://problem/53686019>
     68
     69            Reviewed by Andy Estes.
     70
     71            * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     72            * TestWebKitAPI/Tests/WebKitCocoa/WKWebViewPrintFormatter.mm:
     73            Add some tests for WKWebViewPrintFormatter; specifically that it is
     74            possible to _recalcPageCount twice in quick succession, and that
     75            we don't hang if we start painting the printed content immediately
     76            after a Web Content process crash.
     77
    1782019-09-09  Kocsen Chung  <kocsen_chung@apple.com>
    279

  • branches/safari-608-branch/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r249469 r249697  
    106106                297234B7173AFAC700983601 /* CustomProtocolsInvalidScheme_Bundle.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 297234B5173AFAC700983601 /* CustomProtocolsInvalidScheme_Bundle.cpp */; };
    107107                2D00065F1C1F589A0088E6A7 /* WKPDFViewResizeCrash.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D00065D1C1F58940088E6A7 /* WKPDFViewResizeCrash.mm */; };
     108                2D01D06E23218FEE0039AA3A /* WKWebViewPrintFormatter.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D01D06D23218FEE0039AA3A /* WKWebViewPrintFormatter.mm */; };
    108109                2D08E9372267D0F4002518DA /* ReparentWebViewTimeout.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D08E9362267D0F3002518DA /* ReparentWebViewTimeout.mm */; };
    109110                2D1646E21D1862CD00015A1A /* DeferredViewInWindowStateChange.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2D1646E11D1862CD00015A1A /* DeferredViewInWindowStateChange.mm */; };
    … …  
    15351536                29AB8AA3164C7A9300D49BEC /* TestBrowsingContextLoadDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = TestBrowsingContextLoadDelegate.h; sourceTree = "<group>"; };
    15361537                2D00065D1C1F58940088E6A7 /* WKPDFViewResizeCrash.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKPDFViewResizeCrash.mm; sourceTree = "<group>"; };
     1538                2D01D06D23218FEE0039AA3A /* WKWebViewPrintFormatter.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = WKWebViewPrintFormatter.mm; sourceTree = "<group>"; };
    15371539                2D08E9362267D0F3002518DA /* ReparentWebViewTimeout.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = ReparentWebViewTimeout.mm; sourceTree = "<group>"; };
    15381540                2D1646E11D1862CD00015A1A /* DeferredViewInWindowStateChange.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; name = DeferredViewInWindowStateChange.mm; path = WebKit/DeferredViewInWindowStateChange.mm; sourceTree = "<group>"; };
    … …  
    28892891                                F4106C6821ACBF84004B89A1 /* WKWebViewFirstResponderTests.mm */,
    28902892                                D3BE5E341E4CE85E00FD563A /* WKWebViewGetContents.mm */,
     2893                                2D01D06D23218FEE0039AA3A /* WKWebViewPrintFormatter.mm */,
    28912894                                37A9DBE7213B4C9300D261A2 /* WKWebViewServerTrustKVC.mm */,
    28922895                                93F56DA81E5F9181003EDE84 /* WKWebViewSnapshot.mm */,
    … …  
    47544757                                F4FA91811E61849B007B8C1D /* WKWebViewMacEditingTests.mm in Sources */,
    47554758                                1CACADA1230620AE0007D54C /* WKWebViewOpaque.mm in Sources */,
     4759                                2D01D06E23218FEE0039AA3A /* WKWebViewPrintFormatter.mm in Sources */,
    47564760                                37A9DBE9213B4C9300D261A2 /* WKWebViewServerTrustKVC.mm in Sources */,
    47574761                                93F56DA91E5F919D003EDE84 /* WKWebViewSnapshot.mm in Sources */,
Note: See TracChangeset for help on using the changeset viewer.