[webkit-reviews] review granted: [Bug 108394] Begin to make XSSAuditor thread aware : [Attachment 185830] Patch

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has granted Tony Gentilcore
<tonyg at chromium.org>'s request for review:
Bug 108394: Begin to make XSSAuditor thread aware
https://bugs.webkit.org/show_bug.cgi?id=108394

Attachment 185830: Patch
https://bugs.webkit.org/attachment.cgi?id=185830&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=185830&action=review


> Source/WebCore/html/parser/HTMLDocumentParser.cpp:371
> +		   m_xssAuditorDelegate.didBlockScript(*request);	     

Maybe request.release() ?  I can imagine the delegate wanting to take ownership
of the request.

> Source/WebCore/html/parser/XSSAuditor.cpp:275
> +    OwnPtr<DidBlockScriptRequest> request;

Another minor style comment: you can probably skip this local variable
entirely.

> Source/WebCore/html/parser/XSSAuditor.cpp:282
> -	   return;
> +	   return request.release();

Here you can "return nullptr;"

> Source/WebCore/html/parser/XSSAuditor.cpp:-319
> -

Here you can return request.release();

> Source/WebCore/html/parser/XSSAuditor.cpp:303
> +    return request.release();

Here again you can return nullptr;

> Source/WebCore/html/parser/XSSAuditorDelegate.cpp:43
> +    , m_notifyClient(true)

nit: I probably would have flipped this around as m_didNotifyClient, but that's
a super minor issue.