| Summary: | Add webSocketEnabled flag in Settings | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Fumitoshi Ukai <ukai> | ||||||
| Component: | WebCore Misc. | Assignee: | Nobody <webkit-unassigned> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ap, brettw, eric, sam | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
|
Description
Fumitoshi Ukai
2009-09-03 02:41:26 PDT
Created attachment 38977 [details]
add webSocketEnabled in Settings
---
3 files changed, 23 insertions(+), 0 deletions(-)
Comment on attachment 38977 [details]
add webSocketEnabled in Settings
Why? This doesn't do anything yet? Are there other web sockets code paths you're about to add? Generally we don't add dead code (unless it's about to be used...), which seems to be exactly what this patch is doing.
(In reply to comment #2) > (From update of attachment 38977 [details]) > Why? This doesn't do anything yet? Are there other web sockets code paths > you're about to add? Generally we don't add dead code (unless it's about to be > used...), which seems to be exactly what this patch is doing. I want this code to support --enable-web-sockets switch (in chromium). Will add hooks once WebSocket API code (bug 28038) is landed. Why is it necessary for this to be a runtime setting rather than a compile-time choice? I expect his response will be that "chromium makes all new features command-line switches, so they need to be run-time switchable." Chromium is constantly shipping from tip of tree (the developer branch) so in-progress features like this default to off, but can be enabled by passing command-line switches. Eric is correct. We like to ship off trunk without worrying about half-completed features. On the dev channel, we can make a feature available for testing by interested devlopers, and still eventually push that build to stable without major changes. It would be nice to mention this policy on webkit-dev, along with some guidelines on when these runtime switches can be removed. Great minds think alike: http://groups.google.com/group/chromium-dev/browse_thread/thread/be626eabfdf0ca48 Comment on attachment 38977 [details]
add webSocketEnabled in Settings
My understanding of the new policy is that this should have "experimental" in the name.
Created attachment 40226 [details]
rename webSocketsEnabled to experimentalWebSocketEnabled
Comment on attachment 40226 [details]
rename webSocketsEnabled to experimentalWebSocketEnabled
I dont' think the explicit resetDefaultsToConsistentValues reset calls are really necessary, but they're certainly not a bad idea either.
LGTM. If you want this auto-committed, you'll need to set cq? as well. I can't remember if you're a committer yet or not.
Comment on attachment 40226 [details] rename webSocketsEnabled to experimentalWebSocketEnabled Rejecting patch 40226 from commit-queue. ukai@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/modules/committers.py. (In reply to comment #12) > (From update of attachment 40226 [details]) > Rejecting patch 40226 from commit-queue. > > ukai@chromium.org does not have committer permissions according to > http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/modules/committers.py. Hmm, I was too early to set commit-queue+? Comment on attachment 40226 [details] rename webSocketsEnabled to experimentalWebSocketEnabled Rejecting patch 40226 from commit-queue. ukai@chromium.org does not have committer permissions according to http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/modules/committers.py. Clearly you did not use bugzilla-tool to do the commit, or run the layout tests, since it looks like you broke all platforms: http://build.webkit.org/changes/1106 http://build.webkit.org/waterfall :( Assuming I'm reading the buildbot output correctly. I'm going to roll out this patch. (In reply to comment #13) > (In reply to comment #12) > > (From update of attachment 40226 [details] [details]) > > Rejecting patch 40226 from commit-queue. > > > > ukai@chromium.org does not have committer permissions according to > > http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/modules/committers.py. > > Hmm, I was too early to set commit-queue+? No, you just haven't added yourself to committers.py yet. You can't set commit-queue+ until you do. I'm glad to see that you have commit bit now! But please get another committer to walk you through your next commit, since there were several process errors here. Most notably, you did not watch the bots after committing. You also did not update the bug with the committed revision (yes, we need to document that somewhere). And you did not close the bug after committing. bugzilla-tool land-diff can do all of these things for you, although you're also welcome to perform the actions manually. (In reply to comment #16) > (In reply to comment #13) > > (In reply to comment #12) > > > (From update of attachment 40226 [details] [details] [details]) > > > Rejecting patch 40226 from commit-queue. > > > > > > ukai@chromium.org does not have committer permissions according to > > > http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/modules/committers.py. > > > > Hmm, I was too early to set commit-queue+? > > No, you just haven't added yourself to committers.py yet. You can't set > commit-queue+ until you do. I believe I added myself to committers.py (r48852) before I set commit-queue+... (In reply to comment #15) > Clearly you did not use bugzilla-tool to do the commit, or run the layout > tests, since it looks like you broke all platforms: > http://build.webkit.org/changes/1106 > http://build.webkit.org/waterfall > > :( > > Assuming I'm reading the buildbot output correctly. > > I'm going to roll out this patch. Sorry, I've prepared a fix in http://bugs.webkit.org/show_bug.cgi?id=29840. Do you roll back this? If so, I'll merge the fix here. (In reply to comment #18) > I believe I added myself to committers.py (r48852) before I set > commit-queue+... Sorry, that's my fault. I was running the commit-queue in a way such that it was not auto-updating committers.py correctly. I've fixed that now. My apologies for my aggressive tone above. It was undeserved. Thank you for so quick a response! (In reply to comment #21) > My apologies for my aggressive tone above. It was undeserved. Thank you for > so quick a response! No problem. I should be more carefully before I committed. Thanks for your help! Landed as r48861 I am confused why this flag is called experimentalWebSocketsEnabled (I see in one comment it was stated this is a new rule. Where was this rule established). Do we plan on changing the setting once the implementation is done? Does this work for things like "WebSocket" in window? window.hasOwnProperty("WebSocket")?
(In reply to comment #24) > I am confused why this flag is called experimentalWebSocketsEnabled (I see in > one comment it was stated this is a new rule. Where was this rule > established). In this thread? https://lists.webkit.org/pipermail/webkit-dev/2009-September/009881.html > Do we plan on changing the setting once the implementation is > done? Does this work for things like "WebSocket" in window? > window.hasOwnProperty("WebSocket")? I assumed a web developer would check with 'typeof window.WebSocket == "undefined"', as described in https://lists.webkit.org/pipermail/webkit-dev/2009-September/010000.html |