Move fileapi into its own component

Move fileapi into its own component

Before this change fileapi is a static library. As it has static data, this causes problems when used from multiple components like chrome and content.

BUG=129885
TEST=Existing tests


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

[benwells, 2012-05-28 06:06:41]

An alternative to this approach would be to create a new dll or to put the
IsolatedContext lazy instance into an existing dll like webkit_glue.

[kinuko, 2012-05-28 06:58:41]

I'd defer this to jam.

[jam, 2012-05-28 07:17:29]

On 2012/05/28 06:06:41, benwells wrote:
> An alternative to this approach would be to create a new dll or to put the
> IsolatedContext lazy instance into an existing dll like webkit_glue.

either of these sound better to me (I prefer the dll one), instead of adding new
content APIs. we want to keep the content api as small as possible, and not add
stuff only because of the component build.

[benwells, 2012-05-28 07:49:57]

On 2012/05/28 07:17:29, John Abd-El-Malek wrote:
> On 2012/05/28 06:06:41, benwells wrote:
> > An alternative to this approach would be to create a new dll or to put the
> > IsolatedContext lazy instance into an existing dll like webkit_glue.
> 
> either of these sound better to me (I prefer the dll one), instead of adding
new
> content APIs. we want to keep the content api as small as possible, and not
add
> stuff only because of the component build.

OK, thanks for the quick feedback!

[benwells, 2012-05-29 10:23:00]

kinuko for flieapi
jam for layering
tony as webkit owner.

There are a couple of minor changes for me to make, which are noted in the
comments, but apart from that this is ready for review.

https://chromiumcodereview.appspot.com/10447055/diff/6038/chrome/browser/exte...
File chrome/browser/extensions/platform_app_browsertest.cc (left):

https://chromiumcodereview.appspot.com/10447055/diff/6038/chrome/browser/exte...
chrome/browser/extensions/platform_app_browsertest.cc:302: #if
!defined(OS_LINUX)
This was working on linux but not linux_shared.

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/blob/webkit_...
File webkit/blob/webkit_blob.gypi (left):

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/blob/webkit_...
webkit/blob/webkit_blob.gypi:57:
'<(DEPTH)/webkit/support/webkit_support.gyp:glue',
Removing this hasn't caused any problems with try jobs, and removes a circular
dependency for windows component builds.

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
File webkit/fileapi/file_system_url_request_job_factory.h (right):

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
webkit/fileapi/file_system_url_request_job_factory.h:24:
CreateFileSystemProtocolHandler(FileSystemContext* context);
I should fix this indent while I am here.

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/glue/webkit_...
File webkit/glue/webkit_glue.cc (left):

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/glue/webkit_...
webkit/glue/webkit_glue.cc:350: }
Having this here caused a circular dependency. webkit_glue depends on fileapi,
and fileapi did depend on webkit_glue just for this funciton.

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/glue/webkit_...
File webkit/glue/webkit_glue.gypi (right):

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/glue/webkit_...
webkit/glue/webkit_glue.gypi:136:
'<(DEPTH)/webkit/support/webkit_support.gyp:fileapi',
I don't think this change is needed; I'll remove it.

[kinuko, 2012-05-29 10:55:36]

The code change in fileapi lgtm, thanks for doing this!!

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
File webkit/fileapi/file_system_operation_context.h (right):

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
webkit/fileapi/file_system_operation_context.h:12: #include
"webkit/fileapi/file_system_context.h"
Why we need this to be included?

[benwells, 2012-05-29 11:40:21]

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
File webkit/fileapi/file_system_operation_context.h (right):

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
webkit/fileapi/file_system_operation_context.h:12: #include
"webkit/fileapi/file_system_context.h"
On 2012/05/29 10:55:36, kinuko wrote:
> Why we need this to be included?

On Windows this was needed to compile.

I assume adding the dll export for the FileSystemOperationContext meant it
needed to have full information about this type it is using. I'm not sure if
it's because of the getter or the scoped refptr.

[kinuko, 2012-05-29 11:47:54]

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
File webkit/fileapi/file_system_operation_context.h (right):

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
webkit/fileapi/file_system_operation_context.h:12: #include
"webkit/fileapi/file_system_context.h"
On 2012/05/29 11:40:22, benwells wrote:
> On 2012/05/29 10:55:36, kinuko wrote:
> > Why we need this to be included?
> 
> On Windows this was needed to compile.
> 
> I assume adding the dll export for the FileSystemOperationContext meant it
> needed to have full information about this type it is using. I'm not sure if
> it's because of the getter or the scoped refptr.

Ok thanks for checking!

[jam, 2012-05-29 17:31:15]

lgtm

[ericu, 2012-05-29 17:59:49]

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
File webkit/fileapi/fileapi_export.h (right):

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
webkit/fileapi/fileapi_export.h:12: #if defined(FILEAPI_IMPLEMENTATION)
What's the difference between FILEAPI_EXPORT and FILEAPI_EXPORT_PRIVATE?  Unless
I'm misreading this file, I don't see any in the definitions.  If there's going
to be a difference later, is there any automated way of catching it if we use
the wrong one?

[tony, 2012-05-29 18:52:42]

lgtm

[benwells, 2012-05-29 23:46:51]

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
File webkit/fileapi/fileapi_export.h (right):

https://chromiumcodereview.appspot.com/10447055/diff/6038/webkit/fileapi/file...
webkit/fileapi/fileapi_export.h:12: #if defined(FILEAPI_IMPLEMENTATION)
On 2012/05/29 17:59:49, ericu wrote:
> What's the difference between FILEAPI_EXPORT and FILEAPI_EXPORT_PRIVATE? 
Unless
> I'm misreading this file, I don't see any in the definitions.  If there's
going
> to be a difference later, is there any automated way of catching it if we use
> the wrong one?

There is no difference. Apparently this is a documentation kind of thing, which
means there is no way of knowing if it is right or not which is less than
desirable.

This pattern is used elsewhere (e.g. BASE_EXPORT), I was told on irc it's the
correct thing to do.

[commit-bot: I haz the power, 2012-05-30 00:55:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/10447055/3009

[commit-bot: I haz the power, 2012-05-30 03:17:15]

Try job failure for 10447055-3009 (retry) on win_rel for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2012-05-30 04:38:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/benwells@chromium.org/10447055/3009

[commit-bot: I haz the power, 2012-05-30 08:14:32]

Change committed as 139498