Validate image files before writing them to media galleries.

chrome/browser/media_galleries/fileapi/media_file_validator_factory.cc

+// Copyright 2013 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/media_galleries/fileapi/media_file_validator_factory.h"
+
+#include "base/files/file_path.h"
+#include "base/platform_file.h"
+#include "chrome/browser/media_galleries/fileapi/supported_image_type_validator.h"
+#include "content/public/browser/browser_thread.h"
+#include "webkit/fileapi/copy_or_move_file_validator.h"
+#include "webkit/fileapi/file_system_url.h"
+
+using content::BrowserThread;
+
+namespace chrome {
+
+namespace {
+
+class InvalidFileValidator : public fileapi::CopyOrMoveFileValidator {
+ public:
+  virtual ~InvalidFileValidator() {}
+  virtual void StartValidation(

[Greg Billock, 2013/05/22 16:21:59]

The signature I'd expect here is StartValidation($FILE, callback).

[vandebo (ex-Chrome), 2013/05/22 18:41:17]

$FILE is passed into the factory so you can create an appropriate validator. 
The caller doesn't want to pass it twice, and it should be able to pass in a
different file than it passed to the factory.

[Greg Billock, 2013/05/23 18:18:29]

The same validator always gets created, right?

[vandebo (ex-Chrome), 2013/05/24 01:13:05]

Right now we only have one validator because that's all I've done.  But we'll
need more to support audio, video, other image formats, etc.

+      const fileapi::CopyOrMoveFileValidator::ResultCallback& result_callback) {
+    result_callback.Run(base::PLATFORM_FILE_ERROR_SECURITY);
+  }
+
+ private:
+  friend class chrome::MediaFileValidatorFactory;
+
+  InvalidFileValidator() {};
+
+  DISALLOW_COPY_AND_ASSIGN(InvalidFileValidator);
+};
+
+}  // namespace
+
+MediaFileValidatorFactory::MediaFileValidatorFactory() {}
+MediaFileValidatorFactory::~MediaFileValidatorFactory() {}
+
+fileapi::CopyOrMoveFileValidator*
+MediaFileValidatorFactory::CreateCopyOrMoveFileValidator(
+    const fileapi::FileSystemURL& src,
+    const base::FilePath& platform_path) {
+  base::FilePath src_path = src.virtual_path();
+  if (SupportedImageTypeValidator::SupportsFileType(src_path))

[Greg Billock, 2013/05/22 16:21:59]

I'd move all this logic to the SupportedImageTypeValidator.

[vandebo (ex-Chrome), 2013/05/22 18:41:17]

Not sure what you mean, SupportsFileType is a member of
SupportedImageTypeValidator.

[Greg Billock, 2013/05/23 18:18:29]

Meaning I'd have this method just return new SupportedImageTypeValidator(path).
Then have the StartValidation routine make this call to SupportsFileType, and
return the sec error if not. Then you don't need the whole InvalidFileValidator
class at all, and the whole validation operation resides in one spot, instead of
split between here and the actual validator. It'd also mean you can ditch the
constructor argument, which is odd, and perhaps even ditch the factory entirely,
which would be even better.

Is there a plan to make all different sorts of validators? That would motivate
this, but it seems overkill for one.

[vandebo (ex-Chrome), 2013/05/24 01:13:05]

Yes, there will be multiple validators and I will need to figure out which one
to use.  The factory may be a bit overkill - it was designed when all the code
still lived in webkit/fileapi.  Now I could just have the code from this
function live in MediaFileSystemMountPointProvider, but that means that the
mount point provider will have to be able to depend on the validator in the
generic mechanism.

+    return new SupportedImageTypeValidator(platform_path);
+  // TODO(vandebo) Support other file types.

[Greg Billock, 2013/05/22 16:21:59]

":"

[vandebo (ex-Chrome), 2013/05/22 18:41:17]

Done.

+
+  return new InvalidFileValidator();
+}
+
+}  // namespace chrome