Bug 78460

Summary: [WebSocket] Add owner Frame* as an argument to SocketStreamHandle::create, etc
Product: WebKit Reporter: Takashi Toyoshima <toyoshim>
Component: WebCore Misc.Assignee: Takashi Toyoshima <toyoshim>
Status: RESOLVED WONTFIX    
Severity: Normal CC: ap, bashi, rakuco, tkent, webkit.review.bot, yutak
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 78459    
Attachments:
Description Flags
Patch
none
to show overview to dependent change reviewers none

Takashi Toyoshima
Reported 2012-02-12 22:58:12 PST
Chromium port requires Frame* object to share information in the frame. This change is a port dependent part of the change for 78459.
Attachments
Patch (29.46 KB, patch)
2012-02-12 23:15 PST, Takashi Toyoshima 		no flags
DetailsFormatted DiffDiff
to show overview to dependent change reviewers (36.33 KB, patch)
2012-02-13 00:37 PST, Takashi Toyoshima 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Takashi Toyoshima
Comment 1 2012-02-12 23:15:53 PST
Created attachment 126722 [details] Patch
Kent Tamura
Comment 2 2012-02-13 00:03:49 PST
Comment on attachment 126722 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126722&action=review > Source/WebCore/ChangeLog:8 > + Add owner Frame* as an argument to SocketStreamHandle::create, etc > + https://bugs.webkit.org/show_bug.cgi?id=78460 > + > + Reviewed by NOBODY (OOPS!). > + > + No new tests because of just adding an unsed argument. Please write reasons why you add an unused argument. > Source/WebCore/websockets/WebSocketChannel.cpp:143 > + Document* document = static_cast<Document*>(m_context); Is this cast safe? Can't m_context be a worker?
Takashi Toyoshima
Comment 3 2012-02-13 00:37:23 PST
Created attachment 126730 [details] to show overview to dependent change reviewers
Takashi Toyoshima
Comment 4 2012-02-13 00:39:00 PST
Sorry, please just ignore previous attachment. I'll post it to another bug entry, but mistakenly post here.
Takashi Toyoshima
Comment 5 2012-02-13 01:22:06 PST
Comment on attachment 126722 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126722&action=review >> Source/WebCore/ChangeLog:8 >> + No new tests because of just adding an unsed argument. > > Please write reasons why you add an unused argument. OK, I added. >> Source/WebCore/websockets/WebSocketChannel.cpp:143 >> + Document* document = static_cast<Document*>(m_context); > > Is this cast safe? Can't m_context be a worker? Yes, currently m_context always has a Document. But, it could cause a security risk for future changes. I'll find a better way.
Alexey Proskuryakov
Comment 6 2012-02-13 01:26:43 PST
> Add owner Frame* as an argument to SocketStreamHandle::create, etc Please don't. This would be an extreme layering violation.
Takashi Toyoshima
Comment 7 2012-02-13 02:52:58 PST
Ah... I see. I should not make a dependency on WebCore in platform layer, right? I'll find another way.
Kent Tamura
Comment 8 2012-02-13 15:35:07 PST
(In reply to comment #7) > I should not make a dependency on WebCore in platform layer, right? Right. The code in platform/ must not depend on files in other directories.
Note You need to log in before you can comment on or make changes to this bug.