Split Media-related code from IsolatedMountPointProvider

Split Media-related code from IsolatedMountPointProvider

- Add MediaFileSystemMountPointProvider
- Change CopyOrMoveFileValidatorUnittest not to rely on Media-FS types

BUG=175936
TEST=content_unittests:CopyOrMoveValidatorTest.*

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=195432

[kinuko, 2013-04-18 11:46:54]

Hi Steve, Greg, this one's a proposal patch to split Media-related code from
IsolatedMountPointProvider as a preparation to move out all of the
webkit/fileapi/media code under chrome/browser/media_galleries or somewhere.

Can you take a look, and/or add relevant reviewers? Or please feel free to just
take over this patch if you want to work on your own. (I can review then)

(Btw this change is created on top of yet another non-submitted patch:
https://codereview.chromium.org/14341004/. Please feel free to comment on the
patch too)

Thanks!

[Greg Billock, 2013-04-18 16:18:02]

https://codereview.chromium.org/14352004/diff/3001/webkit/fileapi/file_system...
File webkit/fileapi/file_system_context.cc (right):

https://codereview.chromium.org/14352004/diff/3001/webkit/fileapi/file_system...
webkit/fileapi/file_system_context.cc:90:
RegisterMountPointProvider(media_provider_.get());
So the goal is to eventually have these registered from outside?

https://codereview.chromium.org/14352004/diff/3001/webkit/fileapi/media/nativ...
File webkit/fileapi/media/native_media_file_util.cc (right):

https://codereview.chromium.org/14352004/diff/3001/webkit/fileapi/media/nativ...
webkit/fileapi/media/native_media_file_util.cc:23:
MediaFileSystemMountPointProvider::kMediaPathFilterKey);
I don't understand why this fileutil would know about MediaPathFilter. Is this
just part of the way to decoupling? Or is this just something different that
doesn't even belong in MediaFileSystemMountPointProvider?

[kinuko, 2013-04-19 15:19:31]

https://codereview.chromium.org/14352004/diff/3001/webkit/fileapi/file_system...
File webkit/fileapi/file_system_context.cc (right):

https://codereview.chromium.org/14352004/diff/3001/webkit/fileapi/file_system...
webkit/fileapi/file_system_context.cc:90:
RegisterMountPointProvider(media_provider_.get());
On 2013/04/18 16:18:02, Greg Billock wrote:
> So the goal is to eventually have these registered from outside?

Yes, that's the plan.  Now the ctor of this class (FileSystemContext) takes
'additional_providers' parameters so we can pass providers from outside
webkit/fileapi.

The last patch to unlock us to pass chrome-specific providers to this ctor is:
https://codereview.chromium.org/14317004/

https://codereview.chromium.org/14352004/diff/3001/webkit/fileapi/media/nativ...
File webkit/fileapi/media/native_media_file_util.cc (right):

https://codereview.chromium.org/14352004/diff/3001/webkit/fileapi/media/nativ...
webkit/fileapi/media/native_media_file_util.cc:23:
MediaFileSystemMountPointProvider::kMediaPathFilterKey);
On 2013/04/18 16:18:02, Greg Billock wrote:
> I don't understand why this fileutil would know about MediaPathFilter. Is this
> just part of the way to decoupling? Or is this just something different that
> doesn't even belong in MediaFileSystemMountPointProvider?

My assumption here is: MediaFSMountPointProvider and media-related file utils
belong to the same MediaGalleries module (or MediaFS module), so they may share
some common variables like this, while FileAPI layer doesn't know anything about
Media so it just provides general setter like {Set,Get}UserValue like this for
FS modules to get/set their own variables.  Let me know if this assumption
doesn't fit with the model in your mind.

[Greg Billock, 2013-04-19 15:27:47]

lgtm

https://codereview.chromium.org/14352004/diff/3001/webkit/fileapi/media/nativ...
File webkit/fileapi/media/native_media_file_util.cc (right):

https://codereview.chromium.org/14352004/diff/3001/webkit/fileapi/media/nativ...
webkit/fileapi/media/native_media_file_util.cc:23:
MediaFileSystemMountPointProvider::kMediaPathFilterKey);
No, that makes sense. I was thinking I was in native_file_util.

On 2013/04/19 15:19:32, kinuko wrote:
> On 2013/04/18 16:18:02, Greg Billock wrote:
> > I don't understand why this fileutil would know about MediaPathFilter. Is
this
> > just part of the way to decoupling? Or is this just something different that
> > doesn't even belong in MediaFileSystemMountPointProvider?
> 
> My assumption here is: MediaFSMountPointProvider and media-related file utils
> belong to the same MediaGalleries module (or MediaFS module), so they may
share
> some common variables like this, while FileAPI layer doesn't know anything
about
> Media so it just provides general setter like {Set,Get}UserValue like this for
> FS modules to get/set their own variables.  Let me know if this assumption
> doesn't fit with the model in your mind.
>

[commit-bot: I haz the power, 2013-04-20 05:05:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinuko@chromium.org/14352004/16002

[commit-bot: I haz the power, 2013-04-20 05:05:35]

Failed to apply patch for webkit/fileapi/copy_or_move_file_validator.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file webkit/fileapi/copy_or_move_file_validator.h
  Hunk #1 FAILED at 8.
  1 out of 1 hunk FAILED -- saving rejects to file
webkit/fileapi/copy_or_move_file_validator.h.rej

Patch:       webkit/fileapi/copy_or_move_file_validator.h
Index: webkit/fileapi/copy_or_move_file_validator.h
diff --git a/webkit/fileapi/copy_or_move_file_validator.h
b/webkit/fileapi/copy_or_move_file_validator.h
index
a3e4a42a468084873366365fef707942a37ecabe..424a2b07c63cf14850116659ba1cee0a35cfd94b
100644
--- a/webkit/fileapi/copy_or_move_file_validator.h
+++ b/webkit/fileapi/copy_or_move_file_validator.h
@@ -8,10 +8,11 @@
 #include "base/callback.h"
 #include "base/files/file_path.h"
 #include "base/platform_file.h"
+#include "webkit/storage/webkit_storage_export.h"
 
 namespace fileapi {
 
-class CopyOrMoveFileValidator {
+class WEBKIT_STORAGE_EXPORT CopyOrMoveFileValidator {
  public:
   // Callback that is invoked when validation completes. A result of
   // base::PLATFORM_FILE_OK means the file validated.

[commit-bot: I haz the power, 2013-04-20 05:09:16]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/kinuko@chromium.org/14352004/24001

[commit-bot: I haz the power, 2013-04-20 05:23:33]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[kinuko, 2013-04-22 02:18:58]

Committed patchset #5 manually as r195432 (presubmit successful).