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

Changeset 275485 in webkit


Ignore:
Timestamp:
Apr 5, 2021, 8:19:38 PM (5 years ago)
Author:
Chris Dumez
Message:

Make sure we no longer show the previous page when running a JS prompt
​https://bugs.webkit.org/show_bug.cgi?id=215782
<rdar://problem/67698601>

Reviewed by Darin Adler.

Source/WebCore:

Add linked-on-after check for the behavior change to minimize the risk of
breakage.

  • platform/cocoa/VersionChecks.h:

Source/WebKit:

Make sure we no longer show the previous page when running a JS prompt.
If we have not yet done a layer tree commit since the last load commit, then
we are likely still showing the previous page. If we are asked to run a JS
prompt / alert / confirm at this point, it would be confusing to still show
the previous page. In order to address the issue, we now make the view blank
in such scenario (ideally, we'd have painted the new page but this is
currently not a trivial thing to do).

To make the view blank, the approach chosen is the set the opacity of our
root layer to 0 when the JS prompt is requested (before the first paint) and
set the opacity back to 1 after the first paint.

To minimize the risks of breakage, the behavior change is behind a
linked-on-after check. Also, we only trigger this behavior if the WKWebView
client actually implements the corresponding JS prompt delegate.

  • UIProcess/Cocoa/UIDelegate.mm:

(WebKit::UIDelegate::UIClient::runJavaScriptAlert):
(WebKit::UIDelegate::UIClient::runJavaScriptConfirm):
(WebKit::UIDelegate::UIClient::runJavaScriptPrompt):

  • UIProcess/PageClient.h:

(WebKit::PageClient::makeViewBlank):

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::stopMakingViewBlankDueToLackOfRenderingUpdate):
(WebKit::WebPageProxy::makeViewBlankIfUnpaintedSinceLastLoadCommit):
(WebKit::WebPageProxy::didCommitLoadForFrame):
(WebKit::WebPageProxy::runJavaScriptAlert):

  • UIProcess/WebPageProxy.h:
  • UIProcess/WebPageProxy.messages.in:
  • UIProcess/ios/PageClientImplIOS.h:
  • UIProcess/ios/PageClientImplIOS.mm:

(WebKit::PageClientImpl::makeViewBlank):

  • UIProcess/ios/WebPageProxyIOS.mm:

(WebKit::WebPageProxy::didCommitLayerTree):

  • UIProcess/mac/PageClientImplMac.h:
  • UIProcess/mac/PageClientImplMac.mm:

(WebKit::PageClientImpl::makeViewBlank):

  • UIProcess/mac/WebPageProxyMac.mm:

(WebKit::WebPageProxy::didUpdateRenderingAfterCommittingLoad):

  • WebProcess/WebCoreSupport/WebChromeClient.cpp:

(WebKit::WebChromeClient::runJavaScriptAlert):
(WebKit::WebChromeClient::runJavaScriptConfirm):
(WebKit::WebChromeClient::runJavaScriptPrompt):
Add flag to make sure pending async messages are processed *before* the JS prompt sync IPCs.
Without this, it was possible for the UIProcess to receive the JS prompt IPC *before* the
DidCommitLoadForFrame IPC, which would confuse our logic, since we would think this is a JS
prompt for the previous page.

  • WebProcess/WebPage/WebPage.cpp:

(WebKit::WebPage::didCommitLoad):

  • WebProcess/WebPage/WebPage.h:

(WebKit::WebPage::sendSyncWithDelayedReply):

  • WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:

(WebKit::TiledCoreAnimationDrawingArea::updateRendering):

  • WebProcess/WebPage/mac/WebPageMac.mm:

(WebKit::WebPage::didUpdateRendering):

