Support file utilities and mime-related methods on workers.

Support file utilities and mime-related methods on workers.

They are necessary to support File::create and File.size() on workers.

BUG=none
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=62692

[ericu, 2010-10-11 17:43:18]

http://codereview.chromium.org/3660003/diff/7001/8005
File chrome/browser/worker_host/worker_process_host.cc (right):

http://codereview.chromium.org/3660003/diff/7001/8005#newcode513
chrome/browser/worker_host/worker_process_host.cc:513: void
WorkerProcessHost::OnGetMimeTypeFromFile(
This seems harmless, but should we do any access checks before giving out
information about a file?

http://codereview.chromium.org/3660003/diff/7001/8008
File chrome/worker/worker_webkitclient_impl.cc (right):

http://codereview.chromium.org/3660003/diff/7001/8008#newcode104
chrome/worker/worker_webkitclient_impl.cc:104: if (!file_utilities_.get()) {
The renderer creates this in its constructor, the worker lazily.  How about
making them both lazy?

http://codereview.chromium.org/3660003/diff/7001/8009
File chrome/worker/worker_webkitclient_impl.h (right):

http://codereview.chromium.org/3660003/diff/7001/8009#newcode11
chrome/worker/worker_webkitclient_impl.h:11: #include
"webkit/glue/webfileutilities_impl.h"
Do we still need to include webfileutilities_impl, or is the forward declaration
enough?

[kinuko, 2010-10-11 18:43:58]

http://codereview.chromium.org/3660003/diff/7001/8005
File chrome/browser/worker_host/worker_process_host.cc (right):

http://codereview.chromium.org/3660003/diff/7001/8005#newcode513
chrome/browser/worker_host/worker_process_host.cc:513: void
WorkerProcessHost::OnGetMimeTypeFromFile(
On 2010/10/11 17:43:18, ericu wrote:
> This seems harmless, but should we do any access checks before giving out
> information about a file?

The renderer-side host code in resource_message_filter.cc doesn't have any
checks either.  (Adding darin who's the original author in the file to make
sure)

http://codereview.chromium.org/3660003/diff/7001/8008
File chrome/worker/worker_webkitclient_impl.cc (right):

http://codereview.chromium.org/3660003/diff/7001/8008#newcode104
chrome/worker/worker_webkitclient_impl.cc:104: if (!file_utilities_.get()) {
On 2010/10/11 17:43:18, ericu wrote:
> The renderer creates this in its constructor, the worker lazily.  How about
> making them both lazy?

Done.

http://codereview.chromium.org/3660003/diff/7001/8009
File chrome/worker/worker_webkitclient_impl.h (right):

http://codereview.chromium.org/3660003/diff/7001/8009#newcode11
chrome/worker/worker_webkitclient_impl.h:11: #include
"webkit/glue/webfileutilities_impl.h"
On 2010/10/11 17:43:18, ericu wrote:
> Do we still need to include webfileutilities_impl, or is the forward
declaration
> enough?

Changed to the forward declaration.

[ericu, 2010-10-12 00:13:15]

On 2010/10/11 18:43:58, Kinuko Yasuda wrote:
> http://codereview.chromium.org/3660003/diff/7001/8005
> File chrome/browser/worker_host/worker_process_host.cc (right):
> 
> http://codereview.chromium.org/3660003/diff/7001/8005#newcode513
> chrome/browser/worker_host/worker_process_host.cc:513: void
> WorkerProcessHost::OnGetMimeTypeFromFile(
> On 2010/10/11 17:43:18, ericu wrote:
> > This seems harmless, but should we do any access checks before giving out
> > information about a file?
> 
> The renderer-side host code in resource_message_filter.cc doesn't have any
> checks either.  (Adding darin who's the original author in the file to make
> sure)
> 
> http://codereview.chromium.org/3660003/diff/7001/8008
> File chrome/worker/worker_webkitclient_impl.cc (right):
> 
> http://codereview.chromium.org/3660003/diff/7001/8008#newcode104
> chrome/worker/worker_webkitclient_impl.cc:104: if (!file_utilities_.get()) {
> On 2010/10/11 17:43:18, ericu wrote:
> > The renderer creates this in its constructor, the worker lazily.  How about
> > making them both lazy?
> 
> Done.
> 
> http://codereview.chromium.org/3660003/diff/7001/8009
> File chrome/worker/worker_webkitclient_impl.h (right):
> 
> http://codereview.chromium.org/3660003/diff/7001/8009#newcode11
> chrome/worker/worker_webkitclient_impl.h:11: #include
> "webkit/glue/webfileutilities_impl.h"
> On 2010/10/11 17:43:18, ericu wrote:
> > Do we still need to include webfileutilities_impl, or is the forward
> declaration
> > enough?
> 
> Changed to the forward declaration.

If you'll upload a new version, I'll LGTM; I don't think we need to wait for
Darin, given that the risk is small and it's in the current code.  If he thinks
there's a need for a check, we can add that in another changelist.

[jianli, 2010-10-14 01:10:11]

LGTM with some minor comments.

http://codereview.chromium.org/3660003/diff/7002/13001
File chrome/browser/renderer_host/file_utilities_dispatcher_host.cc (right):

http://codereview.chromium.org/3660003/diff/7002/13001#newcode158
chrome/browser/renderer_host/file_utilities_dispatcher_host.cc:158: int mode,
Not aligned correctly.

http://codereview.chromium.org/3660003/diff/7002/13002
File chrome/browser/renderer_host/file_utilities_dispatcher_host.h (right):

http://codereview.chromium.org/3660003/diff/7002/13002#newcode26
chrome/browser/renderer_host/file_utilities_dispatcher_host.h:26:
~FileUtilitiesDispatcherHost();
Why not moving the destructor to private scope, like:
 private:
  ~FileUtilitiesDispatcherHost();
  friend class base::RefCountedThreadSafe<FileUtilitiesDispatcherHost>;

http://codereview.chromium.org/3660003/diff/7002/13009
File chrome/worker/worker_webkitclient_impl.cc (right):

http://codereview.chromium.org/3660003/diff/7002/13009#newcode57
chrome/worker/worker_webkitclient_impl.cc:57: long long& result) {
Not aligned correctly.

http://codereview.chromium.org/3660003/diff/7002/13009#newcode231
chrome/worker/worker_webkitclient_impl.cc:231: ChildThread::current()->Send(new
ViewHostMsg_GetMimeTypeFromExtension(
Better to use WorkerThread::current().

http://codereview.chromium.org/3660003/diff/7002/13009#newcode239
chrome/worker/worker_webkitclient_impl.cc:239: ChildThread::current()->Send(new
ViewHostMsg_GetMimeTypeFromFile(
ditto.

http://codereview.chromium.org/3660003/diff/7002/13009#newcode248
chrome/worker/worker_webkitclient_impl.cc:248: ChildThread::current()->Send(
ditto.

[kinuko, 2010-10-14 19:22:36]

Thanks, all fixed.

http://codereview.chromium.org/3660003/diff/7002/13001
File chrome/browser/renderer_host/file_utilities_dispatcher_host.cc (right):

http://codereview.chromium.org/3660003/diff/7002/13001#newcode158
chrome/browser/renderer_host/file_utilities_dispatcher_host.cc:158: int mode,
On 2010/10/14 01:10:11, jianli wrote:
> Not aligned correctly.

Done.

http://codereview.chromium.org/3660003/diff/7002/13002
File chrome/browser/renderer_host/file_utilities_dispatcher_host.h (right):

http://codereview.chromium.org/3660003/diff/7002/13002#newcode26
chrome/browser/renderer_host/file_utilities_dispatcher_host.h:26:
~FileUtilitiesDispatcherHost();
On 2010/10/14 01:10:11, jianli wrote:
> Why not moving the destructor to private scope, like:
>  private:
>   ~FileUtilitiesDispatcherHost();
>   friend class base::RefCountedThreadSafe<FileUtilitiesDispatcherHost>;

Done.

http://codereview.chromium.org/3660003/diff/7002/13009
File chrome/worker/worker_webkitclient_impl.cc (right):

http://codereview.chromium.org/3660003/diff/7002/13009#newcode57
chrome/worker/worker_webkitclient_impl.cc:57: long long& result) {
On 2010/10/14 01:10:11, jianli wrote:
> Not aligned correctly.

Done.

http://codereview.chromium.org/3660003/diff/7002/13009#newcode231
chrome/worker/worker_webkitclient_impl.cc:231: ChildThread::current()->Send(new
ViewHostMsg_GetMimeTypeFromExtension(
On 2010/10/14 01:10:11, jianli wrote:
> Better to use WorkerThread::current().

Done.

http://codereview.chromium.org/3660003/diff/7002/13009#newcode239
chrome/worker/worker_webkitclient_impl.cc:239: ChildThread::current()->Send(new
ViewHostMsg_GetMimeTypeFromFile(
On 2010/10/14 01:10:11, jianli wrote:
> ditto.

Done.

http://codereview.chromium.org/3660003/diff/7002/13009#newcode248
chrome/worker/worker_webkitclient_impl.cc:248: ChildThread::current()->Send(
On 2010/10/14 01:10:11, jianli wrote:
> ditto.

Done.

[ericu, 2010-10-14 23:51:59]

LGTM.

On Thu, Oct 14, 2010 at 12:22 PM, <kinuko@chromium.org> wrote:

> Thanks, all fixed.
>
>
>
> http://codereview.chromium.org/3660003/diff/7002/13001
> File chrome/browser/renderer_host/file_utilities_dispatcher_host.cc
> (right):
>
> http://codereview.chromium.org/3660003/diff/7002/13001#newcode158
> chrome/browser/renderer_host/file_utilities_dispatcher_host.cc:158: int
> mode,
> On 2010/10/14 01:10:11, jianli wrote:
>
>> Not aligned correctly.
>>
>
> Done.
>
>
> http://codereview.chromium.org/3660003/diff/7002/13002
> File chrome/browser/renderer_host/file_utilities_dispatcher_host.h
> (right):
>
> http://codereview.chromium.org/3660003/diff/7002/13002#newcode26
> chrome/browser/renderer_host/file_utilities_dispatcher_host.h:26:
> ~FileUtilitiesDispatcherHost();
> On 2010/10/14 01:10:11, jianli wrote:
>
>> Why not moving the destructor to private scope, like:
>>  private:
>>   ~FileUtilitiesDispatcherHost();
>>   friend class
>>
> base::RefCountedThreadSafe<FileUtilitiesDispatcherHost>;
>
> Done.
>
>
> http://codereview.chromium.org/3660003/diff/7002/13009
> File chrome/worker/worker_webkitclient_impl.cc (right):
>
> http://codereview.chromium.org/3660003/diff/7002/13009#newcode57
> chrome/worker/worker_webkitclient_impl.cc:57: long long& result) {
> On 2010/10/14 01:10:11, jianli wrote:
>
>> Not aligned correctly.
>>
>
> Done.
>
>
> http://codereview.chromium.org/3660003/diff/7002/13009#newcode231
> chrome/worker/worker_webkitclient_impl.cc:231:
> ChildThread::current()->Send(new ViewHostMsg_GetMimeTypeFromExtension(
> On 2010/10/14 01:10:11, jianli wrote:
>
>> Better to use WorkerThread::current().
>>
>
> Done.
>
>
> http://codereview.chromium.org/3660003/diff/7002/13009#newcode239
> chrome/worker/worker_webkitclient_impl.cc:239:
> ChildThread::current()->Send(new ViewHostMsg_GetMimeTypeFromFile(
> On 2010/10/14 01:10:11, jianli wrote:
>
>> ditto.
>>
>
> Done.
>
>
> http://codereview.chromium.org/3660003/diff/7002/13009#newcode248
> chrome/worker/worker_webkitclient_impl.cc:248:
> ChildThread::current()->Send(
> On 2010/10/14 01:10:11, jianli wrote:
>
>> ditto.
>>
>
> Done.
>
>
> http://codereview.chromium.org/3660003/show
>

[darin (slow to review), 2010-10-15 00:02:02]

LGTM2