Bug 73336 - Get rid of AllowCrossThreadAccess throughout ThreadableWebSocketChannelClientWrapper
Summary: Get rid of AllowCrossThreadAccess throughout ThreadableWebSocketChannelClient...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 73290
Blocks: 67942
  Show dependency tree / graph
 
Reported: 2011-11-29 10:54 PST by Takashi Toyoshima
Modified: 2011-11-30 18:33 PST (History)
7 users (show)

See Also:

Attachments
Patch (10.33 KB, patch)
2011-11-29 11:50 PST, Takashi Toyoshima 		no flags Details | Formatted Diff | Diff
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 Takashi Toyoshima 2011-11-29 10:54:25 PST 
We shouldn't use "AllowCrossThreadAccess" in ThreadableWebSocketChannelClientWrapper.

Here's why:
1. It was added to highlight the places where we were not doing anything to help with lifetime of the object being passed.
2. At the time it was added, ThreadSafeRefCounted* objects passed through createCallbackTask did not have the ref count increased, so AllowCrossThreadAccess needed to be added here.
3. ThreadableWebSocketChannelClientWrapper is designed to be ref counted across threads -- largely to make sure that its lifetime is preserved by this call.

This issue is derived from a review in https://bugs.webkit.org/show_bug.cgi?id=73290
Comment 1 Takashi Toyoshima 2011-11-29 11:50:56 PST 
Created attachment 117013 [details]
Patch
Comment 2 David Levin 2011-11-29 11:52:36 PST 
Comment on attachment 117013 [details]
Patch

Thanks!
Comment 3 Takashi Toyoshima 2011-11-29 11:57:24 PST 
Thank YOU, too for good advice :-)
Comment 4 WebKit Review Bot 2011-11-30 18:33:53 PST 
Comment on attachment 117013 [details]
Patch

Clearing flags on attachment: 117013

Committed r101583: <http://trac.webkit.org/changeset/101583>
Comment 5 WebKit Review Bot 2011-11-30 18:33:58 PST 
All reviewed patches have been landed.  Closing bug.