Changeset 255518 in webkit


Ignore:
Timestamp:
Jan 31, 2020 12:58:43 PM (4 years ago)
Author:
Chris Dumez
Message:

[ iOS ] imported/w3c/web-platform-tests/IndexedDB/key-generators/reading-autoincrement-indexes-cursors.any.serviceworker.html is flaky failing.
​https://bugs.webkit.org/show_bug.cgi?id=206934
<rdar://problem/58991581>

Source/WebKit:

Reviewed by Brady Eidson.

Flakiness would happen when the service worker would take too long to launch and the responsiveness timer would fire and
report the process as unresponsive while still launching or very shortly after. When a service worker is reported as
unresponsive, we would kill it.

To address the issue, several changes were made:

  • Responsiveness checks are now disabled for slow builds (Debug, ASAN, GuardMalloc)
  • We only start the ResponsivenessTimer after the process has finished launching since the responsiveness check relies on IPC to the process and we cannot send the IPC until after the process has launched.
  • UIProcess/CoordinatedGraphics/DrawingAreaProxyCoordinatedGraphics.cpp:

(WebKit::DrawingAreaProxyCoordinatedGraphics::sendUpdateBackingStoreState):

  • UIProcess/ProvisionalPageProxy.cpp:

(WebKit::ProvisionalPageProxy::goToBackForwardItem):

  • UIProcess/WebPageProxy.cpp:

(WebKit::WebPageProxy::launchProcessForReload):
(WebKit::WebPageProxy::launchProcessWithItem):
(WebKit::WebPageProxy::loadRequestWithNavigationShared):
(WebKit::WebPageProxy::loadFile):
(WebKit::WebPageProxy::loadDataWithNavigationShared):
(WebKit::WebPageProxy::loadAlternateHTML):
(WebKit::WebPageProxy::loadWebArchiveData):
(WebKit::WebPageProxy::navigateToPDFLinkWithSimulatedClick):
(WebKit::WebPageProxy::stopLoading):
(WebKit::WebPageProxy::reload):
(WebKit::WebPageProxy::goToBackForwardItem):
(WebKit::WebPageProxy::dispatchActivityStateChange):
(WebKit::WebPageProxy::processNextQueuedMouseEvent):
(WebKit::WebPageProxy::handleKeyboardEvent):
(WebKit::WebPageProxy::handleGestureEvent):
(WebKit::WebPageProxy::handlePreventableTouchEvent):
(WebKit::WebPageProxy::handleTouchEvent):
(WebKit::WebPageProxy::runJavaScriptAlert):
(WebKit::WebPageProxy::runJavaScriptConfirm):
(WebKit::WebPageProxy::runJavaScriptPrompt):
(WebKit::WebPageProxy::runBeforeUnloadConfirmPanel):
(WebKit::WebPageProxy::runOpenPanel):

  • UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::mayBecomeUnresponsive):
(WebKit::WebProcessProxy::didFinishLaunching):
(WebKit::WebProcessProxy::startResponsivenessTimer):
(WebKit::WebProcessProxy::isResponsive):

  • UIProcess/WebProcessProxy.h:

(WebKit::WebProcessProxy::responsivenessTimer):

LayoutTests:

Unskip test which should no longer be flaky.

  • platform/ios-wk2/TestExpectations:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r255516 r255518  
     12020-01-31  Chris Dumez  <cdumez@apple.com>
     2
     3        [ iOS ] imported/w3c/web-platform-tests/IndexedDB/key-generators/reading-autoincrement-indexes-cursors.any.serviceworker.html is flaky failing.
     4        https://bugs.webkit.org/show_bug.cgi?id=206934
     5        <rdar://problem/58991581>
     6
     7        Unskip test which should no longer be flaky.
     8
     9        * platform/ios-wk2/TestExpectations:
     10
    1112020-01-31  Jason Lawrence  <lawrence.j@apple.com>
    212

  • trunk/LayoutTests/platform/ios-wk2/TestExpectations

    r255462 r255518  
    13601360webkit.org/b/206752 imported/w3c/web-platform-tests/2dcontext/imagebitmap/createImageBitmap-serializable.html [ Pass Failure ]
    13611361
    1362 webkit.org/b/206934 imported/w3c/web-platform-tests/IndexedDB/key-generators/reading-autoincrement-indexes-cursors.any.serviceworker.html [ Pass Failure ]
    1363 
    13641362webkit.org/b/206946 http/tests/security/cookies/third-party-cookie-blocking-main-frame.html [ Pass Timeout ]
    13651363webkit.org/b/206965 storage/indexeddb/intversion-long-queue-private.html [ Pass Failure ]

  • trunk/Source/WebKit/ChangeLog

    r255517 r255518  
     12020-01-31  Chris Dumez  <cdumez@apple.com>
     2
     3        [ iOS ] imported/w3c/web-platform-tests/IndexedDB/key-generators/reading-autoincrement-indexes-cursors.any.serviceworker.html is flaky failing.
     4        https://bugs.webkit.org/show_bug.cgi?id=206934
     5        <rdar://problem/58991581>
     6
     7        Reviewed by Brady Eidson.
     8
     9        Flakiness would happen when the service worker would take too long to launch and the responsiveness timer would fire and
     10        report the process as unresponsive while still launching or very shortly after. When a service worker is reported as
     11        unresponsive, we would kill it.
     12
     13        To address the issue, several changes were made:
     14        - Responsiveness checks are now disabled for slow builds (Debug, ASAN, GuardMalloc)
     15        - We only start the ResponsivenessTimer after the process has finished launching since the responsiveness
     16          check relies on IPC to the process and we cannot send the IPC until after the process has launched.
     17
     18        * UIProcess/CoordinatedGraphics/DrawingAreaProxyCoordinatedGraphics.cpp:
     19        (WebKit::DrawingAreaProxyCoordinatedGraphics::sendUpdateBackingStoreState):
     20        * UIProcess/ProvisionalPageProxy.cpp:
     21        (WebKit::ProvisionalPageProxy::goToBackForwardItem):
     22        * UIProcess/WebPageProxy.cpp:
     23        (WebKit::WebPageProxy::launchProcessForReload):
     24        (WebKit::WebPageProxy::launchProcessWithItem):
     25        (WebKit::WebPageProxy::loadRequestWithNavigationShared):
     26        (WebKit::WebPageProxy::loadFile):
     27        (WebKit::WebPageProxy::loadDataWithNavigationShared):
     28        (WebKit::WebPageProxy::loadAlternateHTML):
     29        (WebKit::WebPageProxy::loadWebArchiveData):
     30        (WebKit::WebPageProxy::navigateToPDFLinkWithSimulatedClick):
     31        (WebKit::WebPageProxy::stopLoading):
     32        (WebKit::WebPageProxy::reload):
     33        (WebKit::WebPageProxy::goToBackForwardItem):
     34        (WebKit::WebPageProxy::dispatchActivityStateChange):
     35        (WebKit::WebPageProxy::processNextQueuedMouseEvent):
     36        (WebKit::WebPageProxy::handleKeyboardEvent):
     37        (WebKit::WebPageProxy::handleGestureEvent):
     38        (WebKit::WebPageProxy::handlePreventableTouchEvent):
     39        (WebKit::WebPageProxy::handleTouchEvent):
     40        (WebKit::WebPageProxy::runJavaScriptAlert):
     41        (WebKit::WebPageProxy::runJavaScriptConfirm):
     42        (WebKit::WebPageProxy::runJavaScriptPrompt):
     43        (WebKit::WebPageProxy::runBeforeUnloadConfirmPanel):
     44        (WebKit::WebPageProxy::runOpenPanel):
     45        * UIProcess/WebProcessProxy.cpp:
     46        (WebKit::WebProcessProxy::mayBecomeUnresponsive):
     47        (WebKit::WebProcessProxy::didFinishLaunching):
     48        (WebKit::WebProcessProxy::startResponsivenessTimer):
     49        (WebKit::WebProcessProxy::isResponsive):
     50        * UIProcess/WebProcessProxy.h:
     51        (WebKit::WebProcessProxy::responsivenessTimer):
     52
    1532020-01-31  Kate Cheney  <katherine_cheney@apple.com>
    254

  • trunk/Source/WebKit/UIProcess/CoordinatedGraphics/DrawingAreaProxyCoordinatedGraphics.cpp

    r255060 r255518  
    176176
    177177    // Stop the responsiveness timer that was started in sendUpdateBackingStoreState.
    178     process().responsivenessTimer().stop();
     178    process().stopResponsivenessTimer();
    179179
    180180    if (layerTreeContext != m_layerTreeContext) {
    … …  
    320320        // Start the responsiveness timer. We will stop it when we hear back from the WebProcess
    321321        // in didUpdateBackingStoreState.
    322         process().responsivenessTimer().start();
     322        process().startResponsivenessTimer();
    323323    }
    324324

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

    r255458 r255518  
    182182    send(Messages::WebPage::UpdateBackForwardListForReattach(WTFMove(itemStates)));
    183183    send(Messages::WebPage::GoToBackForwardItem(navigation.navigationID(), item.itemID(), *navigation.backForwardFrameLoadType(), WebCore::ShouldTreatAsContinuingLoad::Yes, WTFMove(websitePolicies)));
    184     m_process->responsivenessTimer().start();
     184    m_process->startResponsivenessTimer();
    185185}
    186186

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

    r255461 r255518  
    977977    // We allow stale content when reloading a WebProcess that's been killed or crashed.
    978978    send(Messages::WebPage::GoToBackForwardItem(navigation->navigationID(), m_backForwardList->currentItem()->itemID(), FrameLoadType::IndexedBackForward, ShouldTreatAsContinuingLoad::No, WTF::nullopt));
    979     m_process->responsivenessTimer().start();
     979    m_process->startResponsivenessTimer();
    980980
    981981    return navigation;
    … …  
    10001000
    10011001    send(Messages::WebPage::GoToBackForwardItem(navigation->navigationID(), item.itemID(), FrameLoadType::IndexedBackForward, ShouldTreatAsContinuingLoad::No, WTF::nullopt));
    1002     m_process->responsivenessTimer().start();
     1002    m_process->startResponsivenessTimer();
    10031003
    10041004    return navigation;
    … …  
    12901290    process->send(Messages::WebPage::LoadRequest(loadParameters), webPageID);
    12911291#endif
    1292     process->responsivenessTimer().start();
     1292    process->startResponsivenessTimer();
    12931293}
    12941294
    … …  
    13451345    send(Messages::WebPage::LoadRequest(loadParameters));
    13461346#endif
    1347     m_process->responsivenessTimer().start();
     1347    m_process->startResponsivenessTimer();
    13481348
    13491349    return navigation;
    … …  
    13911391    process->assumeReadAccessToBaseURL(*this, baseURL);
    13921392    process->send(Messages::WebPage::LoadData(loadParameters), webPageID);
    1393     process->responsivenessTimer().start();
     1393    process->startResponsivenessTimer();
    13941394}
    13951395
    … …  
    14341434    m_process->assumeReadAccessToBaseURL(*this, unreachableURL);
    14351435    send(Messages::WebPage::LoadAlternateHTML(loadParameters));
    1436     m_process->responsivenessTimer().start();
     1436    m_process->startResponsivenessTimer();
    14371437}
    14381438
    … …  
    14611461
    14621462    send(Messages::WebPage::LoadData(loadParameters));
    1463     m_process->responsivenessTimer().start();
     1463    m_process->startResponsivenessTimer();
    14641464}
    14651465
    … …  
    14801480
    14811481    send(Messages::WebPage::NavigateToPDFLinkWithSimulatedClick(urlString, documentPoint, screenPoint));
    1482     m_process->responsivenessTimer().start();
     1482    m_process->startResponsivenessTimer();
    14831483}
    14841484
    … …  
    14971497        m_provisionalPage = nullptr;
    14981498    }
    1499     m_process->responsivenessTimer().start();
     1499    m_process->startResponsivenessTimer();
    15001500}
    15011501
    … …  
    15281528
    15291529    send(Messages::WebPage::Reload(navigation->navigationID(), options.toRaw(), sandboxExtensionHandle));
    1530     m_process->responsivenessTimer().start();
     1530    m_process->startResponsivenessTimer();
    15311531
    15321532#if ENABLE(SPEECH_SYNTHESIS)
    … …  
    15971597
    15981598    send(Messages::WebPage::GoToBackForwardItem(navigation ? navigation->navigationID() : 0, item.itemID(), frameLoadType, ShouldTreatAsContinuingLoad::No, WTF::nullopt));
    1599     m_process->responsivenessTimer().start();
     1599    m_process->startResponsivenessTimer();
    16001600
    16011601    return navigation;
    … …  
    19511951            // state, it might not send back a reply (since it won't paint anything if the web page is hidden) so we
    19521952            // stop the unresponsiveness timer here.
    1953             m_process->responsivenessTimer().stop();
     1953            m_process->stopResponsivenessTimer();
    19541954        }
    19551955    }
    … …  
    24812481    WebEvent::Type eventType = event.type();
    24822482    if (eventType == WebEvent::MouseDown || eventType == WebEvent::MouseForceChanged || eventType == WebEvent::MouseForceDown)
    2483         m_process->responsivenessTimer().startWithLazyStop();
     2483        m_process->startResponsivenessTimer(WebProcessProxy::UseLazyStop::Yes);
    24842484    else if (eventType != WebEvent::MouseMove) {
    24852485        // NOTE: This does not start the responsiveness timer because mouse move should not indicate interaction.
    2486         m_process->responsivenessTimer().start();
     2486        m_process->startResponsivenessTimer();
    24872487    }
    24882488
    … …  
    26732673    m_keyEventQueue.append(event);
    26742674
    2675     ResponsivenessTimer& responsivenessTimer = m_process->responsivenessTimer();
    2676     if (event.type() == WebEvent::KeyDown)
    2677         responsivenessTimer.startWithLazyStop();
    2678     else
    2679         responsivenessTimer.start();
     2675    m_process->startResponsivenessTimer(event.type() == WebEvent::KeyDown ? WebProcessProxy::UseLazyStop::Yes : WebProcessProxy::UseLazyStop::No);
    26802676
    26812677    if (m_keyEventQueue.size() == 1) { // Otherwise, sent from DidReceiveEvent message handler.
    … …  
    28482844    // FIXME: Consider doing some coalescing here.
    28492845
    2850     ResponsivenessTimer& responsivenessTimer = m_process->responsivenessTimer();
    2851     if (event.type() == WebEvent::GestureStart || event.type() == WebEvent::GestureChange)
    2852         responsivenessTimer.startWithLazyStop();
    2853     else
    2854         responsivenessTimer.start();
     2846    m_process->startResponsivenessTimer((event.type() == WebEvent::GestureStart || event.type() == WebEvent::GestureChange) ? WebProcessProxy::UseLazyStop::Yes ? WebProcessProxy::UseLazyStop::No);
    28552847
    28562848    send(Messages::EventDispatcher::GestureEvent(m_webPageID, event), 0);
    … …  
    29232915    }
    29242916
    2925     m_process->responsivenessTimer().start();
     2917    m_process->startResponsivenessTimer();
    29262918    bool handled = false;
    29272919    bool replyReceived = sendSync(Messages::WebPage::TouchEventSync(event), Messages::WebPage::TouchEventSync::Reply(handled), 1_s, IPC::SendSyncOption::ForceDispatchWhenDestinationIsWaitingForUnboundedSyncReply);
    … …  
    29352927    else if (handled)
    29362928        m_handledSynchronousTouchEventWhileDispatchingPreventableTouchStart = true;
    2937     m_process->responsivenessTimer().stop();
     2929    m_process->stopResponsivenessTimer();
    29382930}
    29392931
    … …  
    29792971    if (!m_isPageSuspended) {
    29802972        m_touchEventQueue.append(event);
    2981         m_process->responsivenessTimer().start();
     2973        m_process->startResponsivenessTimer();
    29822974        send(Messages::WebPage::TouchEvent(event));
    29832975    } else {
    … …  
    54565448
    54575449    // Since runJavaScriptAlert() can spin a nested run loop we need to turn off the responsiveness timer.
    5458     m_process->responsivenessTimer().stop();
     5450    m_process->stopResponsivenessTimer();
    54595451
    54605452    if (m_controlledByAutomation) {
    … …  
    54755467
    54765468    // Since runJavaScriptConfirm() can spin a nested run loop we need to turn off the responsiveness timer.
    5477     m_process->responsivenessTimer().stop();
     5469    m_process->stopResponsivenessTimer();
    54785470
    54795471    if (m_controlledByAutomation) {
    … …  
    54945486#endif
    54955487    // Since runJavaScriptPrompt() can spin a nested run loop we need to turn off the responsiveness timer.
    5496     m_process->responsivenessTimer().stop();
     5488    m_process->stopResponsivenessTimer();
    54975489
    54985490    if (m_controlledByAutomation) {
    … …  
    56615653
    56625654    // Since runBeforeUnloadConfirmPanel() can spin a nested run loop we need to turn off the responsiveness timer and the tryClose timer.
    5663     m_process->responsivenessTimer().stop();
     5655    m_process->stopResponsivenessTimer();
    56645656    m_tryCloseTimeoutTimer.stop();
    56655657    m_uiClient->runBeforeUnloadConfirmPanel(*this, message, frame, WTFMove(securityOrigin), WTFMove(reply));
    … …  
    57085700
    57095701    // Since runOpenPanel() can spin a nested run loop we need to turn off the responsiveness timer.
    5710     m_process->responsivenessTimer().stop();
     5702    m_process->stopResponsivenessTimer();
    57115703
    57125704    if (!m_uiClient->runOpenPanel(*this, frame, WTFMove(frameSecurityOrigin), parameters.ptr(), m_openPanelResultListener.get())) {
    … …  
    62966288
    62976289    // Since showPopupMenu() can spin a nested run loop we need to turn off the responsiveness timer.
    6298     m_process->responsivenessTimer().stop();
     6290    m_process->stopResponsivenessTimer();
    62996291
    63006292    // Showing a popup menu runs a nested runloop, which can handle messages that cause |this| to get closed.
    … …  
    63396331
    63406332    // Since showContextMenu() can spin a nested run loop we need to turn off the responsiveness timer.
    6341     m_process->responsivenessTimer().stop();
     6333    m_process->stopResponsivenessTimer();
    63426334
    63436335    // m_activeContextMenu might get cleared if WebPageProxy code is re-entered from the menu runloop or delegates.
    … …  
    66866678    case WebEvent::GestureEnd:
    66876679#endif
    6688         m_process->responsivenessTimer().stop();
     6680        m_process->stopResponsivenessTimer();
    66896681        break;
    66906682    }
    … …  
    79417933{
    79427934    // Since runModal() can (and probably will) spin a nested run loop we need to turn off the responsiveness timer.
    7943     m_process->responsivenessTimer().stop();
     7935    m_process->stopResponsivenessTimer();
    79447936
    79457937    // Our Connection's run loop might have more messages waiting to be handled after this RunModal message.

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

    r255438 r255518  
    864864bool WebProcessProxy::mayBecomeUnresponsive()
    865865{
    866     return !platformIsBeingDebugged();
     866#if !defined(NDEBUG) || ASAN_ENABLED
     867    // Disable responsiveness checks in slow builds to avoid false positives.
     868    return false;
     869#else
     870    if (platformIsBeingDebugged())
     871        return false;
     872
     873    static bool isLibgmallocEnabled = [] {
     874        char* variable = getenv("DYLD_INSERT_LIBRARIES");
     875        if (!variable)
     876            return false;
     877        if (!strstr(variable, "libgmalloc"))
     878            return false;
     879        return true;
     880    }();
     881    if (isLibgmallocEnabled)
     882        return false;
     883
     884    return true;
     885#endif
    867886}
    868887
    … …  
    893912    unblockAccessibilityServerIfNeeded();
    894913#endif
     914
     915    if (m_shouldStartResponsivenessTimerWhenLaunched) {
     916        auto useLazyStop = *std::exchange(m_shouldStartResponsivenessTimerWhenLaunched, WTF::nullopt);
     917        startResponsivenessTimer(useLazyStop);
     918    }
    895919}
    896920
    … …  
    11211145{
    11221146    responsivenessTimer().stop();
     1147}
     1148
     1149void WebProcessProxy::startResponsivenessTimer(UseLazyStop useLazyStop)
     1150{
     1151    if (isLaunching()) {
     1152        m_shouldStartResponsivenessTimerWhenLaunched = useLazyStop;
     1153        return;
     1154    }
     1155
     1156    if (useLazyStop == UseLazyStop::Yes)
     1157        responsivenessTimer().startWithLazyStop();
     1158    else
     1159        responsivenessTimer().start();
    11231160}
    11241161
    … …  
    13431380        m_isResponsiveCallbacks.append(WTFMove(callback));
    13441381
    1345     responsivenessTimer().start();
     1382    startResponsivenessTimer();
    13461383    send(Messages::WebProcess::MainThreadPing(), 0);
    13471384}

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

    r255457 r255518  
    176176    RefPtr<API::UserInitiatedAction> userInitiatedActivity(uint64_t);
    177177
    178     ResponsivenessTimer& responsivenessTimer() { return m_responsivenessTimer; }
    179178    bool isResponsive() const;
    180179
    … …  
    218217    void requestTermination(ProcessTerminationReason);
    219218
     219    enum class UseLazyStop : bool { No, Yes };
     220    void startResponsivenessTimer(UseLazyStop = UseLazyStop::No);
    220221    void stopResponsivenessTimer();
    221222
    … …  
    417418    static const HashSet<String>& platformPathsWithAssumedReadAccess();
    418419
     420    ResponsivenessTimer& responsivenessTimer() { return m_responsivenessTimer; }
    419421    void updateBackgroundResponsivenessTimer();
    420422
    … …  
    541543    bool m_hasIssuedAttachmentElementRelatedSandboxExtensions { false };
    542544#endif
     545    Optional<UseLazyStop> m_shouldStartResponsivenessTimerWhenLaunched;
    543546
    544547#if PLATFORM(WATCHOS)
Note: See TracChangeset for help on using the changeset viewer.