JavaScript Content Handler Version 0.0

mojo/apps/js/js_app.h

+// Copyright 2014 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 JS_APP_H_
+#define JS_APP_H_
+
+#include "base/memory/ref_counted.h"
+#include "base/single_thread_task_runner.h"
+#include "base/threading/thread.h"
+#include "gin/public/isolate_holder.h"
+#include "gin/shell_runner.h"
+#include "mojo/apps/js/mojo_runner_delegate.h"
+#include "mojo/services/public/interfaces/content_handler/content_handler.mojom.h"
+
+namespace mojo {
+namespace apps {
+
+class JSContentHandlerApp;
+
+class JSAppRunnerDelegate : public MojoRunnerDelegate {
+ public:
+  JSAppRunnerDelegate();
+};
+
+class JSApp : public base::RefCountedThreadSafe<JSApp> {

[Aaron Boodman, 2014/09/10 02:22:09]

In general it is preferred that every class have a class header comment that
explains in a paragraph or so the 'big picture' of where the class fits in the
design, what its responsibility is, etc.

[Aaron Boodman, 2014/09/10 02:22:09]

RefCounting can make understanding code harder because it becomes unclear when,
and on what thread, a class will actually be destroyed.

I think you can get away without it if you have JSApp manage its own lifetime as
mentioned elsewhere in these comments.

[hansmuller, 2014/09/11 00:26:17]

OK

+ public:
+  JSApp(JSContentHandlerApp* content_handler_app,
+        const std::string& url,
+        URLResponsePtr content);
+
+  static JSApp* From(v8::Isolate* isolate);
+
+  ScopedMessagePipeHandle ConnectToService(
+      const std::string& application_url, const std::string& interface_name);
+
+  bool Start();
+  void Quit();
+
+ private:
+  JSContentHandlerApp* content_handler_app_;
+  std::string url_;
+  URLResponsePtr content_;
+  base::Thread thread_;
+  scoped_refptr<base::SingleThreadTaskRunner> content_handler_task_runner_;
+  scoped_refptr<base::SingleThreadTaskRunner> js_app_task_runner_;
+  JSAppRunnerDelegate runner_delegate_;
+  scoped_ptr<gin::IsolateHolder> isolate_holder_;
+  scoped_ptr<gin::ShellRunner> shell_runner_;
+
+  void Run();
+  void Terminate();
+  bool on_content_handler_thread() const;
+  bool on_js_app_thread() const;
+
+  ~JSApp();
+  friend class base::RefCountedThreadSafe<JSApp>;

[Aaron Boodman, 2014/09/10 02:22:09]

decl order is incorrect -- see reference to styleguide elsewhere in comments.

[hansmuller, 2014/09/11 00:26:16]

I've removed the RefCountedThreadSafe boilerplate here. I think that's what
failed style.

+  DISALLOW_COPY_AND_ASSIGN(JSApp);
+};
+
+}  // namespace apps
+}  // namespace mojo
+
+#endif  // JS_APP_H_