GCP2.0 Device: Receiving printjobs.

cloud_print/gcp20/prototype/cloud_print_request.h

Index: cloud_print/gcp20/prototype/cloud_print_request.h
diff --git a/cloud_print/gcp20/prototype/cloud_print_request.h b/cloud_print/gcp20/prototype/cloud_print_request.h
new file mode 100644
index 0000000000000000000000000000000000000000..9c82fe8c9a421a1411e0eba4cba86283956084b5
--- /dev/null
+++ b/cloud_print/gcp20/prototype/cloud_print_request.h
@@ -0,0 +1,97 @@
+// 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.
+
+#ifndef CLOUD_PRINT_GCP20_PROTOTYPE_CLOUD_PRINT_REQUEST_H_
+#define CLOUD_PRINT_GCP20_PROTOTYPE_CLOUD_PRINT_REQUEST_H_
+
+#include <string>
+
+#include "base/basictypes.h"
+#include "base/callback.h"
+#include "base/memory/weak_ptr.h"
+#include "net/url_request/url_fetcher.h"
+#include "net/url_request/url_fetcher_delegate.h"
+
+// Request to CloudPrint with timeout control.
+// Delegate should delete this object once it is deleting.
+class CloudPrintRequest : public net::URLFetcherDelegate,
+                          public base::SupportsWeakPtr<CloudPrintRequest> {
+ public:
+  class Delegate {
+   public:
+    Delegate() {}
+    virtual ~Delegate() {}
+
+    // Invoked when |fetcher_| finished fetching successfully.
+    // Use for erasing instance of CloudPrintRequest class.
+    virtual void OnFetchComplete() = 0;
+
+    // Invoked when |fetcher_| finished fetching successfully.
+    // Use for erasing instance of CloudPrintRequest class.
+    virtual void OnFetchError(const std::string& server_api,
+                              int server_code,
+                              int server_http_code) = 0;
+
+    // Invoked when timeout is reached.
+    // Use for erasing instance of CloudPrintRequest class.
+    virtual void OnFetchTimeoutReached() = 0;
+
+    // Invoked when accesstoken is needed.
+    virtual std::string access_token() = 0;

[gene, 2013/07/23 09:36:23]

GetAccessToken() - ???
By formatting function a getter you assume a certain simple implementation.
However, you don't know how delegate implementor will actually implement this.
It is better to follow regular naming conversion here.

[maksymb, 2013/07/23 19:02:37]

Done.

+
+    // Invoked when context_getter is needed.
+    virtual scoped_refptr<net::URLRequestContextGetter> context_getter() = 0;

[gene, 2013/07/23 09:36:23]

GetContextGetter()
same here

[maksymb, 2013/07/23 19:02:37]

Done.

+  };
+
+  typedef base::Callback<void(const std::string&)> ParserCallback;
+
+  struct DataForUpload {

[gene, 2013/07/23 09:36:23]

Looks like you use this struct only once to pass params.
Since it has only 2 members, I would suggest get rid of this struct and pass
them to function directly.
For example,
static scoped_ptr<CloudPrintRequest> CreatePost(
    const GURL& url,
    const std::string& data_mimetype,
    const std::string& data,
    Delegate* delegate);

[maksymb, 2013/07/23 19:02:37]

Done.

+    std::string content;
+    std::string content_type;
+  };
+
+  virtual ~CloudPrintRequest();
+
+  // Creates GET request.
+  static scoped_ptr<CloudPrintRequest> CreateGet(
+      const GURL& url,

[gene, 2013/07/23 09:36:23]

nit: looks like you can put params to the function line:
static scoped_ptr<CloudPrintRequest> CreateGet(const GURL& url,
                                                                            
Delegate* delegate);

[maksymb, 2013/07/23 19:02:37]

Done.

+      Delegate* delegate);
+
+  // Creates POST request.
+  static scoped_ptr<CloudPrintRequest> CreatePost(
+      const GURL& url,

[gene, 2013/07/23 09:36:23]

nit: as above comment?

[maksymb, 2013/07/23 19:02:37]

Done.

+      const DataForUpload& data,
+      Delegate* delegate);
+
+  // Starts request. Once fetch was completed, parser will be called.
+  void Run(const ParserCallback& parser);

[gene, 2013/07/23 09:36:23]

This is really confusing. Why do you need both delegate and callback????
Looking at the cc file, looks like you call callback immediately after
OnFetchComplete(). Why don't you just add const std::string& parameter to
OnFetchComplete and get rid of callback here?

After reading class that uses CloudPrintRequest I see why you go this way. Still
it will be better to keep only delegate or only callbacks. Since your delegate
functions are called only once, it will be easier to switch everything to
callbacks if you like. How about this API:
    Run(success_callback, error_callback, access_token, context_getter)
Here:
    error_callback(bool timeout, server_api, server_code, http_error_code)
???

[maksymb, 2013/07/23 19:02:37]

Left Delegate only.

+
+  // Add header to request.
+  void AddHeader(const std::string& header);
+
+ private:
+  explicit CloudPrintRequest(Delegate* delegate);
+
+  // net::URLFetcherDelegate methods:
+  virtual void OnURLFetchComplete(const net::URLFetcher* source) OVERRIDE;
+
+  // Creates request.
+  static scoped_ptr<CloudPrintRequest> CreateRequest(
+      const GURL& url,
+      net::URLFetcher::RequestType method,
+      Delegate* delegate);
+
+  // Method for handling timeout.
+  void OnRequestTimeout();
+
+  scoped_ptr<net::URLFetcher> fetcher_;
+  ParserCallback parser_;
+
+  Delegate* delegate_;
+
+  DISALLOW_COPY_AND_ASSIGN(CloudPrintRequest);
+};
+
+#endif  // CLOUD_PRINT_GCP20_PROTOTYPE_CLOUD_PRINT_REQUEST_H_
+