Add a FrameLoaderClient willInjectUserScriptForFrame callback
Created attachment 344783 [details] WIP
Created attachment 344786 [details] Patch
Attachment 344786 [details] did not pass style-queue: ERROR: Source/WebCore/loader/FrameLoaderClient.h:335: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 1 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.
<rdar://problem/42095701>
What's the use case for this new API callback?
(In reply to Sam Weinig from comment #5) > What's the use case for this new API callback? Apps on top of WebKit can currently rely on globalObjectIsAvailableForFrame to properly initialize worlds. globalObjectIsAvailableForFrame is called at the time of receiving the first page byte. This works well if the world is created before that, it fails otherwise and may lead to executing scripts in worlds that are not properly set up. This new callback allows to properly set up worlds just before executing a user script.
It looks like this doesn’t apply cleanly after https://trac.webkit.org/changeset/233752/webkit.
(In reply to Brian Weinstein from comment #7) > It looks like this doesn’t apply cleanly after > https://trac.webkit.org/changeset/233752/webkit. Yep, the call to this->loader().client().willInjectUserScript(world); probably needs to be rebased.
(In reply to youenn fablet from comment #8) > (In reply to Brian Weinstein from comment #7) > > It looks like this doesn’t apply cleanly after > > https://trac.webkit.org/changeset/233752/webkit. > > Yep, the call to this->loader().client().willInjectUserScript(world); > probably needs to be rebased. It should go in Frame::injectUserScriptImmediately, correct? Just for my testing, I have: void Frame::injectUserScriptImmediately(DOMWrapperWorld& world, const UserScript& script) { auto* document = this->document(); if (!document) return; if (script.injectedFrames() == InjectInTopFrameOnly && !isMainFrame()) return; if (!UserContentURLPattern::matchesPatterns(document->url(), script.whitelist(), script.blacklist())) return; if (m_page) m_page->setAsRunningUserScripts(); this->loader().client().willInjectUserScript(world); m_script->evaluateInWorld(ScriptSourceCode(script.source(), script.url()), world); }
Yes, I ll reupload a new version shortly after linch
(In reply to Sam Weinig from comment #5) > What's the use case for this new API callback? We are currently using globalObjectIsAvailableForFrame to set up things if we have a user script, but if we don't have a user script then use _addUserScriptImmediately (introduced in r233752) then things won't be set up when a user script is injected. Moving the setup to this new callback fixes the problem without compatibility issues.
Created attachment 344874 [details] Rebasing
Attachment 344874 [details] did not pass style-queue: ERROR: Source/WebCore/loader/FrameLoaderClient.h:335: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 1 in 25 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 344874 [details] Rebasing Clearing flags on attachment: 344874 Committed r233782: <https://trac.webkit.org/changeset/233782>
All reviewed patches have been landed. Closing bug.
Looks like the new test http/tests/contentextensions/injected-script-callback.html Is crashing constantly on Debug WK2 platforms. Test History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fcontentextensions%2Finjected-script-callback.html Link to crash log: https://build.webkit.org/results/Apple%20High%20Sierra%20Debug%20WK2%20(Tests)/r233795%20(4095)/http/tests/contentextensions/injected-script-callback-crash-log.txt
Thanks truitt, I’ll look at it today
Committed r233799: <https://trac.webkit.org/changeset/233799>