[webkit-reviews] review granted: [Bug 74457] Change adoptPtr(new ...) to ...::create in Page.cpp : [Attachment 119098] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Greg Billock <gbillock at google.com>'s
request for review:
Bug 74457: Change adoptPtr(new ...) to ...::create in Page.cpp
https://bugs.webkit.org/show_bug.cgi?id=74457

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

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=119098&action=review


This looks fine, but does not go far enough. In most of these cases, the
constructors of these classes should be made private at the same time.

I’d rather see a patch that does fewer classes at a time, but makes the
constructors private for those classes.

> Source/WebCore/ChangeLog:8
> +	   No new tests. (OOPS!)

Can’t land a patch with that line in it.

> Source/WebCore/loader/ProgressTracker.h:31
> +#include <wtf/PassOwnPtr.h>

If we just have a return type of PassOwnPtr, I think we can do with just
<wtf/Forward.h>.

> Source/WebCore/notifications/NotificationController.h:32
> +#include <wtf/PassOwnPtr.h>

Ditto.

> Source/WebCore/page/SpeechInput.h:39
> +#include <wtf/PassOwnPtr.h>

I’m surprised this include is needed. Doesn’t Forward.h forward-declare
PassOwnPtr?