Call XSSAuditor's didBlockScript() for the threaded HTML parser
Created attachment 186189 [details] Patch
Comment on attachment 186189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186189&action=review > Source/WebCore/html/parser/XSSAuditorDelegate.cpp:44 > + return isStringSafeToSendToAnotherThread(m_reportURL.string()) I'm skeptical this is going to work but am not sure what to do here. Is it going to be safe to send KURL across threads? How do we ASSERT that?
Comment on attachment 186189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186189&action=review > Source/WebCore/html/parser/CompactHTMLToken.cpp:98 > + *m_didBlockScriptRequest = *other.didBlockScriptRequest(); This doesn't seem right. OwnPtr isn't copyable. We need to implement some sort of fancier VectorTraits to teach Vector how to move the CompactHTMLToken. There should be examples already in the codebase of how to do this. > Source/WebCore/html/parser/CompactHTMLToken.h:80 > + DidBlockScriptRequest* didBlockScriptRequest() const; The "did" in this function name is very strange. Can we rename DidBlockScriptRequest to XSSInfo ? >> Source/WebCore/html/parser/XSSAuditorDelegate.cpp:44 >> + return isStringSafeToSendToAnotherThread(m_reportURL.string()) > > I'm skeptical this is going to work but am not sure what to do here. Is it going to be safe to send KURL across threads? How do we ASSERT that? Maybe we should add a isSafeToSendToAnotherThread function to KURL?
Created attachment 186454 [details] Patch
Comment on attachment 186189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186189&action=review >> Source/WebCore/html/parser/CompactHTMLToken.cpp:98 >> + *m_didBlockScriptRequest = *other.didBlockScriptRequest(); > > This doesn't seem right. OwnPtr isn't copyable. We need to implement some sort of fancier VectorTraits to teach Vector how to move the CompactHTMLToken. There should be examples already in the codebase of how to do this. From RenderGeometryMap.h, it looks like SimpleClassVectorTraits does the trick. >> Source/WebCore/html/parser/CompactHTMLToken.h:80 >> + DidBlockScriptRequest* didBlockScriptRequest() const; > > The "did" in this function name is very strange. Can we rename DidBlockScriptRequest to XSSInfo ? Done >>> Source/WebCore/html/parser/XSSAuditorDelegate.cpp:44 >>> + return isStringSafeToSendToAnotherThread(m_reportURL.string()) >> >> I'm skeptical this is going to work but am not sure what to do here. Is it going to be safe to send KURL across threads? How do we ASSERT that? > > Maybe we should add a isSafeToSendToAnotherThread function to KURL? Done
Comment on attachment 186454 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=186454&action=review > Source/WebCore/html/parser/CompactHTMLToken.h:102 > +// This is required for a struct with OwnPtr. We know CompactHTMLToken is simple enough that > +// initializing to 0 and moving with memcpy (and then not destructing the original) will work. > +template<> struct VectorTraits<WebCore::CompactHTMLToken> : SimpleClassVectorTraits { }; If you have this declaration, do you need the CompactHTMLToken copy constructor? If not, we should remove it.
(In reply to comment #6) > (From update of attachment 186454 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=186454&action=review > > > Source/WebCore/html/parser/CompactHTMLToken.h:102 > > +// This is required for a struct with OwnPtr. We know CompactHTMLToken is simple enough that > > +// initializing to 0 and moving with memcpy (and then not destructing the original) will work. > > +template<> struct VectorTraits<WebCore::CompactHTMLToken> : SimpleClassVectorTraits { }; > > If you have this declaration, do you need the CompactHTMLToken copy constructor? If not, we should remove it. Yes. It doesn't compile without the copy ctor. Even with this declaration we still need a way to teach it to copy the structure that the OwnPtr points to, which is where the copy ctor comes in.
Created attachment 186653 [details] Patch for landing
Comment on attachment 186653 [details] Patch for landing Clearing flags on attachment: 186653 Committed r141905: <http://trac.webkit.org/changeset/141905>
All reviewed patches have been landed. Closing bug.
(In reply to comment #10) > All reviewed patches have been landed. Closing bug. This is causing build breakage: http://build.webkit.org/builders/Apple%20Lion%20Debug%20%28Build%29/builds/11613/steps/compile-webkit/logs/stdio Undefined symbols for architecture x86_64: "__ZNK3WTF6String27isSafeToSendToAnotherThreadEv", referenced from: __ZNK7WebCore4KURL27isSafeToSendToAnotherThreadEv in KURL.o __ZNK7WebCore7XSSInfo27isSafeToSendToAnotherThreadEv in XSSAuditorDelegate.o Please fix this soon or we will need to roll it out.
(In reply to comment #11) > (In reply to comment #10) > > All reviewed patches have been landed. Closing bug. > > This is causing build breakage: > > http://build.webkit.org/builders/Apple%20Lion%20Debug%20%28Build%29/builds/11613/steps/compile-webkit/logs/stdio > > Undefined symbols for architecture x86_64: > "__ZNK3WTF6String27isSafeToSendToAnotherThreadEv", referenced from: > __ZNK7WebCore4KURL27isSafeToSendToAnotherThreadEv in KURL.o > __ZNK7WebCore7XSSInfo27isSafeToSendToAnotherThreadEv in XSSAuditorDelegate.o > > Please fix this soon or we will need to roll it out. I'm on it.
Re-opened since this is blocked by bug 108963
Created attachment 186738 [details] Patch
Comment on attachment 186738 [details] Patch Clearing flags on attachment: 186738 Committed r142004: <http://trac.webkit.org/changeset/142004>