Changeset 260653 in webkit


Ignore:
Timestamp:
Apr 24, 2020 10:50:41 AM (4 years ago)
Author:
BJ Burg
Message:

Web Automation: timeout underneath Automation.evaluateJavaScriptFunction in Selenium test frame_switching_tests.py::testShouldNotBeAbleToDoAnythingTheFrameIsDeletedFromUnderUs[Safari]
​https://bugs.webkit.org/show_bug.cgi?id=210162
<rdar://problem/60561009>

Reviewed by Devin Rousso.

Source/WebCore:

  • page/DOMWindow.h: Expose DOMWindow::{register, unregister}Observer.

Source/WebKit:

When an iframe is detached from the DOM, it is no longer exposed as a browsing context
and it's not possible to Evaluate JavaScript or perform other commands with it. This
patch adds frame lifecycle monitoring so that pending script evaluations are cancelled
with FrameNotFound as soon as the iframe is detached from the DOM. This change also avoids
running more commands with the frame if it's detached from its DOMWindow and ready to be GC'd.

  • Sources.txt:
  • WebKit.xcodeproj/project.pbxproj:
  • WebProcess/Automation/WebAutomationDOMWindowObserver.h: Added.
  • WebProcess/Automation/WebAutomationDOMWindowObserver.cpp: Added.

(WebKit::WebAutomationDOMWindowObserver::WebAutomationDOMWindowObserver):
(WebKit::WebAutomationDOMWindowObserver::~WebAutomationDOMWindowObserver):
(WebKit::WebAutomationDOMWindowObserver::frame const):
(WebKit::WebAutomationDOMWindowObserver::willDestroyGlobalObjectInCachedFrame):
(WebKit::WebAutomationDOMWindowObserver::willDestroyGlobalObjectInFrame):
(WebKit::WebAutomationDOMWindowObserver::willDetachGlobalObjectFromFrame):
This class is a stripped-down copy of DOMWindowExtension, which is the only other
client of DOMWindow::Observer interface. When a frame is detached, destroyed, or
navigates (global object detached), then call the callback and unregister.

  • WebProcess/Automation/WebAutomationSessionProxy.h:
  • WebProcess/Automation/WebAutomationSessionProxy.cpp:

(WebKit::WebAutomationSessionProxy::~WebAutomationSessionProxy):
(WebKit::WebAutomationSessionProxy::didClearWindowObjectForFrame):
(WebKit::WebAutomationSessionProxy::willDestroyGlobalObjectForFrame):
(WebKit::WebAutomationSessionProxy::evaluateJavaScriptFunction):
(WebKit::WebAutomationSessionProxy::ensureObserverForFrame): For non-main frames,
ensure we add a frame observer if we are about to evaluate JavaScript upon the frame.
This acts as a watchdog in case the frame becomes detached while waiting for pending
JS evaluations. When a frame is detached, the JS evaluation may or may not complete.

(WebKit::WebAutomationSessionProxy::selectOptionElement): Fix hilarious typo.

  • WebProcess/GPU/graphics/cocoa/ImageBufferShareableIOSurfaceBackend.cpp:
  • WebProcess/GPU/media/WebMediaStrategy.cpp:

(WebKit::WebMediaStrategy::clearNowPlayingInfo):
(WebKit::WebMediaStrategy::setNowPlayingInfo):
Adding a new file seems to have exposed a few missing includes and namespace qualifiers.
This is due to unified sources chunking.

