Benchmark tool for GPU-accelerated video rendering

media/tools/shader_bench/window.h

+// Copyright (c) 2010 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 MEDIA_TOOLS_SHADER_BENCH_WINDOW_H_
+#define MEDIA_TOOLS_SHADER_BENCH_WINDOW_H_
+
+#include "base/scoped_ptr.h"

[tfarina, 2010/11/29 18:28:36]

It seems you are not using this include here. 

[vrk (LEFT CHROMIUM), 2010/11/30 01:21:35]

Done.

+#include "base/task.h"

[tfarina, 2010/11/29 18:28:36]

If I'm not wrong, it can be possible to forward declare Task and Painter.

[vrk (LEFT CHROMIUM), 2010/11/30 01:21:35]

Done.

+#include "gfx/native_widget_types.h"
+#include "media/tools/shader_bench/painter.h"
+
+namespace media {
+
+class Window {
+ public:
+  Window(int width, int height);

[tfarina, 2010/11/29 18:28:36]

For readability, could you add a blank line between these methods? I also would
move these virtual functions below PluginWindow (no need to address this
though).

[vrk (LEFT CHROMIUM), 2010/11/30 01:21:35]

Done.

+  virtual void Start(int limit, Task* done_task, Painter* painter);
+  virtual void OnPaint();
+  virtual void MainLoop();
+  gfx::NativeWindow CreateNativeWindow(int width, int height);
+  gfx::PluginWindowHandle PluginWindow();
+ protected:

[tfarina, 2010/11/29 18:28:36]

Please, left the blank line above protected, same thing with private.

[vrk (LEFT CHROMIUM), 2010/11/30 01:21:35]

Done.

+  Task* done_task_;
+  Painter* painter_;
+  int limit_;
+  int count_;
+  bool running_;
+  gfx::NativeWindow window_handle_;
+ private:

[Alpha Left Google, 2010/11/29 20:06:13]

Empty line above this. Each section should have an empty line from the previous
one.

[vrk (LEFT CHROMIUM), 2010/11/30 01:21:35]

Done.

+  DISALLOW_COPY_AND_ASSIGN(Window);
+};
+
+}  // namespace media
+
+#endif  // MEDIA_TOOLS_SHADER_BENCH_WINDOW_H_