Pass the XSSAuditor's report URL to the XSSAuditorDelegate on the main thread.
Created attachment 192790 [details] Patch
Forking this from https://bugs.webkit.org/show_bug.cgi?id= for clarity.
Comment on attachment 192790 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192790&action=review > Source/WebCore/html/parser/HTMLDocumentParser.cpp:538 > m_xssAuditor.init(document()); > + m_xssAuditorDelegate.setReportURL(m_xssAuditor.reportURL()); This will cause us to set the report URL every time HTMLDocumentParser::pumpTokenizer is called. It seems like we only need to do that once... Maybe we should pass the m_xssAuditorDelegate to m_xssAuditor in init and have it store the report URL only in the delegate? > Source/WebCore/html/parser/XSSAuditor.h:63 > + KURL reportURL() const { return m_reportURL; } KURL -> const KURL&
(In reply to comment #3) > (From update of attachment 192790 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192790&action=review > > > Source/WebCore/html/parser/HTMLDocumentParser.cpp:538 > > m_xssAuditor.init(document()); > > + m_xssAuditorDelegate.setReportURL(m_xssAuditor.reportURL()); > > This will cause us to set the report URL every time HTMLDocumentParser::pumpTokenizer is called. It seems like we only need to do that once... Maybe we should pass the m_xssAuditorDelegate to m_xssAuditor in init and have it store the report URL only in the delegate? I didn't like this idea until I actually read your comment. :) Throwing away the property on both XSSAuditor and XSSInfo would be great. > > > Source/WebCore/html/parser/XSSAuditor.h:63 > > + KURL reportURL() const { return m_reportURL; } > > KURL -> const KURL& *ahem* We can throw this away too. :)
Created attachment 192801 [details] Patch
Comment on attachment 192801 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192801&action=review > Source/WebCore/html/parser/XSSAuditor.cpp:302 > m_reportURL = xssProtectionReportURL; // FIXME: Combine the two report URLs in some reasonable way. You can delete m_reportURL now. It's not needed.
(In reply to comment #6) > (From update of attachment 192801 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=192801&action=review > > > Source/WebCore/html/parser/XSSAuditor.cpp:302 > > m_reportURL = xssProtectionReportURL; // FIXME: Combine the two report URLs in some reasonable way. > > You can delete m_reportURL now. It's not needed. I was planning on doing that in a separate patch, along with the XSSInfo::m_reportURL, as I figured it would make this patch easier to review. I'm happy to do it all in one if you'd prefer.
Created attachment 192814 [details] Patch
(In reply to comment #8) > Created an attachment (id=192814) [details] > Patch This wraps up everything except the one-report-only change; I'd still like to do that in the other bug, as I'd also like to reorder some of the statements in that block, and the diff will be ugly if I do both here. :)
Comment on attachment 192814 [details] Patch This looks great. Thanks for iterating on it.
Comment on attachment 192814 [details] Patch Rejecting attachment 192814 [details] from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-02', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 192814, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 13882 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 54>At revision 13882. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://webkit-commit-queue.appspot.com/results/17189262
Comment on attachment 192814 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=192814&action=review > Source/WebCore/ChangeLog:16 > + No new tests (OOPS!). The following ChangeLog files contain OOPS: trunk/Source/WebCore/ChangeLog Please don't ever say "OOPS" in a ChangeLog file.
Created attachment 192881 [details] Patch for landing
Comment on attachment 192881 [details] Patch for landing Clearing flags on attachment: 192881 Committed r145695: <http://trac.webkit.org/changeset/145695>
All reviewed patches have been landed. Closing bug.