Bug 100165 - Add a strategy for shared workers
Summary: Add a strategy for shared workers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-10-23 15:29 PDT by Alexey Proskuryakov
Modified: 2012-10-24 10:29 PDT (History)
8 users (show)

See Also:

Attachments
proposed patch (36.81 KB, patch)
2012-10-23 15:36 PDT, Alexey Proskuryakov 		webkit-ews: commit-queue-
Details | Formatted Diff | Diff
with Qt copy-pasta fixed (36.81 KB, patch)
2012-10-23 15:57 PDT, Alexey Proskuryakov 		webkit-ews: commit-queue-
Details | Formatted Diff | Diff
more Qt fixing (36.81 KB, patch)
2012-10-23 17:20 PDT, Alexey Proskuryakov 		eflews.bot: commit-queue-
Details | Formatted Diff | Diff
with EFL build fix (38.04 KB, patch)
2012-10-24 00:03 PDT, Alexey Proskuryakov 		beidson: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2012-10-23 15:29:50 PDT 
Just an empty one for now. Will be used to create a worker process when appropriate.
Comment 1 Alexey Proskuryakov 2012-10-23 15:36:12 PDT 
Created attachment 170252 [details]
proposed patch

Since bug 100031 wasn't such a great idea, this is a lot of copy/pasted code.
Comment 2 Brady Eidson 2012-10-23 15:39:54 PDT 
A lot of the copy/paste into the WebKit1 ports could be avoided by adding a default strategy to WebCore.  This is something I currently have in my working tree for the strategy I'm considering adding soon... thoughts?
Comment 3 Early Warning System Bot 2012-10-23 15:45:22 PDT 
Comment on attachment 170252 [details]
proposed patch

Attachment 170252 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14525126
Comment 4 Alexey Proskuryakov 2012-10-23 15:56:43 PDT 
That would save some effort, but it would be quite a layering violation.

Having PlatformStrategies know the names of non-platform classes is one thing - we can think of it as a universal connector. But directly including them is certainly wrong.
Comment 5 Alexey Proskuryakov 2012-10-23 15:57:53 PDT 
Created attachment 170257 [details]
with Qt copy-pasta fixed
Comment 6 Brady Eidson 2012-10-23 16:02:54 PDT 
(In reply to comment #4)
> That would save some effort, but it would be quite a layering violation.
> 
> Having PlatformStrategies know the names of non-platform classes is one thing - we can think of it as a universal connector. But directly including them is certainly wrong.

I was thinking to *not* have individual strategies be in WebCore/Platform.

Imagine if PluginStrategy.h/cpp were in WebCore/Plugins, as an example.  All PlatformStrategies.h/cpp would do would provide access to the default PluginStrategy for those platforms that don't need different behavior...  that's what I was thinking.

Does that sound as bad?
Comment 7 Early Warning System Bot 2012-10-23 16:03:03 PDT 
Comment on attachment 170257 [details]
with Qt copy-pasta fixed

Attachment 170257 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14539042
Comment 8 EFL EWS Bot 2012-10-23 16:46:57 PDT 
Comment on attachment 170257 [details]
with Qt copy-pasta fixed

Attachment 170257 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14505630
Comment 9 Brady Eidson 2012-10-23 17:19:28 PDT 
We chatted in person.

I'll review after EWS has been pleased.
Comment 10 Alexey Proskuryakov 2012-10-23 17:20:32 PDT 
Created attachment 170274 [details]
more Qt fixing

Not sure how to fix EFL. Grepped everywhere trying to find how it finds PluginStrategy.h for comparison, but couldn't find that.
Comment 11 EFL EWS Bot 2012-10-23 19:02:52 PDT 
Comment on attachment 170274 [details]
more Qt fixing

Attachment 170274 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14543024
Comment 12 Alexey Proskuryakov 2012-10-23 22:25:42 PDT 
Could someone working on EFL please help with this build failure?
Comment 13 Brady Eidson 2012-10-23 22:38:33 PDT 
Does EFL have workers enabled?
Comment 14 Chris Dumez 2012-10-23 23:06:23 PDT 
(In reply to comment #13)
> Does EFL have workers enabled?

Yes, we do have worker enabled:
Source/cmake/OptionsEfl.cmake:WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_SHARED_WORKERS ON)
Source/cmake/OptionsEfl.cmake:WEBKIT_OPTION_DEFAULT_PORT_VALUE(ENABLE_WORKERS ON)
Comment 15 Chris Dumez 2012-10-23 23:08:19 PDT 
I'm looking into it.
Comment 16 Chris Dumez 2012-10-23 23:17:27 PDT 
This will fix the EFL build:

diff --git a/Source/WebKit/CMakeLists.txt b/Source/WebKit/CMakeLists.txt
index a7983d9..5d0e361 100644
--- a/Source/WebKit/CMakeLists.txt
+++ b/Source/WebKit/CMakeLists.txt
@@ -43,6 +43,7 @@ SET(WebKit_INCLUDE_DIRECTORIES
     "${WEBCORE_DIR}/svg"
     "${WEBCORE_DIR}/svg/graphics"
     "${WEBCORE_DIR}/svg/properties"
+    "${WEBCORE_DIR}/workers"
     "${JAVASCRIPTCORE_DIR}"
     "${JAVASCRIPTCORE_DIR}/ForwardingHeaders"
     "${JAVASCRIPTCORE_DIR}/API"
Comment 17 Alexey Proskuryakov 2012-10-24 00:03:41 PDT 
Created attachment 170331 [details]
with EFL build fix

Thank you so much!

Build fix included, let's see how happy EWS is.
Comment 18 Brady Eidson 2012-10-24 00:19:10 PDT 
Comment on attachment 170331 [details]
with EFL build fix

EFL passed, yay!
Comment 19 WebKit Review Bot 2012-10-24 08:52:52 PDT 
Comment on attachment 170331 [details]
with EFL build fix

Rejecting attachment 170331 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 12787 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
50>At revision 12787.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/14569231
Comment 20 Alexey Proskuryakov 2012-10-24 10:29:23 PDT 
Committed manually: http://trac.webkit.org/changeset/132367