Bug 154966 – Remove PassRefPtr from ThreadableLoader and relatives

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[youenn fablet]

We should remove PassRefPtr from ThreadableLoader and relatives.

[youenn fablet]

Created attachment 272756 [details]
Patch

[WebKit Commit Bot]

Attachment 272756 [details] did not pass style-queue:


ERROR: Source/WebCore/loader/WorkerThreadableLoader.cpp:89:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
ERROR: Source/WebCore/loader/WorkerThreadableLoader.cpp:89:  Wrong number of spaces before statement. (expected: 8)  [whitespace/indent] [4]
Total errors found: 2 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Darin Adler]

Comment on attachment 272756 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=272756&action=review

> Source/WebCore/loader/DocumentThreadableLoader.cpp:79
>      return DocumentThreadableLoader::create(document, client, request, options, nullptr);

Should just be "return create"; no need to repeat the class name.

> Source/WebCore/loader/ThreadableLoader.cpp:75
>      ASSERT(client);
>      ASSERT(context);

Note these two heavy clues that the context and client arguments should be references, not pointers.

> Source/WebCore/loader/WorkerThreadableLoader.cpp:55
> +    , m_bridge(*(new MainThreadBridge(*m_workerClientWrapper, workerGlobalScope->thread().workerLoaderProxy(), taskMode, request, options, workerGlobalScope->url().strippedForUseAsReferrer(), workerGlobalScope->securityOrigin(), workerGlobalScope->contentSecurityPolicy())))

Extra parentheses here around the call to "new" are unneeded.

[youenn fablet]

Created attachment 272836 [details]
Patch for landing

[youenn fablet]

Thanks for the review.

(In reply to comment #3)
> Comment on attachment 272756 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=272756&action=review
> 
> > Source/WebCore/loader/DocumentThreadableLoader.cpp:79
> >      return DocumentThreadableLoader::create(document, client, request, options, nullptr);
> 
> Should just be "return create"; no need to repeat the class name.

Fixed.

> > Source/WebCore/loader/ThreadableLoader.cpp:75
> >      ASSERT(client);
> >      ASSERT(context);
> 
> Note these two heavy clues that the context and client arguments should be
> references, not pointers.


Yes, code could be improved with that respect.
There might also be some RefPtr->Ref changes.

> > Source/WebCore/loader/WorkerThreadableLoader.cpp:55
> > +    , m_bridge(*(new MainThreadBridge(*m_workerClientWrapper, workerGlobalScope->thread().workerLoaderProxy(), taskMode, request, options, workerGlobalScope->url().strippedForUseAsReferrer(), workerGlobalScope->securityOrigin(), workerGlobalScope->contentSecurityPolicy())))
> 
> Extra parentheses here around the call to "new" are unneeded.

Fixed.

[WebKit Commit Bot]

Comment on attachment 272836 [details]
Patch for landing

Clearing flags on attachment: 272836

Committed r197551: <http://trac.webkit.org/changeset/197551>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.