Bug 29757 - Chromium wants to turn off SharedWorkers at runtime
Summary: Chromium wants to turn off SharedWorkers at runtime
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore JavaScript (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Andrew Wilson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-25 17:38 PDT by Andrew Wilson
Modified: 2009-10-11 17:13 PDT (History)
2 users (show)

See Also:


Attachments
proposed patch (3.32 KB, patch)
2009-09-25 18:03 PDT, Andrew Wilson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Wilson 2009-09-25 17:38:32 PDT
We need to add SharedWorkerRepository::isAvailable(), and have window.SharedWorker === undefined when isAvailable returns false.
Comment 1 Andrew Wilson 2009-09-25 18:03:11 PDT
Created attachment 40156 [details]
proposed patch

V8 does not support custom getters for constructors yet, so this just turns on this functionality for JSC.

V8 functionality will be added once https://bugs.webkit.org/show_bug.cgi?id=29758 is addressed
Comment 2 Eric Seidel (no email) 2009-09-28 17:19:10 PDT
Comment on attachment 40156 [details]
proposed patch

Where is the code to make it return false?
         // Returns true if the platform supports SharedWorkers, otherwise false.
 52         static bool isAvailable();
Comment 3 Andrew Wilson 2009-09-28 17:22:55 PDT
That will be in the platform specific implementation. Chromium will provide its own implementation of SharedWorkerRepository (it will not compile the default WebKit implementation in DefaultSharedWorkerRepository).
Comment 4 Eric Seidel (no email) 2009-10-02 12:30:06 PDT
Comment on attachment 40156 [details]
proposed patch

Why wouldn't this be held off of Settings?
Comment 5 Andrew Wilson 2009-10-02 12:42:24 PDT
(In reply to comment #4)
> (From update of attachment 40156 [details])
> Why wouldn't this be held off of Settings?

Because it's not intended to be a user setting, and it doesn't make sense to pollute all platforms with extra settings when there's already a clear platform-specific interface to hang this off of.

I'm basically doing the same thing that MediaPlayer::isAvailable() does, so there's ample precedent for putting it here.
Comment 6 David Levin 2009-10-05 08:38:22 PDT
Comment on attachment 40156 [details]
proposed patch

Looks good. Sorry it took me so long to look at this.
Comment 7 WebKit Commit Bot 2009-10-05 16:34:38 PDT
Comment on attachment 40156 [details]
proposed patch

Clearing flags on attachment: 40156

Committed r49130: <http://trac.webkit.org/changeset/49130>
Comment 8 WebKit Commit Bot 2009-10-05 16:34:41 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Sam Weinig 2009-10-05 16:46:23 PDT
This is wrong.  "SharedWorker" in window will still return true, and that is the way we encourage web developers to due feature detection.
Comment 10 Andrew Wilson 2009-10-11 17:11:42 PDT
(In reply to comment #9)
> This is wrong.  "SharedWorker" in window will still return true, and that is
> the way we encourage web developers to due feature detection.

I've logged two new bugs to track the issues Sam is concerned about, so I'm closing this bug.

https://bugs.webkit.org/show_bug.cgi?id=30240
https://bugs.webkit.org/show_bug.cgi?id=30289