Location:
trunk/Source
Files:
2 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r260647 r260653  
     12020-04-24  Brian Burg  <bburg@apple.com>
     2
     3        Web Automation: timeout underneath Automation.evaluateJavaScriptFunction in Selenium test frame_switching_tests.py::testShouldNotBeAbleToDoAnythingTheFrameIsDeletedFromUnderUs[Safari]
     4        https://bugs.webkit.org/show_bug.cgi?id=210162
     5        <rdar://problem/60561009>
     6
     7        Reviewed by Devin Rousso.
     8
     9        * page/DOMWindow.h: Expose DOMWindow::{register, unregister}Observer.
     10
    1112020-04-24  Zalan Bujtas  <zalan@apple.com>
    212

  • trunk/Source/WebCore/page/DOMWindow.h

    r258850 r260653  
    141141    };
    142142
    143     void registerObserver(Observer&);
    144     void unregisterObserver(Observer&);
     143    WEBCORE_EXPORT void registerObserver(Observer&);
     144    WEBCORE_EXPORT void unregisterObserver(Observer&);
    145145
    146146    void resetUnlessSuspendedForDocumentSuspension();

  • trunk/Source/WebKit/ChangeLog

    r260652 r260653  
     12020-04-24  Brian Burg  <bburg@apple.com>
     2
     3        Web Automation: timeout underneath Automation.evaluateJavaScriptFunction in Selenium test frame_switching_tests.py::testShouldNotBeAbleToDoAnythingTheFrameIsDeletedFromUnderUs[Safari]
     4        https://bugs.webkit.org/show_bug.cgi?id=210162
     5        <rdar://problem/60561009>
     6
     7        Reviewed by Devin Rousso.
     8
     9        When an iframe is detached from the DOM, it is no longer exposed as a browsing context
     10        and it's not possible to Evaluate JavaScript or perform other commands with it. This
     11        patch adds frame lifecycle monitoring so that pending script evaluations are cancelled
     12        with FrameNotFound as soon as the iframe is detached from the DOM. This change also avoids
     13        running more commands with the frame if it's detached from its DOMWindow and ready to be GC'd.
     14
     15        * Sources.txt:
     16        * WebKit.xcodeproj/project.pbxproj:
     17        * WebProcess/Automation/WebAutomationDOMWindowObserver.h: Added.
     18        * WebProcess/Automation/WebAutomationDOMWindowObserver.cpp: Added.
     19        (WebKit::WebAutomationDOMWindowObserver::WebAutomationDOMWindowObserver):
     20        (WebKit::WebAutomationDOMWindowObserver::~WebAutomationDOMWindowObserver):
     21        (WebKit::WebAutomationDOMWindowObserver::frame const):
     22        (WebKit::WebAutomationDOMWindowObserver::willDestroyGlobalObjectInCachedFrame):
     23        (WebKit::WebAutomationDOMWindowObserver::willDestroyGlobalObjectInFrame):
     24        (WebKit::WebAutomationDOMWindowObserver::willDetachGlobalObjectFromFrame):
     25        This class is a stripped-down copy of DOMWindowExtension, which is the only other
     26        client of DOMWindow::Observer interface. When a frame is detached, destroyed, or
     27        navigates (global object detached), then call the callback and unregister.
     28
     29        * WebProcess/Automation/WebAutomationSessionProxy.h:
     30        * WebProcess/Automation/WebAutomationSessionProxy.cpp:
     31        (WebKit::WebAutomationSessionProxy::~WebAutomationSessionProxy):
     32        (WebKit::WebAutomationSessionProxy::didClearWindowObjectForFrame):
     33        (WebKit::WebAutomationSessionProxy::willDestroyGlobalObjectForFrame):
     34        (WebKit::WebAutomationSessionProxy::evaluateJavaScriptFunction):
     35        (WebKit::WebAutomationSessionProxy::ensureObserverForFrame): For non-main frames,
     36        ensure we add a frame observer if we are about to evaluate JavaScript upon the frame.
     37        This acts as a watchdog in case the frame becomes detached while waiting for pending
     38        JS evaluations. When a frame is detached, the JS evaluation may or may not complete.
     39
     40        (WebKit::WebAutomationSessionProxy::selectOptionElement): Fix hilarious typo.
     41
     42        * WebProcess/GPU/graphics/cocoa/ImageBufferShareableIOSurfaceBackend.cpp:
     43        * WebProcess/GPU/media/WebMediaStrategy.cpp:
     44        (WebKit::WebMediaStrategy::clearNowPlayingInfo):
     45        (WebKit::WebMediaStrategy::setNowPlayingInfo):
     46        Adding a new file seems to have exposed a few missing includes and namespace qualifiers.
     47        This is due to unified sources chunking.
     48
    1492020-04-24  Alex Christensen  <achristensen@webkit.org>
    250

  • trunk/Source/WebKit/Sources.txt

    r260182 r260653  
    486486// FIXME(206266): AutomationProtocolObjects.h's "namespace Protocol" conflicts with objc/runtime.h's "typedef struct objc_object Protocol"
    487487WebProcess/Automation/WebAutomationSessionProxy.cpp @no-unify
     488WebProcess/Automation/WebAutomationDOMWindowObserver.cpp
    488489
    489490WebProcess/Cache/WebCacheStorageConnection.cpp

  • trunk/Source/WebKit/WebKit.xcodeproj/project.pbxproj

    r260652 r260653  
    13931393                990D28BB1C6539D300986977 /* AutomationSessionClient.h in Headers */ = {isa = PBXBuildFile; fileRef = 990D28B71C6539A000986977 /* AutomationSessionClient.h */; };
    13941394                990D28C01C6553F100986977 /* APIAutomationSessionClient.h in Headers */ = {isa = PBXBuildFile; fileRef = 990D28B31C6526D400986977 /* APIAutomationSessionClient.h */; };
     1395                990D39F5243BE64800199388 /* WebAutomationDOMWindowObserver.h in Headers */ = {isa = PBXBuildFile; fileRef = 990D39F3243BE64700199388 /* WebAutomationDOMWindowObserver.h */; };
    13951396                990E1E092384AA57004602DF /* _WKRemoteWebInspectorViewControllerPrivate.h in Headers */ = {isa = PBXBuildFile; fileRef = 990E1E082384A88B004602DF /* _WKRemoteWebInspectorViewControllerPrivate.h */; settings = {ATTRIBUTES = (Private, ); }; };
    13961397                991F492F23A812C60054642B /* _WKInspectorDebuggableInfo.h in Headers */ = {isa = PBXBuildFile; fileRef = 991F492D23A812C50054642B /* _WKInspectorDebuggableInfo.h */; settings = {ATTRIBUTES = (Private, ); }; };
    … …  
    44424443                990D28B71C6539A000986977 /* AutomationSessionClient.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AutomationSessionClient.h; sourceTree = "<group>"; };
    44434444                990D28B81C6539A000986977 /* AutomationSessionClient.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = AutomationSessionClient.mm; sourceTree = "<group>"; };
     4445                990D39F2243BE64700199388 /* WebAutomationDOMWindowObserver.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WebAutomationDOMWindowObserver.cpp; sourceTree = "<group>"; };
     4446                990D39F3243BE64700199388 /* WebAutomationDOMWindowObserver.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = WebAutomationDOMWindowObserver.h; sourceTree = "<group>"; };
    44444447                990E1E082384A88B004602DF /* _WKRemoteWebInspectorViewControllerPrivate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = _WKRemoteWebInspectorViewControllerPrivate.h; sourceTree = "<group>"; };
    44454448                991F492D23A812C50054642B /* _WKInspectorDebuggableInfo.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = _WKInspectorDebuggableInfo.h; sourceTree = "<group>"; };
    … …  
    64606463                        isa = PBXGroup;
    64616464                        children = (
     6465                                990D39F2243BE64700199388 /* WebAutomationDOMWindowObserver.cpp */,
     6466                                990D39F3243BE64700199388 /* WebAutomationDOMWindowObserver.h */,
    64626467                                1C0A19441C8FF1A800FE0EBB /* WebAutomationSessionProxy.cpp */,
    64636468                                1C0A19451C8FF1A800FE0EBB /* WebAutomationSessionProxy.h */,
    … …  
    1116811173                                57DCED712142EE6C0016B847 /* WebAuthenticatorCoordinatorProxyMessages.h in Headers */,
    1116911174                                F42D634122A0EFDF00D2FB3A /* WebAutocorrectionData.h in Headers */,
     11175                                990D39F5243BE64800199388 /* WebAutomationDOMWindowObserver.h in Headers */,
    1117011176                                9955A6EC1C7980C200EB6A93 /* WebAutomationSession.h in Headers */,
    1117111177                                99C3AE2D1DADA6AD00AF5C16 /* WebAutomationSessionMacros.h in Headers */,

  • trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp

    r258663 r260653  
    2929#include "AutomationProtocolObjects.h"
    3030#include "CoordinateSystem.h"
     31#include "WebAutomationDOMWindowObserver.h"
    3132#include "WebAutomationSessionMessages.h"
    3233#include "WebAutomationSessionProxyMessages.h"
    … …  
    6566#endif
    6667
     68using namespace WebCore;
     69
    6770namespace WebKit {
    6871
    … …  
    117120WebAutomationSessionProxy::~WebAutomationSessionProxy()
    118121{
     122    m_frameObservers.clear();
     123
    119124    WebProcess::singleton().removeMessageReceiver(Messages::WebAutomationSessionProxy::messageReceiverName());
    120125}
    … …  
    299304}
    300305
     306void WebAutomationSessionProxy::ensureObserverForFrame(WebFrame& frame)
     307{
     308    // If the frame and DOMWindow have become disconnected, then frame is already being destroyed
     309    // and there is no way to get access to the frame from the observer's DOMWindow reference.
     310    if (!frame.coreFrame()->window() || !frame.coreFrame()->window()->frame())
     311        return;
     312
     313    if (m_frameObservers.contains(frame.frameID()))
     314        return;
     315
     316    auto frameID = frame.frameID();
     317    m_frameObservers.set(frameID, WebAutomationDOMWindowObserver::create(*frame.coreFrame()->window(), [this, frameID] (WebAutomationDOMWindowObserver&) {
     318        willDestroyGlobalObjectForFrame(frameID);
     319    }));
     320}
     321
    301322void WebAutomationSessionProxy::didClearWindowObjectForFrame(WebFrame& frame)
    302323{
     324    willDestroyGlobalObjectForFrame(frame.frameID());
     325}
     326
     327void WebAutomationSessionProxy::willDestroyGlobalObjectForFrame(WebCore::FrameIdentifier frameID)
     328{
     329    // The observer is no longer needed, let it become GC'd and unregister itself from DOMWindow.
     330    if (m_frameObservers.contains(frameID))
     331        m_frameObservers.remove(frameID);
     332
    303333    String errorMessage = "Callback was not called before the unload event."_s;
    304     String errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::JavaScriptError);
    305 
    306     auto pendingFrameCallbacks = m_webFramePendingEvaluateJavaScriptCallbacksMap.take(frame.frameID());
     334    String errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::FrameNotFound);
     335
     336    auto pendingFrameCallbacks = m_webFramePendingEvaluateJavaScriptCallbacksMap.take(frameID);
    307337    for (uint64_t callbackID : pendingFrameCallbacks)
    308338        WebProcess::singleton().parentProcessConnection()->send(Messages::WebAutomationSession::DidEvaluateJavaScriptFunction(callbackID, errorMessage, errorType), 0);
    … …  
    318348    }
    319349    auto* frame = optionalFrameID ? WebProcess::singleton().webFrame(*optionalFrameID) : &page->mainWebFrame();
    320     if (!frame) {
     350    if (!frame || !frame->coreFrame()->window() || !frame->coreFrame()->window()->frame()) {
    321351        WebProcess::singleton().parentProcessConnection()->send(Messages::WebAutomationSession::DidEvaluateJavaScriptFunction(callbackID, { },
    322352            Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::FrameNotFound)), 0);
    323353        return;
    324354    }
     355
     356    // No need to track the main frame, this is handled by didClearWindowObjectForFrame.
     357    if (!frame->coreFrame()->isMainFrame())
     358        ensureObserverForFrame(*frame);
    325359
    326360    JSObjectRef scriptObject = scriptObjectForFrame(*frame);
    … …  
    736770
    737771    if (!isValidNodeHandle(nodeHandle)) {
    738         String invalidNodeIdentifierrrorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidNodeIdentifier);
    739         completionHandler(invalidNodeIdentifierrrorType);
     772        String invalidNodeIdentifierErrorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::InvalidNodeIdentifier);
     773        completionHandler(invalidNodeIdentifierErrorType);
    740774        return;
    741775    }

  • trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.h

    r255343 r260653  
    4444class WebFrame;
    4545class WebPage;
     46class WebAutomationDOMWindowObserver;
    4647
    4748class WebAutomationSessionProxy : public IPC::MessageReceiver {
    … …  
    5455
    5556    void didClearWindowObjectForFrame(WebFrame&);
     57    void willDestroyGlobalObjectForFrame(WebCore::FrameIdentifier);
    5658
    5759    void didEvaluateJavaScriptFunction(WebCore::FrameIdentifier, uint64_t callbackID, const String& result, const String& errorType);
    … …  
    6264    JSObjectRef scriptObjectForFrame(WebFrame&);
    6365    WebCore::Element* elementForNodeHandle(WebFrame&, const String&);
     66
     67    void ensureObserverForFrame(WebFrame&);
    6468
    6569    // Implemented in generated WebAutomationSessionProxyMessageReceiver.cpp
    … …  
    8589
    8690    HashMap<WebCore::FrameIdentifier, Vector<uint64_t>> m_webFramePendingEvaluateJavaScriptCallbacksMap;
     91    HashMap<WebCore::FrameIdentifier, RefPtr<WebAutomationDOMWindowObserver>> m_frameObservers;
    8792};
    8893

  • trunk/Source/WebKit/WebProcess/GPU/graphics/cocoa/ImageBufferShareableIOSurfaceBackend.cpp

    r257730 r260653  
    3030#if HAVE(IOSURFACE)
    3131
     32#include <WebCore/GraphicsContextCG.h>
    3233#include <wtf/IsoMallocInlines.h>
    3334#include <wtf/StdLibExtras.h>

  • trunk/Source/WebKit/WebProcess/GPU/media/WebMediaStrategy.cpp

    r258040 r260653  
    6767    }
    6868#endif
    69     MediaSessionManagerCocoa::clearNowPlayingInfo();
     69    WebCore::MediaSessionManagerCocoa::clearNowPlayingInfo();
    7070}
    7171
    … …  
    7979    }
    8080#endif
    81     MediaSessionManagerCocoa::setNowPlayingInfo(setAsNowPlayingApplication, nowPlayingInfo);
     81    WebCore::MediaSessionManagerCocoa::setNowPlayingInfo(setAsNowPlayingApplication, nowPlayingInfo);
    8282}
    8383#endif
Note: See TracChangeset for help on using the changeset viewer.