Test file errors in downloads.

content/test/test_file_error_injector.h

+// Copyright (c) 2012 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.
+
+#ifndef CONTENT_TEST_TEST_FILE_ERROR_INJECTOR_H_
+#define CONTENT_TEST_TEST_FILE_ERROR_INJECTOR_H_
+#pragma once
+
+#include <string>
+
+#include "base/memory/ref_counted.h"
+#include "net/base/net_errors.h"
+
+class GURL;
+
+namespace content {
+
+class DownloadId;
+
+// Test helper for injecting errors into download file operations.
+// All errors for a download must be injected before it starts.
+// This class needs to be |RefCountedThreadSafe| because the implementation
+// is referenced by other classes that live past the time when the user is
+// nominally done with it.  These classes are internal to content/.
+//
+// NOTE: No more than one download with the same URL can be in progress at
+// the same time.  You can have multiple simultaneous downloads as long as the
+// URLs are different, as the URLs are used as keys to get information about
+// the download.
+//
+// Example:
+//
+// FileErrorInfo a = { url1, ... };
+// FileErrorInfo b = { url2, ... };
+//
+// scoped_refptr<TestFileErrorInjector> injector =
+//     TestFileErrorInjector::Create();
+//
+// injector->AddError();
+// injector->AddError();

[Randy Smith (Not in Mondays), 2012/03/06 23:05:19]

Shouldn't these refer to a & b?

[ahendrickson, 2012/03/08 22:00:08]

Done.

+// injector->InjectErrors();
+//
+// download_manager->DownloadUrl(url1, ...);
+// download_manager->DownloadUrl(url2, ...);
+// ... wait for downloads to finish or get an injected error ...

[Randy Smith (Not in Mondays), 2012/03/06 23:05:19]

Thank you--I think this comment makes this class much more usable.

+//
+class TestFileErrorInjector
+    : public base::RefCountedThreadSafe<TestFileErrorInjector> {
+ public:
+  enum FileOperationCode {
+    FILE_OPERATION_INITIALIZE,
+    FILE_OPERATION_WRITE,
+    FILE_OPERATION_RENAME
+  };
+
+  // Structure that encapsulates the information needed to inject a file error.
+  struct FileErrorInfo {
+    std::string url;  // Full URL of the download.  Identifies the download.
+    FileOperationCode code;  // Operation to affect.
+    int operation_instance;  // 0-based count of operation calls, for each code.
+    net::Error net_error;  // Error to inject.
+  };
+
+  TestFileErrorInjector() {}

[Randy Smith (Not in Mondays), 2012/03/06 23:05:19]

If you want all instances of this class to be created through Create(), this
should be private.  That also allows you to enforce that all instances of this
class are accessed through a scoped_refptr<> which I think you want.

[ahendrickson, 2012/03/08 22:00:08]

Made it protected, as another class inherits from this.

[Randy Smith (Not in Mondays), 2012/03/12 01:44:50]

Good point; oops on my part.

+
+  // Creates an instance.  May only be called once.

[Randy Smith (Not in Mondays), 2012/03/06 23:05:19]

I'd say something about lifetime here, since many people when they see "May only
be called once" will think "Singleton".  Maybe a suffix something like "but will
not be kept alive past the last copy of the returned scoped_refptr"?

[ahendrickson, 2012/03/08 22:00:08]

Done.

+  static scoped_refptr<TestFileErrorInjector> Create();
+
+  // Adds an error.
+  // Must be called before |InjectErrors()| for a particular download file.

[Randy Smith (Not in Mondays), 2012/03/06 23:05:19]

This is a little misleading; you can have multiple calls to AddError() before or
after InjectErrors().  I think what you're trying to say is "Will only take
effect after the next call to InjectErorrs()"; if so, that's probably a better
way to say it.

[ahendrickson, 2012/03/08 22:00:08]

Done.

[Randy Smith (Not in Mondays), 2012/03/12 01:44:50]

Not seeing it?

[ahendrickson, 2012/03/12 11:51:02]

I did change the comment.  If it's still not clear, please let me know.

[Randy Smith (Not in Mondays), 2012/03/12 17:43:50]

Yeah, sorry, I think the interleaving in the diff format confused me.  This
looks good.

+  // Only one error per file will be used.

[Randy Smith (Not in Mondays), 2012/03/06 23:05:19]

We have one comment thread indicating that it's an interface violation for this
to be called more than once for a given file, but the implementation looks to me
as if it just has the "last call wins" property.  Whichever is true, it should
be documented here (I don't care about the behavior--I just want it documented.)

[ahendrickson, 2012/03/08 22:00:08]

The implementation currently DCHECKs if the same URL is used twice when you call
AddError().

[Randy Smith (Not in Mondays), 2012/03/12 01:44:50]

Cool; thanks.

+  virtual bool AddError(const FileErrorInfo& error_info) = 0;
+
+  // Clears all errors.
+  // Only affects files created after the next call to InjectErrors().
+  virtual void ClearErrors() = 0;
+
+  // Injects the errors such that new download files will be affected.
+  // The download system must already be initialized before calling this.
+  // Multiple calls are allowed, but only useful if the errors have changed.
+  // Replaces the injected error list.
+  virtual bool InjectErrors() = 0;
+
+  // Get information about the DownloadFiles created.
+  virtual size_t CurrentFileCount() const = 0;
+  virtual size_t TotalFileCount() const = 0;
+  virtual bool HadFile(const GURL& url) const = 0;
+  virtual const content::DownloadId GetId(const GURL& url) const = 0;
+  virtual void ClearFoundFiles() = 0;
+
+  static std::string DebugString(FileOperationCode code);
+
+ protected:
+  virtual ~TestFileErrorInjector();
+
+ private:
+  friend class base::RefCountedThreadSafe<TestFileErrorInjector>;
+
+  DISALLOW_COPY_AND_ASSIGN(TestFileErrorInjector);
+};
+
+}  // namespace content
+
+#endif  // CONTENT_TEST_TEST_FILE_ERROR_INJECTOR_H_