Safari needs a way to convert a JS window object or frame DOM element into a WKWebProcessPlugInFrame in the injected bundle for web extensions.
<rdar://problem/89030846>
Created attachment 452203 [details] Patch
Comment on attachment 452203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452203&action=review > Source/WebCore/page/Frame.cpp:1146 > + JSC::JSGlobalObject* globalObject = toJS(context); I would think you should be able to do: return globalObject ? activeDOMWindow(*globalObject).frame() : nullptr; No?
Comment on attachment 452203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452203&action=review >> Source/WebCore/page/Frame.cpp:1146 >> + JSC::JSGlobalObject* globalObject = toJS(context); > > I would think you should be able to do: > return globalObject ? activeDOMWindow(*globalObject).frame() : nullptr; > > No? And if so, we probably don't need this new function on the Frame class at all.
Comment on attachment 452203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452203&action=review >>> Source/WebCore/page/Frame.cpp:1146 >>> + JSC::JSGlobalObject* globalObject = toJS(context); >> >> I would think you should be able to do: >> return globalObject ? activeDOMWindow(*globalObject).frame() : nullptr; >> >> No? > > And if so, we probably don't need this new function on the Frame class at all. We need to be able to get the Frame from a JSHTMLIFrameElement, JSHTMLFrameElement or JSDOMWindow.
Comment on attachment 452203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=452203&action=review > Source/WebCore/page/Frame.cpp:1141 > +Frame* Frame::fromJSValue(JSContextRef context, JSValueRef valueRef) I think we should rename to make it clear we want a *content* frame, as suggested below. > Source/WebCore/page/Frame.cpp:1149 > + We can simplify a bit here: ``` if (auto* window = JSDOMWindow::toWrapped(vm, value)) return window->frame(); ``` > Source/WebCore/page/Frame.cpp:1159 > + if (auto* frameElement = JSC::jsDynamicCast<JSHTMLIFrameElement*>(vm, value)) I think this may look a bit better: ``` auto* jsNode = JSC::jsDynamicCast<JSNode*>(vm, value); if (!jsNode || !is<HTMLFrameOwnerElement>(jsNode->wrapped())) return nullptr; return downcast<HTMLFrameOwnerElement>(*jsNode->wrapped()).contentFrame(); ``` > Source/WebKit/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInFramePrivate.h:36 > ++ (instancetype)lookUpFrameFromJSValue:(JSValue *)value; I think we may want to rename this to lookUpContentFrameFromJSValue given the behavior. You don't really want the frame, you want the "content" frame in the case the value is a <frame> / <iframe>. We may also want to convey the fact that this only works for window and frame elements, maybe something like: lookUpContentFrameForWindowOrFrameElement:
Created attachment 452246 [details] Patch
Comment on attachment 452246 [details] Patch r=me
Committed r290001 (247387@main): <https://commits.webkit.org/247387@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 452246 [details].