Defines initial inner circle of interfaces for background offlining.

components/offline_pages/background_offliner.h

+// Copyright 2016 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 COMPONENTS_OFFLINE_PAGES_BACKGROUND_OFFLINER_H_
+#define COMPONENTS_OFFLINE_PAGES_BACKGROUND_OFFLINER_H_
+
+#include <stdint.h>

[fgorski, 2016/04/25 21:07:37]

nit: do you need this?

[dougarnett, 2016/04/25 21:49:08]

Done.

+
+#include "base/callback.h"
+
+namespace offline_pages {
+
+class SavePageRequest;
+
+// Interface of a class responsible for constructing an offline page given
+// a request with a URL.
+class BackgroundOffliner {
+ public:
+  // Completion status of processing an offline page request.
+  enum CompletionStatus {
+    SAVED = 0,
+    CANCELED = 1,
+    TIMEOUT = 2,

[fgorski, 2016/04/25 21:07:37]

Would timeout be a different thing than one of the two below?

Also should fialed_xxx offer details of what failed, like timeout and it would
be up to the coordinator to interpret the result as retry vs. not retry?

[dougarnett, 2016/04/25 21:49:08]

There is a question about what the Coordinator will care about. At this point, I
am thinking about retry policy but not sure about more. Thinking the Offliner
can provider Loader-specific UMA and interpret whether retriable or not. Would
like to avoid passing superset of client and server failure types to Coordinator
unless useful to it. 

I could drop the TIMEOUT one for now - just the two chunked types of FAILUREs?
Was thinking that TIMEOUT might be an interesting distinction for retry policy
but really not sure about it yet - so can add later if/when makes sense.

+    FAILED_CONSIDER_RETRY = 3,
+    FAILED_DO_NOT_RETRY = 4,
+  };
+
+  typedef base::Callback<void(CompletionStatus)> CompletionCallback;
+

[Pete Williamson, 2016/04/25 20:12:16]

Seems like we need a constructor, which brings up the question about how this
class is configured, whether we need to pass in policy at config time, or have a
hardcoded policy class that uses a data store somewhere for flexibility.

[dougarnett, 2016/04/25 21:49:08]

Yep, good stuff. Trying to get minimum/basline connections/terminology in *.h's
that
we can expand from. Can increase the scope of the CL if you think it is
important.

+  virtual ~BackgroundOffliner() {}
+
+  // Processes |request| to load and save an offline page.
+  // Only one request may be outstanding and will return false if a new
+  // request is not accepted.

[Pete Williamson, 2016/04/25 20:12:16]

Will we always be limited to one request?  To me that seems like a limitation of
the current pre-render implementation.  How about deleting the "Only one request
may be outstanding and" part, and leaving the warning about returning false if
not accepted?

[fgorski, 2016/04/25 21:07:37]

+1 on only one request

[dougarnett, 2016/04/25 21:49:08]

Ok, generalized the wording. I do think we stick to one initially though.
If we were to end up supporting multiple requests, we likely will want
to change the interface (to take list or at least return a request id
that the callback references). 

+  virtual bool LoadAndSave(
+      const SavePageRequest& request,
+      const CompletionCallback& callback) = 0;
+
+  // Clears the currently processing request, if any.
+  virtual void CancelCurrent() = 0;

[fgorski, 2016/04/25 21:07:37]

Just Cancel.

[dougarnett, 2016/04/25 21:49:08]

Done.

+
+  // TODO(dougarnett): add policy support methods.
+};
+
+}  // namespace offline_pages
+
+#endif  // COMPONENTS_OFFLINE_PAGES_BACKGROUND_OFFLINER_H_