Readability review.

remoting/host/host_extension_session.h

Index: remoting/host/host_extension_session.h
diff --git a/remoting/host/host_extension_session.h b/remoting/host/host_extension_session.h
index a4a0bd710abd7c776688028bd9b4ce2bb52d32c3..d355cf16ff7d49e4df8ed0e638c9d7ff656aed3a 100644
--- a/remoting/host/host_extension_session.h
+++ b/remoting/host/host_extension_session.h
@@ -27,13 +27,18 @@ class HostExtensionSession {
  public:
   virtual ~HostExtensionSession() {}
 
-  // Optional hook functions for HostExtensions which need to wrap or replace
-  // parts of the video, audio, input, etc pipelines.
-  // These are called in response to ResetVideoPipeline().
-  virtual scoped_ptr<webrtc::DesktopCapturer> OnCreateVideoCapturer(
-      scoped_ptr<webrtc::DesktopCapturer> capturer);
-  virtual scoped_ptr<VideoEncoder> OnCreateVideoEncoder(
-      scoped_ptr<VideoEncoder> encoder);
+  // Hook functions called when the video pipeline is being (re)constructed.
+  // Implementations will receive these calls only if they express the need to
+  // modify the pipeline (see below). They may replace, wrap or take ownership

[Peter Kasting, 2014/08/26 20:58:15]

Nit: For clarity, I think maybe "take ownership" should be removed, as it's
redundant with the rest of your comment in the context of e.g. wrapping the
provided object, and it doesn't make much sense in the context of an ownership
transfer (since, as you note, the pipeline won't be constructed in that case).

[Wez, 2014/08/28 00:13:20]

Done.

+  // of |capturer| and/or |encoder|, e.g. to filter video frames in some way.
+  // If either |capturer| or |encoder| are reset then the video pipeline is not
+  // constructed.
+  virtual void OnCreateVideoCapturer(
+      scoped_ptr<webrtc::DesktopCapturer>* capturer);
+  virtual void OnCreateVideoEncoder(scoped_ptr<VideoEncoder>* encoder);
+
+  // Must return true if the HostExtensionSession needs the opportunity to
+  // modify the video pipeline.
   virtual bool ModifiesVideoPipeline() const;

[Peter Kasting, 2014/08/26 20:58:15]

Tiny nit: I wonder if "WantsToModifyVideoPipeline" would be clearer?  Not sure.

[Wez, 2014/08/28 00:13:20]

Acknowledged. I think Modifies* is sufficiently expressive, and more concise. :)

 
   // Called when the host receives an |ExtensionMessage| for the |ClientSession|