Add content API for DevTools HTTP handler

content/public/browser/devtools/devtools_http_handler.h

+// Copyright (c) 2011 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_PUBLIC_BROWSER_DEVTOOLS_DEVTOOLS_HTTP_HANDLER_H_
+#define CONTENT_PUBLIC_BROWSER_DEVTOOLS_DEVTOOLS_HTTP_HANDLER_H_
+#pragma once
+
+#include <string>
+
+#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"

[jam, 2011/11/21 19:26:56]

nit: not needed

[yurys, 2011/12/01 18:02:20]

Done.

+#include "content/common/content_export.h"
+
+namespace content {
+
+class DevToolsHttpHandlerDelegate;
+
+class DevToolsHttpHandler

[jam, 2011/11/21 19:26:56]

nit: please add a comment about what this class is used for

[yurys, 2011/12/01 18:02:20]

Done.

+    : public base::RefCountedThreadSafe<DevToolsHttpHandler> {

[jam, 2011/11/21 19:26:56]

it seems that the refcounting is an internal implementation detail. do we have
to expose that to embedders? can we structure DevToolsHttpHandlerImpl such that
it has an inner context class that is refcounted, but the interface itself is
not? i.e. see this example http://codereview.chromium.org/8603012/

[yurys, 2011/12/01 18:02:20]

Done. I made DevToolsHttpHandlerImpl refcounted but DevToolsHttpHandler is not
refcounted and has protected destructor so the only way for the API clients to
delete the handler is to call Stop on it. The implementation class keeps
scoped_refptr to self until Stop is called on it. Please let me know if you
think that converting DevToolsHttpHandlerImpl into some internal refcounted
context would work better.

+ public:
+  // Takes ownership over |delegate|.
+  CONTENT_EXPORT static scoped_refptr<DevToolsHttpHandler> Start(

[jam, 2011/11/21 19:26:56]

nit: usually for functions that construct an object, we call it "Create". also,
i'm curious why you're returning a scoped_refptr instead of a pointer? it seems
obvious that whoever calls a Create function on a refcounted object will want to
put in a scoped_refptr 

[yurys, 2011/12/01 18:02:20]

Done. This class is not refcounted anymore. I think reason for returning here
scoped_refptr was to force clients to assign this object to a scoped_refptr when
returning a raw pointer compiler won't help you if you miss that.

+      const std::string& ip,
+      int port,
+      const std::string& frontend_url,
+      DevToolsHttpHandlerDelegate* delegate);
+
+  // Called from the main thread in order to stop protocol handler.
+  // Will schedule tear down task on IO thread.
+  virtual CONTENT_EXPORT void Stop() = 0;
+
+ protected:
+  DevToolsHttpHandler() {}

[jam, 2011/11/21 19:26:56]

nit: not necessary

[yurys, 2011/12/01 18:02:20]

Done.

+  virtual ~DevToolsHttpHandler() {}
+
+ private:
+  friend class base::RefCountedThreadSafe<DevToolsHttpHandler>;
+
+  DISALLOW_COPY_AND_ASSIGN(DevToolsHttpHandler);

[jam, 2011/11/21 19:26:56]

(and also in the other interface)

nit: no need for this macro on interfaces. these objects aren't instantiable so
they can't be copied or assigned in the first place

[yurys, 2011/12/01 18:02:20]

Done.

+};
+
+}  // namespace content
+
+#endif  // CONTENT_PUBLIC_BROWSER_DEVTOOLS_DEVTOOLS_HTTP_HANDLER_H_