WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
112179
Pass the XSSAuditor's report URL to the XSSAuditorDelegate on the main thread.
https://bugs.webkit.org/show_bug.cgi?id=112179
Summary
Pass the XSSAuditor's report URL to the XSSAuditorDelegate on the main thread.
Mike West
Reported
2013-03-12 12:50:53 PDT
Pass the XSSAuditor's report URL to the XSSAuditorDelegate on the main thread.
Attachments
Patch
(5.46 KB, patch)
2013-03-12 12:55 PDT
,
Mike West
no flags
Details
Formatted Diff
Diff
Patch
(7.13 KB, patch)
2013-03-12 14:06 PDT
,
Mike West
no flags
Details
Formatted Diff
Diff
Patch
(11.84 KB, patch)
2013-03-12 14:53 PDT
,
Mike West
no flags
Details
Formatted Diff
Diff
Patch for landing
(11.80 KB, patch)
2013-03-13 01:43 PDT
,
Mike West
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Mike West
Comment 1
2013-03-12 12:55:50 PDT
Created
attachment 192790
[details]
Patch
Mike West
Comment 2
2013-03-12 12:56:51 PDT
Forking this from
https://bugs.webkit.org/show_bug.cgi?id
= for clarity.
Adam Barth
Comment 3
2013-03-12 13:27:44 PDT
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&
Mike West
Comment 4
2013-03-12 13:40:10 PDT
(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. :)
Mike West
Comment 5
2013-03-12 14:06:13 PDT
Created
attachment 192801
[details]
Patch
Adam Barth
Comment 6
2013-03-12 14:33:40 PDT
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.
Mike West
Comment 7
2013-03-12 14:36:14 PDT
(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.
Mike West
Comment 8
2013-03-12 14:53:54 PDT
Created
attachment 192814
[details]
Patch
Mike West
Comment 9
2013-03-12 14:55:28 PDT
(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. :)
Adam Barth
Comment 10
2013-03-12 15:58:26 PDT
Comment on
attachment 192814
[details]
Patch This looks great. Thanks for iterating on it.
WebKit Review Bot
Comment 11
2013-03-12 16:21:42 PDT
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
Adam Barth
Comment 12
2013-03-12 16:29:38 PDT
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.
Mike West
Comment 13
2013-03-13 01:43:38 PDT
Created
attachment 192881
[details]
Patch for landing
WebKit Review Bot
Comment 14
2013-03-13 02:34:38 PDT
Comment on
attachment 192881
[details]
Patch for landing Clearing flags on attachment: 192881 Committed
r145695
: <
http://trac.webkit.org/changeset/145695
>
WebKit Review Bot
Comment 15
2013-03-13 02:34:42 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug