Modified: trunk/Source/WebKit/ChangeLog (254327 => 254328)
--- trunk/Source/WebKit/ChangeLog 2020-01-10 06:51:56 UTC (rev 254327)
+++ trunk/Source/WebKit/ChangeLog 2020-01-10 08:42:11 UTC (rev 254328)
@@ -1,3 +1,32 @@
+2020-01-10 Carlos Garcia Campos <[email protected]>
+
+ Automation: scripts are executed in the wrong js context after a history navigation
+ https://bugs.webkit.org/show_bug.cgi?id=204880
+ <rdar://problem/58413615>
+
+ Reviewed by Brian Burg.
+
+ After a history navigation we use the script object from the previous frame js context because
+ didClearWindowObjectForFrame() is not called in that case. We are caching the script object for every frame ID,
+ and after a history navigation the frame ID is the same, but the frame js context isn't. That also means we might
+ be leaking the script objects in those cases, because we end up calling JSValueUnprotect with the wrong
+ context. It would be easier to set the script object as a property of the global object and let JSC handle the
+ lifetime. Instead of caching the script object and protect/unprotect it, we just check if the global object of
+ the current js context has the property or not to get or create it. We use a private symbol as the key of the
+ global object property to ensure it's not visible.
+
+ * WebProcess/Automation/WebAutomationSessionProxy.cpp:
+ (WebKit::WebAutomationSessionProxy::WebAutomationSessionProxy): Initialize m_scriptObjectIdentifier.
+ (WebKit::WebAutomationSessionProxy::scriptObject): Helper function to get the script object for the given
+ _javascript_ context.
+ (WebKit::WebAutomationSessionProxy::setScriptObject): Helper function to set the script object for the given
+ _javascript_ context.
+ (WebKit::WebAutomationSessionProxy::scriptObjectForFrame): Get or create the script object.
+ (WebKit::WebAutomationSessionProxy::elementForNodeHandle): Get the script object from global object.
+ (WebKit::WebAutomationSessionProxy::didClearWindowObjectForFrame): Remove the code to unprotect script objects
+ of the frame.
+ * WebProcess/Automation/WebAutomationSessionProxy.h: Add m_scriptObjectIdentifier and remove m_webFrameScriptObjectMap.
+
2020-01-09 Ross Kirsling <[email protected]>
REGRESSION(r253868): Socket-based remote inspector cannot inspect any target
Modified: trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp (254327 => 254328)
--- trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp 2020-01-10 06:51:56 UTC (rev 254327)
+++ trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.cpp 2020-01-10 08:42:11 UTC (rev 254328)
@@ -108,6 +108,7 @@
WebAutomationSessionProxy::WebAutomationSessionProxy(const String& sessionIdentifier)
: m_sessionIdentifier(sessionIdentifier)
+ , m_scriptObjectIdentifier(JSC::PrivateName::Description, "automationSessionProxy"_s)
{
WebProcess::singleton().addMessageReceiver(Messages::WebAutomationSessionProxy::messageReceiverName(), *this);
}
@@ -211,31 +212,48 @@
return JSValueMakeUndefined(context);
}
+JSObjectRef WebAutomationSessionProxy::scriptObject(JSGlobalContextRef context)
+{
+ JSC::JSGlobalObject* globalObject = toJS(context);
+ JSC::VM& vm = globalObject->vm();
+ JSC::JSLockHolder locker(vm);
+ auto scriptObjectID = JSC::Identifier::fromUid(m_scriptObjectIdentifier);
+ if (!globalObject->hasProperty(globalObject, scriptObjectID))
+ return nullptr;
+
+ return const_cast<JSObjectRef>(toRef(globalObject, globalObject->get(globalObject, scriptObjectID)));
+}
+
+void WebAutomationSessionProxy::setScriptObject(JSGlobalContextRef context, JSObjectRef object)
+{
+ JSC::JSGlobalObject* globalObject = toJS(context);
+ JSC::VM& vm = globalObject->vm();
+ JSC::JSLockHolder locker(vm);
+ auto scriptObjectID = JSC::Identifier::fromUid(m_scriptObjectIdentifier);
+ PutPropertySlot slot(globalObject);
+ globalObject->methodTable(vm)->put(globalObject, globalObject, scriptObjectID, toJS(globalObject, object), slot);
+}
+
JSObjectRef WebAutomationSessionProxy::scriptObjectForFrame(WebFrame& frame)
{
- if (JSObjectRef scriptObject = m_webFrameScriptObjectMap.get(frame.frameID()))
+ JSGlobalContextRef context = frame.jsContext();
+ if (auto* scriptObject = this->scriptObject(context))
return scriptObject;
JSValueRef exception = nullptr;
- JSGlobalContextRef context = frame.jsContext();
+ String script = StringImpl::createWithoutCopying(WebAutomationSessionProxyScriptSource, sizeof(WebAutomationSessionProxyScriptSource));
+ JSObjectRef scriptObjectFunction = const_cast<JSObjectRef>(JSEvaluateScript(context, OpaqueJSString::tryCreate(script).get(), nullptr, nullptr, 0, &exception));
+ ASSERT(JSValueIsObject(context, scriptObjectFunction));
JSValueRef sessionIdentifier = toJSValue(context, m_sessionIdentifier);
JSObjectRef evaluateFunction = JSObjectMakeFunctionWithCallback(context, nullptr, evaluate);
JSObjectRef createUUIDFunction = JSObjectMakeFunctionWithCallback(context, nullptr, createUUID);
JSObjectRef isValidNodeIdentifierFunction = JSObjectMakeFunctionWithCallback(context, nullptr, isValidNodeIdentifier);
-
- String script = StringImpl::createWithoutCopying(WebAutomationSessionProxyScriptSource, sizeof(WebAutomationSessionProxyScriptSource));
-
- JSObjectRef scriptObjectFunction = const_cast<JSObjectRef>(JSEvaluateScript(context, OpaqueJSString::tryCreate(script).get(), nullptr, nullptr, 0, &exception));
- ASSERT(JSValueIsObject(context, scriptObjectFunction));
-
JSValueRef arguments[] = { sessionIdentifier, evaluateFunction, createUUIDFunction, isValidNodeIdentifierFunction };
JSObjectRef scriptObject = const_cast<JSObjectRef>(JSObjectCallAsFunction(context, scriptObjectFunction, nullptr, WTF_ARRAY_LENGTH(arguments), arguments, &exception));
ASSERT(JSValueIsObject(context, scriptObject));
- JSValueProtect(context, scriptObject);
- m_webFrameScriptObjectMap.add(frame.frameID(), scriptObject);
-
+ setScriptObject(context, scriptObject);
return scriptObject;
}
@@ -244,12 +262,11 @@
// Don't use scriptObjectForFrame() since we can assume if the script object
// does not exist, there are no nodes mapped to handles. Using scriptObjectForFrame()
// will make a new script object if it can't find one, preventing us from returning fast.
- JSObjectRef scriptObject = m_webFrameScriptObjectMap.get(frame.frameID());
+ JSGlobalContextRef context = frame.jsContext();
+ auto* scriptObject = this->scriptObject(context);
if (!scriptObject)
return nullptr;
- JSGlobalContextRef context = frame.jsContext();
-
JSValueRef functionArguments[] = {
toJSValue(context, nodeHandle)
};
@@ -268,14 +285,10 @@
void WebAutomationSessionProxy::didClearWindowObjectForFrame(WebFrame& frame)
{
- WebCore::FrameIdentifier frameID = frame.frameID();
- if (JSObjectRef scriptObject = m_webFrameScriptObjectMap.take(frameID))
- JSValueUnprotect(frame.jsContext(), scriptObject);
-
String errorMessage = "Callback was not called before the unload event."_s;
String errorType = Inspector::Protocol::AutomationHelpers::getEnumConstantValue(Inspector::Protocol::Automation::ErrorMessage::_javascript_Error);
- auto pendingFrameCallbacks = m_webFramePendingEvaluateJavaScriptCallbacksMap.take(frameID);
+ auto pendingFrameCallbacks = m_webFramePendingEvaluateJavaScriptCallbacksMap.take(frame.frameID());
for (uint64_t callbackID : pendingFrameCallbacks)
WebProcess::singleton().parentProcessConnection()->send(Messages::WebAutomationSession::DidEvaluateJavaScriptFunction(callbackID, errorMessage, errorType), 0);
}
Modified: trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.h (254327 => 254328)
--- trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.h 2020-01-10 06:51:56 UTC (rev 254327)
+++ trunk/Source/WebKit/WebProcess/Automation/WebAutomationSessionProxy.h 2020-01-10 08:42:11 UTC (rev 254328)
@@ -28,6 +28,7 @@
#include "Connection.h"
#include "CoordinateSystem.h"
#include <_javascript_Core/JSBase.h>
+#include <_javascript_Core/PrivateName.h>
#include <WebCore/FrameIdentifier.h>
#include <WebCore/IntRect.h>
#include <WebCore/PageIdentifier.h>
@@ -56,6 +57,8 @@
void didEvaluateJavaScriptFunction(WebCore::FrameIdentifier, uint64_t callbackID, const String& result, const String& errorType);
private:
+ JSObjectRef scriptObject(JSGlobalContextRef);
+ void setScriptObject(JSGlobalContextRef, JSObjectRef);
JSObjectRef scriptObjectForFrame(WebFrame&);
WebCore::Element* elementForNodeHandle(WebFrame&, const String&);
@@ -77,8 +80,8 @@
void deleteCookie(WebCore::PageIdentifier, Optional<WebCore::FrameIdentifier>, String cookieName, CompletionHandler<void(Optional<String>)>&&);
String m_sessionIdentifier;
+ JSC::PrivateName m_scriptObjectIdentifier;
- HashMap<WebCore::FrameIdentifier, JSObjectRef> m_webFrameScriptObjectMap;
HashMap<WebCore::FrameIdentifier, Vector<uint64_t>> m_webFramePendingEvaluateJavaScriptCallbacksMap;
};
