RESOLVED WONTFIX 78460
[WebSocket] Add owner Frame* as an argument to SocketStreamHandle::create, etc
https://bugs.webkit.org/show_bug.cgi?id=78460
Summary [WebSocket] Add owner Frame* as an argument to SocketStreamHandle::create, etc
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
to show overview to dependent change reviewers (36.33 KB, patch)
2012-02-13 00:37 PST, Takashi Toyoshima
no flags
Takashi Toyoshima
Comment 1 2012-02-12 23:15:53 PST
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.