[webkit-reviews] review denied: [Bug 26126] Refactor methods of WorkerMessagingProxy used to talk to main-thread loader into new interface. : [Attachment 30850] Proposed patch

[bugzilla-daemon at webkit.org]

David Levin <levin at chromium.org> has denied Dmitry Titov
<dimich at chromium.org>'s request for review:
Bug 26126: Refactor methods of WorkerMessagingProxy used to talk to main-thread
loader into new interface.
https://bugs.webkit.org/show_bug.cgi?id=26126

Attachment 30850: Proposed patch
https://bugs.webkit.org/attachment.cgi?id=30850&action=review

------- Additional Comments from David Levin <levin at chromium.org>
A few issues to fix up, so r- for this version of the patch.

> diff --git a/WebCore/WebCore.xcodeproj/project.pbxproj
b/WebCore/WebCore.xcodeproj/project.pbxproj
> index a2742ac..2996d35 100644
> --- a/WebCore/WebCore.xcodeproj/project.pbxproj
> +++ b/WebCore/WebCore.xcodeproj/project.pbxproj
> @@ -208,6 +208,7 @@
>		188604B30F2E654A000B6443 /* DOMTimer.cpp in Sources */ = {isa =
PBXBuildFile; fileRef = 188604B10F2E654A000B6443 /* DOMTimer.cpp */; };
>		188604B40F2E654A000B6443 /* DOMTimer.h in Headers */ = {isa =
PBXBuildFile; fileRef = 188604B20F2E654A000B6443 /* DOMTimer.h */; };
> +		18F831B80FD48C7800D8C56B /* WorkerLoaderProxy.h in Headers */ =
{isa = PBXBuildFile; fileRef = 18F831B70FD48C7800D8C56B /* WorkerLoaderProxy.h
*/; };

Is this out of order?


> diff --git a/WebCore/loader/WorkerThreadableLoader.cpp
b/WebCore/loader/WorkerThreadableLoader.cpp
>  

Is it possible to remove the #include "WorkerMessagingProxy.h"?


>
-WorkerThreadableLoader::MainThreadBridge::MainThreadBridge(PassRefPtr<Threadab
leLoaderClientWrapper> workerClientWrapper, WorkerMessagingProxy&
messagingProxy, const String& taskMode,
>
+WorkerThreadableLoader::MainThreadBridge::MainThreadBridge(PassRefPtr<Threadab
leLoaderClientWrapper> workerClientWrapper, WorkerLoaderProxy* loaderProxy,
const String& taskMode,

Ideally loaderProxy would be a & since it can never be 0.



> diff --git a/WebCore/workers/WorkerLoaderProxy.h
b/WebCore/workers/WorkerLoaderProxy.h
> +
> +	   virtual ~WorkerLoaderProxy() {}
Nice to have a space in { }.

> +	   // Postst callbacks from loading code to the WorkerContext. The
'mode' is used to differentiate

Postst spelling.