Location:
trunk/Source
Files:
19 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r275484 r275485  
     12021-04-05  Chris Dumez  <cdumez@apple.com>
     2
     3        Make sure we no longer show the previous page when running a JS prompt
     4        https://bugs.webkit.org/show_bug.cgi?id=215782
     5        <rdar://problem/67698601>
     6
     7        Reviewed by Darin Adler.
     8
     9        Add linked-on-after check for the behavior change to minimize the risk of
     10        breakage.
     11
     12        * platform/cocoa/VersionChecks.h:
     13
    1142021-04-05  Alex Christensen  <achristensen@webkit.org>
    215

  • trunk/Source/WebCore/platform/cocoa/VersionChecks.h

    r275442 r275485  
    7272    FirstWithDOMWindowReuseRestriction  = DYLD_IOS_VERSION_14_5,
    7373    FirstWithSharedNetworkProcess = DYLD_IOS_VERSION_14_5,
     74    FirstWithBlankViewOnJSPrompt = DYLD_IOS_VERSION_14_5,
    7475#elif PLATFORM(MAC)
    7576    FirstWithNetworkCache = DYLD_MACOSX_VERSION_10_11,
    … …  
    9394    FirstWithNullOriginForNonSpecialSchemedURLs = DYLD_MACOSX_VERSION_12_00,
    9495    FirstWithDOMWindowReuseRestriction = DYLD_MACOSX_VERSION_11_3,
     96    FirstWithBlankViewOnJSPrompt = DYLD_MACOSX_VERSION_11_3,
    9597#endif
    9698};

  • trunk/Source/WebKit/ChangeLog

    r275484 r275485  
     12021-04-05  Chris Dumez  <cdumez@apple.com>
     2
     3        Make sure we no longer show the previous page when running a JS prompt
     4        https://bugs.webkit.org/show_bug.cgi?id=215782
     5        <rdar://problem/67698601>
     6
     7        Reviewed by Darin Adler.
     8
     9        Make sure we no longer show the previous page when running a JS prompt.
     10        If we have not yet done a layer tree commit since the last load commit, then
     11        we are likely still showing the previous page. If we are asked to run a JS
     12        prompt / alert / confirm at this point, it would be confusing to still show
     13        the previous page. In order to address the issue, we now make the view blank
     14        in such scenario (ideally, we'd have painted the new page but this is
     15        currently not a trivial thing to do).
     16
     17        To make the view blank, the approach chosen is the set the opacity of our
     18        root layer to 0 when the JS prompt is requested (before the first paint) and
     19        set the opacity back to 1 after the first paint.
     20
     21        To minimize the risks of breakage, the behavior change is behind a
     22        linked-on-after check. Also, we only trigger this behavior if the WKWebView
     23        client actually implements the corresponding JS prompt delegate.
     24
     25        * UIProcess/Cocoa/UIDelegate.mm:
     26        (WebKit::UIDelegate::UIClient::runJavaScriptAlert):
     27        (WebKit::UIDelegate::UIClient::runJavaScriptConfirm):
     28        (WebKit::UIDelegate::UIClient::runJavaScriptPrompt):
     29        * UIProcess/PageClient.h:
     30        (WebKit::PageClient::makeViewBlank):
     31        * UIProcess/WebPageProxy.cpp:
     32        (WebKit::WebPageProxy::stopMakingViewBlankDueToLackOfRenderingUpdate):
     33        (WebKit::WebPageProxy::makeViewBlankIfUnpaintedSinceLastLoadCommit):
     34        (WebKit::WebPageProxy::didCommitLoadForFrame):
     35        (WebKit::WebPageProxy::runJavaScriptAlert):
     36        * UIProcess/WebPageProxy.h:
     37        * UIProcess/WebPageProxy.messages.in:
     38        * UIProcess/ios/PageClientImplIOS.h:
     39        * UIProcess/ios/PageClientImplIOS.mm:
     40        (WebKit::PageClientImpl::makeViewBlank):
     41        * UIProcess/ios/WebPageProxyIOS.mm:
     42        (WebKit::WebPageProxy::didCommitLayerTree):
     43        * UIProcess/mac/PageClientImplMac.h:
     44        * UIProcess/mac/PageClientImplMac.mm:
     45        (WebKit::PageClientImpl::makeViewBlank):
     46        * UIProcess/mac/WebPageProxyMac.mm:
     47        (WebKit::WebPageProxy::didUpdateRenderingAfterCommittingLoad):
     48        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
     49        (WebKit::WebChromeClient::runJavaScriptAlert):
     50        (WebKit::WebChromeClient::runJavaScriptConfirm):
     51        (WebKit::WebChromeClient::runJavaScriptPrompt):
     52        Add flag to make sure pending async messages are processed *before* the JS prompt sync IPCs.
     53        Without this, it was possible for the UIProcess to receive the JS prompt IPC *before* the
     54        DidCommitLoadForFrame IPC, which would confuse our logic, since we would think this is a JS
     55        prompt for the previous page.
     56
     57        * WebProcess/WebPage/WebPage.cpp:
     58        (WebKit::WebPage::didCommitLoad):
     59        * WebProcess/WebPage/WebPage.h:
     60        (WebKit::WebPage::sendSyncWithDelayedReply):
     61        * WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm:
     62        (WebKit::TiledCoreAnimationDrawingArea::updateRendering):
     63        * WebProcess/WebPage/mac/WebPageMac.mm:
     64        (WebKit::WebPage::didUpdateRendering):
     65
    1662021-04-05  Alex Christensen  <achristensen@webkit.org>
    267

  • trunk/Source/WebKit/UIProcess/Cocoa/UIDelegate.mm

    r275162 r275485  
    306306    }
    307307
     308    page.makeViewBlankIfUnpaintedSinceLastLoadCommit();
     309
    308310    auto checker = CompletionHandlerCallChecker::create(delegate.get(), @selector(webView:runJavaScriptAlertPanelWithMessage:initiatedByFrame:completionHandler:));
    309311    [delegate webView:m_uiDelegate->m_webView.get().get() runJavaScriptAlertPanelWithMessage:message initiatedByFrame:wrapper(API::FrameInfo::create(WTFMove(frameInfo), &page)) completionHandler:makeBlockPtr([completionHandler = WTFMove(completionHandler), checker = WTFMove(checker)] {
    … …  
    331333    }
    332334
     335    page.makeViewBlankIfUnpaintedSinceLastLoadCommit();
     336
    333337    auto checker = CompletionHandlerCallChecker::create(delegate.get(), @selector(webView:runJavaScriptConfirmPanelWithMessage:initiatedByFrame:completionHandler:));
    334338    [delegate webView:m_uiDelegate->m_webView.get().get() runJavaScriptConfirmPanelWithMessage:message initiatedByFrame:wrapper(API::FrameInfo::create(WTFMove(frameInfo), &page)) completionHandler:makeBlockPtr([completionHandler = WTFMove(completionHandler), checker = WTFMove(checker)] (BOOL result) mutable {
    … …  
    355359        return;
    356360    }
     361
     362    page.makeViewBlankIfUnpaintedSinceLastLoadCommit();
    357363
    358364    auto checker = CompletionHandlerCallChecker::create(delegate.get(), @selector(webView:runJavaScriptTextInputPanelWithPrompt:defaultText:initiatedByFrame:completionHandler:));

  • trunk/Source/WebKit/UIProcess/PageClient.h

    r275442 r275485  
    520520    virtual void setMouseEventPolicy(WebCore::MouseEventPolicy) { }
    521521
     522    virtual void makeViewBlank(bool) { }
     523
    522524#if HAVE(PASTEBOARD_DATA_OWNER)
    523525    virtual WebCore::DataOwnerType dataOwnerForPasteboard(PasteboardAccessIntent) const { return WebCore::DataOwnerType::Undefined; }

  • trunk/Source/WebKit/UIProcess/WebPageProxy.cpp

    r275447 r275485  
    24442444#endif
    24452445
     2446void WebPageProxy::stopMakingViewBlankDueToLackOfRenderingUpdate()
     2447{
     2448#if PLATFORM(COCOA)
     2449    ASSERT(m_hasUpdatedRenderingAfterDidCommitLoad);
     2450    RELEASE_LOG_IF_ALLOWED(Process, "stopMakingViewBlankDueToLackOfRenderingUpdate:");
     2451    pageClient().makeViewBlank(false);
     2452#endif
     2453}
     2454
     2455// If we have not painted yet since the last load commit, then we are likely still displaying the previous page.
     2456// Displaying a JS prompt for the new page with the old page behind would be confusing so we make the view blank
     2457// until the next paint in such case.
     2458void WebPageProxy::makeViewBlankIfUnpaintedSinceLastLoadCommit()
     2459{
     2460#if PLATFORM(COCOA)
     2461    if (!m_hasUpdatedRenderingAfterDidCommitLoad) {
     2462        static bool shouldMakeViewBlank = linkedOnOrAfter(WebCore::SDKVersion::FirstWithBlankViewOnJSPrompt);
     2463        if (shouldMakeViewBlank) {
     2464            RELEASE_LOG_IF_ALLOWED(Process, "makeViewBlankIfUnpaintedSinceLastLoadCommit: Making the view blank because of a JS prompt before the first paint for its page");
     2465            pageClient().makeViewBlank(true);
     2466        }
     2467    }
     2468#endif
     2469}
     2470
    24462471void WebPageProxy::discardQueuedMouseEvents()
    24472472{
    … …  
    46344659    m_process->didCommitProvisionalLoad();
    46354660
     4661#if PLATFORM(COCOA)
     4662    if (frame->isMainFrame()) {
     4663        m_hasUpdatedRenderingAfterDidCommitLoad = false;
    46364664#if PLATFORM(IOS_FAMILY)
    4637     if (frame->isMainFrame()) {
    4638         m_hasReceivedLayerTreeTransactionAfterDidCommitLoad = false;
    46394665        m_firstLayerTreeTransactionIdAfterDidCommitLoad = downcast<RemoteLayerTreeDrawingAreaProxy>(*drawingArea()).nextLayerTreeTransactionID();
     4666#endif
    46404667    }
    46414668#endif

  • trunk/Source/WebKit/UIProcess/WebPageProxy.h

    r275442 r275485  
    589589
    590590    WeakPtr<SecKeyProxyStore> secKeyProxyStore(const WebCore::AuthenticationChallenge&);
     591
     592    void makeViewBlankIfUnpaintedSinceLastLoadCommit();
    591593       
    592594    void close();
    … …  
    21962198#endif
    21972199
     2200    void stopMakingViewBlankDueToLackOfRenderingUpdate();
     2201
    21982202    // Spelling and grammar.
    21992203    void checkSpellingOfString(const String& text, CompletionHandler<void(int32_t misspellingLocation, int32_t misspellingLength)>&&);
    … …  
    22152219    void didReceiveEvent(uint32_t opaqueType, bool handled);
    22162220#if PLATFORM(MAC)
     2221    void didUpdateRenderingAfterCommittingLoad();
    22172222    void fontAtSelectionCallback(const FontInfo&, double, bool, CallbackID);
    22182223#endif
    … …  
    25212526    TransactionID m_firstLayerTreeTransactionIdAfterDidCommitLoad;
    25222527    int32_t m_deviceOrientation { 0 };
    2523     bool m_hasReceivedLayerTreeTransactionAfterDidCommitLoad { true };
    25242528    bool m_hasNetworkRequestsOnSuspended { false };
    25252529    bool m_isKeyboardAnimatingIn { false };
    … …  
    26472651    bool m_isInPrintingMode { false };
    26482652    bool m_isPerformingDOMPrintOperation { false };
     2653
     2654#if PLATFORM(COCOA)
     2655    bool m_hasUpdatedRenderingAfterDidCommitLoad { true };
     2656#endif
    26492657
    26502658    WebCore::ResourceRequest m_decidePolicyForResponseRequest;

  • trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in

    r275442 r275485  
    372372
    373373    SetEditableElementIsFocused(bool editableElementIsFocused)
     374    DidUpdateRenderingAfterCommittingLoad()
    374375#endif
    375376

  • trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.h

    r274521 r275485  
    150150    LayerHostingMode viewLayerHostingMode() override { return LayerHostingMode::OutOfProcess; }
    151151
     152    void makeViewBlank(bool) final;
     153
    152154    RefPtr<ViewSnapshot> takeViewSnapshot(Optional<WebCore::IntRect>&&) override;
    153155    void wheelEventWasNotHandledByWebCore(const NativeWebWheelEvent&) override;

  • trunk/Source/WebKit/UIProcess/ios/PageClientImplIOS.mm

    r275407 r275485  
    489489}
    490490
     491void PageClientImpl::makeViewBlank(bool makeBlank)
     492{
     493    [m_contentView layer].opacity = makeBlank ? 0 : 1;
     494}
     495
    491496void PageClientImpl::showSafeBrowsingWarning(const SafeBrowsingWarning& warning, CompletionHandler<void(Variant<WebKit::ContinueUnsafeLoad, URL>&&)>&& completionHandler)
    492497{

  • trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm

    r275442 r275485  
    341341    pageExtendedBackgroundColorDidChange(layerTreeTransaction.pageExtendedBackgroundColor());
    342342
    343     if (!m_hasReceivedLayerTreeTransactionAfterDidCommitLoad) {
     343    if (!m_hasUpdatedRenderingAfterDidCommitLoad) {
    344344        if (layerTreeTransaction.transactionID() >= m_firstLayerTreeTransactionIdAfterDidCommitLoad) {
    345             m_hasReceivedLayerTreeTransactionAfterDidCommitLoad = true;
     345            m_hasUpdatedRenderingAfterDidCommitLoad = true;
     346            stopMakingViewBlankDueToLackOfRenderingUpdate();
    346347            m_lastVisibleContentRectUpdate = VisibleContentRectUpdateInfo();
    347348        }

  • trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.h

    r274521 r275485  
    228228    void requestDOMPasteAccess(const WebCore::IntRect&, const String&, CompletionHandler<void(WebCore::DOMPasteAccessResponse)>&&) final;
    229229
     230    void makeViewBlank(bool) final;
     231
    230232    NSView *activeView() const;
    231233    NSWindow *activeWindow() const;

  • trunk/Source/WebKit/UIProcess/mac/PageClientImplMac.mm

    r274521 r275485  
    998998}
    999999
     1000
     1001void PageClientImpl::makeViewBlank(bool makeBlank)
     1002{
     1003    m_impl->acceleratedCompositingRootLayer().opacity = makeBlank ? 0 : 1;
     1004}
     1005
    10001006#if HAVE(APP_ACCENT_COLORS)
    10011007WebCore::Color PageClientImpl::accentColor()

  • trunk/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm

    r275442 r275485  
    661661}
    662662
     663void WebPageProxy::didUpdateRenderingAfterCommittingLoad()
     664{
     665    if (m_hasUpdatedRenderingAfterDidCommitLoad)
     666        return;
     667
     668    m_hasUpdatedRenderingAfterDidCommitLoad = true;
     669    stopMakingViewBlankDueToLackOfRenderingUpdate();
     670}
     671
    663672#if ENABLE(UI_PROCESS_PDF_HUD)
    664673

  • trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp

    r275480 r275485  
    461461    IPC::UnboundedSynchronousIPCScope unboundedSynchronousIPCScope;
    462462
    463     m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptAlert(webFrame->frameID(), webFrame->info(), alertText), Messages::WebPageProxy::RunJavaScriptAlert::Reply());
     463    m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptAlert(webFrame->frameID(), webFrame->info(), alertText), Messages::WebPageProxy::RunJavaScriptAlert::Reply(), IPC::SendSyncOption::MaintainOrderingWithAsyncMessages);
    464464}
    465465
    … …  
    479479
    480480    bool result = false;
    481     if (!m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptConfirm(webFrame->frameID(), webFrame->info(), message), Messages::WebPageProxy::RunJavaScriptConfirm::Reply(result)))
     481    if (!m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptConfirm(webFrame->frameID(), webFrame->info(), message), Messages::WebPageProxy::RunJavaScriptConfirm::Reply(result), IPC::SendSyncOption::MaintainOrderingWithAsyncMessages))
    482482        return false;
    483483
    … …  
    499499    IPC::UnboundedSynchronousIPCScope unboundedSynchronousIPCScope;
    500500
    501     if (!m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptPrompt(webFrame->frameID(), webFrame->info(), message, defaultValue), Messages::WebPageProxy::RunJavaScriptPrompt::Reply(result)))
     501    if (!m_page.sendSyncWithDelayedReply(Messages::WebPageProxy::RunJavaScriptPrompt(webFrame->frameID(), webFrame->info(), message, defaultValue), Messages::WebPageProxy::RunJavaScriptPrompt::Reply(result), IPC::SendSyncOption::MaintainOrderingWithAsyncMessages))
    502502        return false;
    503503

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp

    r275480 r275485  
    61956195    }
    61966196
     6197#if PLATFORM(MAC)
     6198    m_didUpdateRenderingAfterCommittingLoad = false;
     6199#endif
    61976200#if PLATFORM(IOS_FAMILY)
    61986201    m_hasReceivedVisibleContentRectsAfterDidCommitLoad = false;

  • trunk/Source/WebKit/WebProcess/WebPage/WebPage.h

    r275480 r275485  
    631631    void setTopOverhangImage(WebImage*);
    632632    void setBottomOverhangImage(WebImage*);
     633
     634    void didUpdateRendering();
    633635   
    634636    void setUseSystemAppearance(bool);
    … …  
    12811283
    12821284    template<typename T>
    1283     SendSyncResult sendSyncWithDelayedReply(T&& message, typename T::Reply&& reply)
     1285    SendSyncResult sendSyncWithDelayedReply(T&& message, typename T::Reply&& reply, OptionSet<IPC::SendSyncOption> sendSyncOptions = { })
    12841286    {
    12851287        cancelGesturesBlockedOnSynchronousReplies();
    1286         return sendSync(WTFMove(message), WTFMove(reply), Seconds::infinity(), IPC::SendSyncOption::InformPlatformProcessWillSuspend);
     1288        sendSyncOptions = sendSyncOptions | IPC::SendSyncOption::InformPlatformProcessWillSuspend;
     1289        return sendSync(WTFMove(message), WTFMove(reply), Seconds::infinity(), sendSyncOptions);
    12871290    }
    12881291
    … …  
    22562259    bool m_canUseCredentialStorage { true };
    22572260
     2261#if PLATFORM(MAC)
     2262    bool m_didUpdateRenderingAfterCommittingLoad { false };
     2263#endif
     2264
    22582265    Vector<String> m_corsDisablingPatterns;
    22592266

  • trunk/Source/WebKit/WebProcess/WebPage/mac/TiledCoreAnimationDrawingArea.mm

    r275442 r275485  
    487487
    488488        sendDidFirstLayerFlushIfNeeded();
     489        m_webPage.didUpdateRendering();
    489490        handleActivityStateChangeCallbacksIfNeeded();
    490491        invalidateRenderingUpdateRunLoopObserver();

  • trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm

    r275442 r275485  
    10331033}
    10341034
     1035void WebPage::didUpdateRendering()
     1036{
     1037    if (m_didUpdateRenderingAfterCommittingLoad)
     1038        return;
     1039
     1040    m_didUpdateRenderingAfterCommittingLoad = true;
     1041    send(Messages::WebPageProxy::DidUpdateRenderingAfterCommittingLoad());
     1042}
     1043
    10351044#if ENABLE(WIRELESS_PLAYBACK_TARGET) && !PLATFORM(IOS_FAMILY)
    10361045void WebPage::playbackTargetSelected(PlaybackTargetClientContextIdentifier contextId, const WebCore::MediaPlaybackTargetContext& targetContext) const
Note: See TracChangeset for help on using the changeset viewer.