Bug 62817 – [Qt] Expose runJavaScriptInMainFrame to WebKit2 UIProcess Qt API

• Component: WebKit2
• Status: RESOLVED INVALID

Want to expose the functionality of the WKPageRunJavaScriptInMainFrame function defined in Source/WebKit2/UIProcess/API/C/WKPage.h as a method in the Qt API class QWKPage defined in Source/WebKit2/UIProcess/API/qt.  The value returned should be converted to an instance of the Qt QVariant class.

Created attachment 98166 [details]
Proposed patch

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/qt/qwkpage_p.h:24:  Header file should not contain WebCore config.h. Should be: alphabetically sorted.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:49:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:60:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:63:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/qt/tests/qwkjavascriptresult/tst_qwkjavascriptresult.cpp:23:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/qt/tests/qwkjavascriptresult/tst_qwkjavascriptresult.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 6 in 12 files


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

Created attachment 98172 [details]
Proposed patch, addresses style complaint about including config.h in qwkpage_p.h.  Leaving include order unchanged.

Created attachment 98204 [details]
Proposed patch -- hopefully apply will succeed this time

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:25:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:50:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:61:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:64:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/qt/tests/qwkjavascriptresult/tst_qwkjavascriptresult.cpp:23:  Alphabetical sorting problem.  [build/include_order] [4]
Source/WebKit2/UIProcess/API/qt/tests/qwkjavascriptresult/tst_qwkjavascriptresult.cpp:27:  Alphabetical sorting problem.  [build/include_order] [4]
Total errors found: 6 in 13 files


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

Created attachment 98222 [details]
Proposed patch -- address remaining style complaints

Created attachment 98234 [details]
Proposed fix -- resubmitting after fix of build break introduced by r89461

[Andreas Kling]

Could you tell us about the use case for this feature?
And what happens if I execute a script that loops forever?

(In reply to comment #8)
> Could you tell us about the use case for this feature?
> And what happens if I execute a script that loops forever?

The use case I have in mind is making shallow DOM queries once a page is loaded, e.g. to get navigation tags (link rel="next" etc) or meta tags.  See the last unit test function in tst_qwkjavascriptresult.cpp for an example.

When using Qt with WebKit1 I would navigate the DOM tree directly.  A naive implementation of the same API in WebKit2 using HTMLElement proxy objects would require multiple requests between the UI and web processes.  Making the query in javascript avoids the multiple round-trip problem and provides a generally useful facility for manipulating the loaded web page.  It also allows some result processing to be offloaded to the web process, at the discretion of the caller.

I have not tested the code with a script that loops forever, but I imagine the same thing will happen that would happen if you loaded an HTML page with embedded javascript containing an infinite loop.

[Kenneth Rohde Christiansen]

Kimmo fixes this differently, cc\ing

(In reply to comment #10)
> Kimmo fixes this differently, cc\ing

OK, I see his fix is more direct, though less flexible.

I will file a separate bug to port his change -- actually a variation of it as I need access to meta tags, not just link tags.  I'll leave the I submitted for this bug here as a reference in case anyone needs it.

Any comments on the relative merits (and likelihood of garnering a favorable review) of retrieving both link and meta tags in the same query versus having separate retrieveHeadLinks and retrieveHeadMetas methods?

[Noam Rosenthal]

I think that we should figure out the big picture of whether or not we want to expose JS functionality to C++ in WebKit2. The current thinking was to avoid that. In any case, I think this needs a broader discussion than comments on a patch (i.e. mailing list).

[Benjamin Poulain]

(In reply to comment #12)
> I think that we should figure out the big picture of whether or not we want to expose JS functionality to C++ in WebKit2. The current thinking was to avoid that. In any case, I think this needs a broader discussion than comments on a patch (i.e. mailing list).

I totally agree.
We don't even have the base API yet. I think feature should remain in the C API only for now.

(In reply to comment #13)
> (In reply to comment #12)
> > I think that we should figure out the big picture of whether or not we want to expose JS functionality to C++ in WebKit2. The current thinking was to avoid that. In any case, I think this needs a broader discussion than comments on a patch (i.e. mailing list).
> 
> I totally agree.
> We don't even have the base API yet. I think feature should remain in the C API only for now.

Fair enough.

Filed and proposed patch for https://bugs.webkit.org/show_bug.cgi?id=62817 to add an API to retrieve link and meta element data directly.  Proposed patch there is based on Kimmo's patch mentioned in an earlier comment, but modifies it to get both link and meta element data, not just link element data.

[Benjamin Poulain]

Comment on attachment 98234 [details]
Proposed fix -- resubmitting after fix of build break introduced by r89461

Clear the review flag since everybody agree it is a bit early to add this kind of APIs.
I suggest to wait until the API cleanup, then resubmit this as a private API.

[Jocelyn Turcotte]

=== Bulk closing of Qt bugs ===

If you believe that this bug report is still relevant for a non-Qt port of webkit.org, please re-open it and remove [Qt] from the summary.

If you believe that this is still an important QtWebKit bug, please fill a new report at https://bugreports.qt-project.org and add a link to this issue. See http://qt-project.org/wiki/ReportingBugsInQt for additional guidelines.