Browser Plugin: Renderer-side changes

content/renderer/browser_plugin/guest_render_view_observer.h

+// Copyright (c) 2012 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_RENDERER_GUEST_RENDER_VIEW_OBSERVER_H_
+#define CONTENT_RENDERER_GUEST_RENDER_VIEW_OBSERVER_H_
+#pragma once
+
+#include "content/public/renderer/render_view_observer.h"
+#include "content/renderer/browser_plugin/guest_to_host_channel.h"
+
+namespace content {
+class RenderView;

[jam, 2012/04/06 21:05:23]

nit: don't need this, it must be in render_view_observer.h by definition

[Fady Samuel, 2012/04/06 22:46:32]

Oops, must be a remnant of an earlier revision of this file when it wasn't a
RenderViewObserver. Thanks.

+}
+
+class GuestRenderViewObserver: public content::RenderViewObserver {

[piman, 2012/04/06 23:19:26]

Please add comments to this class about what it's for and how to use it.

Name-wise, "GuestRenderViewObserver" doesn't speak to me much. It tells me what
it is rather than what it's for. Maybe because looking at the interface doesn't
tell me much about what it does.

+public:
+
+  WebGraphicsContext3DCommandBufferImpl* GetWebGraphicsContext3D(
+      const WebKit::WebGraphicsContext3D::Attributes& attributes);
+
+  int routing_id() { return content::RenderViewObserver::routing_id(); }
+
+  void IssueSwapBuffers();

[piman, 2012/04/06 23:19:26]

Please add documentation.

+
+  // RenderViewObserver implementation.
+

[jam, 2012/04/06 21:05:23]

nit: no need for blank line. also see my comments from the other header re
comments and spacing

[Fady Samuel, 2012/04/06 22:46:32]

Done.

+  virtual bool OnMessageReceived(const IPC::Message& msg) OVERRIDE;
+
+private:

[jam, 2012/04/06 21:05:23]

spacing..

[Fady Samuel, 2012/04/06 22:46:32]

Done.

+  GuestRenderViewObserver(content::RenderView* render_view);
+
+  ~GuestRenderViewObserver() { }
+
+  void OnCreateChannel(base::ProcessHandle host_process_handle,
+                       int renderer_id);
+
+  void OnGuestReady(int instance_id,
+                    base::ProcessHandle process_handle,
+                    const IPC::ChannelHandle& channel_handle);
+
+  GuestToHostChannel guest_to_host_channel_;
+
+  friend class RenderViewImpl;

[jam, 2012/04/06 21:05:23]

why are you friending instead of making these public methods? this destroys
encapsulation

[Fady Samuel, 2012/04/06 22:46:32]

Thanks for catching this. This is a remnant of an earlier revision of the code
where the destruction and construction of this object was controlled
differently.

+  friend class GuestToHostChannel;
+};
+
+#endif // CONTENT_RENDERER_GUEST_RENDER_VIEW_OBSERVER_H_