In Bug 186788 I revised the InjectedBundle serialization to use NSSecureCoding. Since some users of InjectedBundles want to serialize custom classes, they need to tell the WebContent process which classes it is allowed to safely serialize. This is generally done in the bundle initialization function. Unfortunately, InjectedBundle::initialize attempts to decode the bundle parameters before running the initialization function, so misses any custom classes needed by the embedding application. This patch checks if serialization of the bundle parameters succeeded. If it did not, it attempts to decode the parameters after the initialization function runs so that it can take advantage of any new classes registered by the embedding application.
<rdar://problem/44573653>
Created attachment 350048 [details] Patch
Created attachment 350119 [details] Patch
Comment on attachment 350119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350119&action=review Could you clarify why we try to decode before and after the initialization function? Why do not we always decode after? > Source/WebKit/WebProcess/InjectedBundle/InjectedBundle.h:163 > + bool decodeBundleParameters(RefPtr<API::Data>); Why is this public? Also, why does it need to ref its parameter? can it simply be a API::Data*?
Comment on attachment 350119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350119&action=review >> Source/WebKit/WebProcess/InjectedBundle/InjectedBundle.h:163 >> + bool decodeBundleParameters(RefPtr<API::Data>); > > Why is this public? > Also, why does it need to ref its parameter? can it simply be a API::Data*? Sure -- it could be private and take a pointer. I'll change that.
Created attachment 350242 [details] Patch
(In reply to Chris Dumez from comment #4) > Comment on attachment 350119 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=350119&action=review > > Could you clarify why we try to decode before and after the initialization > function? Why do not we always decode after? You did not answer my question.
(In reply to Chris Dumez from comment #7) > > Could you clarify why we try to decode before and after the initialization > > function? Why do not we always decode after? > > You did not answer my question. Gosh! Somehow I missed this! I considered that, but wasn't sure if there might be cases where a initialization function depends on the contents of the bundle parameters. I can't give an actual instance where this is known to happen, but if there was such a bundle moving the code could break it. But I'm fine switching it around so we never pass through the 'throw' case.
Created attachment 350248 [details] Patch
Comment on attachment 350248 [details] Patch r=me if bots are happy.
Comment on attachment 350248 [details] Patch Clearing flags on attachment: 350248 Committed r236289: <https://trac.webkit.org/changeset/236289>
All reviewed patches have been landed. Closing bug.
This change caused 8 TestWebKitAPI.ContentFiltering test failures: https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK1%20%28Tests%29/builds/8104
Reverted r236289 for reason: Caused 8 TestWebKitAPI.ContentFiltering test failures. Committed r236304: <https://trac.webkit.org/changeset/236304>
From a debug bot: ASSERTION FAILED: testPlugInClassName /Volumes/Data/slave/sierra-debug/build/Tools/TestWebKitAPI/cocoa/WebProcessPlugIn/WebProcessPlugIn.mm(44) : -[WebProcessPlugIn webProcessPlugIn:initializeWithObject:] 1 0x126b6a4f9 WTFCrash 2 0x11797235b WTFCrashWithInfo(int, char const*, char const*, int) 3 0x117978e59 -[WebProcessPlugIn webProcessPlugIn:initializeWithObject:] 4 0x10df40b64 WebKit::InjectedBundle::initialize(WebKit::WebProcessCreationParameters const&, API::Object*) 5 0x10e1fb078 WebKit::InjectedBundle::create(WebKit::WebProcessCreationParameters&, API::Object*) 6 0x10e18e826 WebKit::WebProcess::initializeWebProcess(WebKit::WebProcessCreationParameters&&) 7 0x10e60ead6 void IPC::callMemberFunctionImpl<WebKit::WebProcess, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&), std::__1::tuple<WebKit::WebProcessCreationParameters>, 0ul>(WebKit::WebProcess*, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&), std::__1::tuple<WebKit::WebProcessCreationParameters>&&, std::__1::integer_sequence<unsigned long, 0ul>) 8 0x10e60e8e8 void IPC::callMemberFunction<WebKit::WebProcess, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&), std::__1::tuple<WebKit::WebProcessCreationParameters>, std::__1::integer_sequence<unsigned long, 0ul> >(std::__1::tuple<WebKit::WebProcessCreationParameters>&&, WebKit::WebProcess*, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&)) 9 0x10e608964 void IPC::handleMessage<Messages::WebProcess::InitializeWebProcess, WebKit::WebProcess, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&)>(IPC::Decoder&, WebKit::WebProcess*, void (WebKit::WebProcess::*)(WebKit::WebProcessCreationParameters&&)) 10 0x10e606475 WebKit::WebProcess::didReceiveWebProcessMessage(IPC::Connection&, IPC::Decoder&) 11 0x10e191d8b WebKit::WebProcess::didReceiveMessage(IPC::Connection&, IPC::Decoder&) 12 0x10d49fbfa IPC::Connection::dispatchMessage(IPC::Decoder&) 13 0x10d492d51 IPC::Connection::dispatchMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >) 14 0x10d4a0713 IPC::Connection::dispatchOneIncomingMessage() 15 0x10d4bd758 IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14::operator()() 16 0x10d4bd669 WTF::Function<void ()>::CallableWrapper<IPC::Connection::enqueueIncomingMessage(std::__1::unique_ptr<IPC::Decoder, std::__1::default_delete<IPC::Decoder> >)::$_14>::call() 17 0x126b917cd WTF::Function<void ()>::operator()() const 18 0x126be9873 WTF::RunLoop::performWork() 19 0x126bea204 WTF::RunLoop::performWork(void*) 20 0x7fff790a43e1 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ 21 0x7fff7908565c __CFRunLoopDoSources0 22 0x7fff79084b46 __CFRunLoopRun 23 0x7fff79084544 CFRunLoopRunSpecific 24 0x7fff785e3ebc RunCurrentEventLoopInMode 25 0x7fff785e3cf1 ReceiveNextEventCommon 26 0x7fff785e3b26 _BlockUntilNextEventMatchingListInModeWithFilter 27 0x7fff76b7aa54 _DPSNextEvent 28 0x7fff772f67ee -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] 29 0x7fff76b6f3db -[NSApplication run] 30 0x7fff76b39e0e NSApplicationMain 31 0x7fff8ee9d8c7 _xpc_objc_main https://build.webkit.org/builders/Apple%20Sierra%20Debug%20WK1%20%28Tests%29/builds/9628/steps/run-api-tests/logs/stdio
Created attachment 367406 [details] Patch
Created attachment 367422 [details] Patch
Comment on attachment 367422 [details] Patch Clearing flags on attachment: 367422 Committed r244299: <https://trac.webkit.org/changeset/244299>
Reverted r244299 for reason: Breaks internal tests. Committed r244431: <https://trac.webkit.org/changeset/244431>
This should be relanded once <rdar://problem/50024284> is finished.
Created attachment 367752 [details] Patch for landing
Created attachment 367753 [details] Patch for landing
Created attachment 367754 [details] Patch for landing
(In reply to Brent Fulgham from comment #21) > This should be relanded once <rdar://problem/50024284> is finished. Disregard this statement. It was not true for this bug.
Comment on attachment 367754 [details] Patch for landing Clearing flags on attachment: 367754 Committed r244437: <https://trac.webkit.org/changeset/244437>
Reverted r244437 for reason: Still breaks internal tests. Committed r244508: <https://trac.webkit.org/changeset/244508>
Created attachment 396437 [details] Patch for landing
Committed r260081: <https://trac.webkit.org/changeset/260081> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396437 [details].