Were added callbacks of initialize and terminate for WebWorkers. An API test was added to check the these callbacks. It verifies the two types of workers, dedicated and shared.
Created attachment 204363 [details] Patch with the changes.
Attachment 204363 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCore/bindings/js/WorkerScriptController.cpp', u'Source/WebCore/bindings/js/WorkerScriptController.h', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleFrame.cpp', u'Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleFrame.h', u'Tools/ChangeLog', u'Tools/TestWebKitAPI/PlatformEfl.cmake', u'Tools/TestWebKitAPI/Tests/WebKit2/WebWorker.cpp', u'Tools/TestWebKitAPI/Tests/WebKit2/WebWorkerSharedTest.html', u'Tools/TestWebKitAPI/Tests/WebKit2/WebWorkerSharedTest.js', u'Tools/TestWebKitAPI/Tests/WebKit2/WebWorkerTest.html', u'Tools/TestWebKitAPI/Tests/WebKit2/WebWorkerTest.js', u'Tools/TestWebKitAPI/Tests/WebKit2/WebWorker_Bundle.cpp']" exit_code: 1 Source/WebKit2/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Source/WebKit2/WebProcess/InjectedBundle/API/c/WKBundleFrame.cpp:41: Alphabetical sorting problem. [build/include_order] [4] Tools/TestWebKitAPI/Tests/WebKit2/WebWorker.cpp:53: Missing space before ( in if( [whitespace/parens] [5] Tools/TestWebKitAPI/Tests/WebKit2/WebWorker.cpp:57: Missing space before ( in if( [whitespace/parens] [5] Tools/TestWebKitAPI/Tests/WebKit2/WebWorker_Bundle.cpp:67: Missing space before ( in if( [whitespace/parens] [5] Source/WebCore/bindings/js/WorkerScriptController.h:87: The parameter name "callback" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/bindings/js/WorkerScriptController.h:89: The parameter name "callback" adds no information, so it should be removed. [readability/parameter_name] [5] Source/WebCore/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Tools/ChangeLog:1: ChangeLog entry has no bug number [changelog/bugnumber] [5] Total errors found: 9 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 204364 [details] Patch
Comment on attachment 204364 [details] Patch Attachment 204364 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/660923
Comment on attachment 204364 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=204364&action=review > Source/WebCore/ChangeLog:3 > + [WK2][EFL] Proof of concept to inject custom JavaScript objects on WebWorker context. I think this patch is for common WK2 ports rather than EFL WK2 port. > Source/WebCore/ChangeLog:7 > + Missing description. > Source/WebKit2/ChangeLog:7 > + ditto.
Comment on attachment 204364 [details] Patch Attachment 204364 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/738583
Thanks for the review. (In reply to comment #5) > (From update of attachment 204364 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=204364&action=review > > > Source/WebCore/ChangeLog:3 > > + [WK2][EFL] Proof of concept to inject custom JavaScript objects on WebWorker context. > > I think this patch is for common WK2 ports rather than EFL WK2 port. > I put the [EFL] because I changed a build file from EFL to run the API test. But I agreed with you, this is a change for all WK2, and removed this tag from summary. > > Source/WebCore/ChangeLog:7 > > + > > Missing description. > > > Source/WebKit2/ChangeLog:7 > > + > > ditto. I will put a description of the bug on a next patch. I'm working on fix the error at mac/mac-wk2 ews error to submit a new patch.
Created attachment 204638 [details] Patch
Comment on attachment 204638 [details] Patch Attachment 204638 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/899052
Comment on attachment 204638 [details] Patch Attachment 204638 [details] did not pass gtk-wk2-ews (gtk-wk2): Output: http://webkit-queues.appspot.com/results/874066
Comment on attachment 204638 [details] Patch Attachment 204638 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/905006
Created attachment 205214 [details] Patch
Comment on attachment 205214 [details] Patch Attachment 205214 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/963075
Comment on attachment 205214 [details] Patch Attachment 205214 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/884619
Created attachment 206940 [details] Patch
Comment on attachment 206940 [details] Patch Attachment 206940 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/1100328
Comment on attachment 206940 [details] Patch Attachment 206940 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/1090709
The API test is running and passing on GTK and EFL ports. I don't have a mac build system available, so could someone help me with this build problem and run the test?
Created attachment 218024 [details] Patch
Comment on attachment 218024 [details] Patch Attachment 218024 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/39228028
Comment on attachment 218024 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218024&action=review > Tools/TestWebKitAPI/PlatformEfl.cmake:57 > + ${TESTWEBKITAPI_DIR}/Tests/WebKit2/WebWorker_Bundle.cpp Wrong alphabetical order. > Tools/TestWebKitAPI/PlatformEfl.cmake:100 > + WebWorker Generally, lower case is placed below upper case. > Tools/TestWebKitAPI/Tests/WebKit2/WebWorker_Bundle.cpp:35 > +#include <cstdio> Add a new line. > Tools/TestWebKitAPI/Tests/WebKit2/WebWorker_Bundle.cpp:76 > + WebWorkerTest(const std::string& identifier) explicit ?
This patch need to be reviewed by WK2 owner. Probably, Sam or Benjamin.
Comment on attachment 218024 [details] Patch Attachment 218024 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/39258032
Comment on attachment 218024 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=218024&action=review Thanks for your comments and for have added others reviewers. I will fix these mistakes now. >> Tools/TestWebKitAPI/PlatformEfl.cmake:57 >> + ${TESTWEBKITAPI_DIR}/Tests/WebKit2/WebWorker_Bundle.cpp > > Wrong alphabetical order. When I run the webkit-check-style, the way it is, there's no alphabetical order error. In the other place I have these errors: Tools/TestWebKitAPI/PlatformEfl.cmake:58: Alphabetical sorting problem. "${TESTWEBKITAPI_DIR}/Tests/WebKit2/WebWorker_Bundle.cpp" should be before "${TESTWEBKITAPI_DIR}/Tests/WebKit2/CoordinatedGraphics/WKViewIsActiveSetIsActive_Bundle.cpp". [list/order] [5] Tools/TestWebKitAPI/PlatformEfl.cmake:59: Alphabetical sorting problem. "${TESTWEBKITAPI_DIR}/Tests/WebKit2/WebWorker_Bundle.cpp" should be before "${TESTWEBKITAPI_DIR}/Tests/WebKit2/efl/WKViewClientWebProcessCallbacks_Bundle.cpp". [list/order] [5] >> Tools/TestWebKitAPI/PlatformEfl.cmake:100 >> + WebWorker > > Generally, lower case is placed below upper case. Ok. >> Tools/TestWebKitAPI/Tests/WebKit2/WebWorker_Bundle.cpp:35 >> +#include <cstdio> > > Add a new line. Ok. > Tools/TestWebKitAPI/Tests/WebKit2/WebWorker_Bundle.cpp:36 > +using namespace std; This using is useless, I will remove it too. >> Tools/TestWebKitAPI/Tests/WebKit2/WebWorker_Bundle.cpp:76 >> + WebWorkerTest(const std::string& identifier) > > explicit ? Ok.
Created attachment 218029 [details] Patch
Comment on attachment 218029 [details] Patch Attachment 218029 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.appspot.com/results/39318061
Comment on attachment 218029 [details] Patch Attachment 218029 [details] did not pass mac-ews (mac): Output: http://webkit-queues.appspot.com/results/39368014
Comment on attachment 218029 [details] Patch I am not completely sure of the utility of this, so please add use cases. Some general concerns: - Use of global callback functions is a bad design. Please use a per-object client interface. - I am not clear on how this would work for SharedWorkers since the InjectedBundle will probably not run in shared worker processes (at least as currently designed). - The use of a boolean to indicate Dedicated/Shared should be avoided. Please use an enum to allow for more types of workers in the future. - The callbacks need more contextual information. For instance, in the non-worker versions of these, we pass a WebFrame, which includes things like the URL. - This should be modeled on the DOMWindowExtension concept, so that it can deal with being the page cache. - Perhaps Workers should have their own separate bundle they load. That would allow for a clear threading model and work for Shared workers.