Bug 45222 – Add ability to send UserData to willSubmitForm

• Component: WebKit2
• Status: RESOLVED FIXED

[Sam Weinig]

Add ability to send UserData to willSubmitForm

[Sam Weinig]

Created attachment 66581 [details]
Patch

[WebKit Review Bot]

Attachment 66581 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style']" exit_code: 1
WebKit2/WebProcess/InjectedBundle/API/c/WKBundlePage.h:150:  Extra space between WKBundlePageWillSubmitFormCallback and willSubmitForm  [whitespace/declaration] [3]
WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.h:35:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:31:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 3 in 12 files


If any of these errors are false positives, please file a bug against check-webkit-style.

Comment on attachment 66581 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66581&action=prettypatch

> WebKit2/UIProcess/WebPageProxy.h:205
> +    void willSubmitForm(WebFrameProxy* frame, WebFrameProxy* frameSource, Vector<std::pair<WTF::String, WTF::String> >& textFieldValues, APIObject*, uint64_t listenerID);
I think it would be helpful to name the argument here.

> WebKit2/WebProcess/InjectedBundle/InjectedBundlePageFormClient.h:45
> +class APIObject;
>  class WebFrame;
>  class WebPage;
> +class ImmutableDictionary;
Please sort.

r=me

[Adam Barth]

Comment on attachment 66581 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=66581&action=prettypatch

Mostly just curious what this code looked like.  Very pretty.  Some (trivial) comments below.

> WebKit2/UIProcess/WebPageProxy.cpp:581
> -            willSubmitForm(process()->webFrame(frameID), process()->webFrame(sourceFrameID), textFieldValues, listenerID);
> +            willSubmitForm(process()->webFrame(frameID), process()->webFrame(sourceFrameID), textFieldValues, 0/*userData*/, listenerID);
Slightly more beautiful would be a noUserData constant that was just a null pointer.

> WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:513
> +    RefPtr<FormState> formState = prpFormState;
I've never really like this pattern, but I don't see a way around it.

> WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:520
> +    RefPtr<APIObject> userData;
> +    webPage->injectedBundleFormClient().willSubmitForm(webPage, form, m_frame, sourceFrame, values, userData);
I'm confused.  Don't we need to actually allocate a userData object here?  Does willSubmitForm take a RefPtr by reference as an out param?

[Sam Weinig]

(In reply to comment #4)
> (From update of attachment 66581 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=66581&action=prettypatch
> 
> Mostly just curious what this code looked like.  Very pretty.  Some (trivial) comments below.
> 
> > WebKit2/UIProcess/WebPageProxy.cpp:581
> > -            willSubmitForm(process()->webFrame(frameID), process()->webFrame(sourceFrameID), textFieldValues, listenerID);
> > +            willSubmitForm(process()->webFrame(frameID), process()->webFrame(sourceFrameID), textFieldValues, 0/*userData*/, listenerID);
> Slightly more beautiful would be a noUserData constant that was just a null pointer.

Ok.

> 
> > WebKit2/WebProcess/WebCoreSupport/WebFrameLoaderClient.cpp:520
> > +    RefPtr<APIObject> userData;
> > +    webPage->injectedBundleFormClient().willSubmitForm(webPage, form, m_frame, sourceFrame, values, userData);
> I'm confused.  Don't we need to actually allocate a userData object here?  Does willSubmitForm take a RefPtr by reference as an out param?

Yes it does and very intentionally.

Thanks for the review.

[Eric Seidel (no email)]

Attachment 66581 [details] was posted by a committer and has review+, assigning to Sam Weinig for commit.

[Sam Weinig]

Fixed in http://trac.webkit.org/changeset/66789.