Missed some items to document here. New documentation.

ppapi/cpp/module.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 PPAPI_CPP_MODULE_H_
 #define PPAPI_CPP_MODULE_H_
 
 #include <map>
 #include <string>
 
@@ skipping 18 matching lines @@
 class Module {
  public:
   typedef std::map<PP_Instance, Instance*> InstanceMap;
 
   // You may not call any other PP functions from the constructor, put them
   // in Init instead. Various things will not be set up until the constructor
   // completes.
   Module();
   virtual ~Module();
 
-  // Returns the global instance of this module object, or NULL if the module
-  // is not initialized yet.
+  /// Get() returns the global instance of this module object, or NULL if the
+  /// module is not initialized yet.
+  ///
+  /// @return The global instance of the module object.
   static Module* Get();
 
-  // This function will be automatically called after the object is created.
-  // This is where you can put functions that rely on other parts of the API,
-  // now that the module has been created.
+  /// Init() is automatically called after the object is created. This is where
+  /// you can put functions that rely on other parts of the API, now that the
+  /// module has been created.
+  ///
+  /// @return True if success, otherwise false.

[dmichael (off chromium), 2011/08/11 22:20:50]

True->true
if->on ? (or success->successful)?

[jond, 2011/08/12 16:58:18]

Done.

[jond, 2011/08/12 16:58:18]

Done.

   virtual bool Init();
 
-  // Returns the internal module handle.
+  /// The <code>pp_module</code> function returns the internal module handle.

[dmichael (off chromium), 2011/08/11 22:20:50]

Why are you using 'The <code>function_name</code> function' here, but
'function_name()' above?

Please make them consistent. I think you've been using the other style
elsewhere, so:
/// pp_module() returns...

[jond, 2011/08/12 16:58:18]

The function name here is lowercase. So, I thought it looked bad to start a
sentence pp_module()... That's why I say "The <code>pp_module</code> function...

On 2011/08/11 22:20:50, dmichael wrote:

[dmichael (off chromium), 2011/08/12 19:13:55]

Okay, that's fine. But it should still be pp_module() instead of
<code>pp_module</code>, shouldn't it? Same elsewhere.

[jond, 2011/08/12 19:29:24]

Actually saying pp_module() function is wrong. That's the pp_module function
function. And, to say The pp_module() is strange. So, that's why its pp_module
function. 

On 2011/08/12 19:13:55, dmichael wrote:

[dmichael (off chromium), 2011/08/12 19:42:29]

Why? I don't read it 'function function'. All I'm saying is it should be
consistent. We should always write function names in 1 way, so that you can tell
at a glance that it's a function name. I'd rather you either use 'The
pp_module() function returns' or just write 'pp_module() returns' (even though
it means starting a sentence with lower-case. This is code documentation;
consistency and correctness should trump grammar in these cases, IMHO.

[jond, 2011/08/12 19:54:37]

I agree. I'll have to change this in several spots. How about "The
get_browser_interface() function" I'm not going to stress on this one.

- Jon

On 2011/08/12 19:42:29, dmichael wrote:

+  ///
+  /// @return A <code>PP_Module</code> internal module handle.
   PP_Module pp_module() const { return pp_module_; }
 
-  // Returns the internal get_browser_interface pointer.
-  // TODO(sehr): This should be removed once the NaCl browser plugin no longer
-  // needs it.
+  /// The <code>get_browser_interface</code> the internal get_browser_interface

[dmichael (off chromium), 2011/08/11 22:20:50]

/// get_browser_interface() returns the internal get_browser_interface pointer.

?

[jond, 2011/08/12 16:58:18]

Done.

+  /// pointer.
+  /// TODO(sehr): This should be removed once the NaCl browser plugin no longer
+  /// needs it.
+  ///
+  /// @return A <code>PPB_GetInterface</code> internal pointer.
   PPB_GetInterface get_browser_interface() const {
     return get_browser_interface_;
   }
 
-  // Returns the core interface for doing basic global operations. This is
-  // guaranteed to be non-NULL once the module has successfully initialized
-  // and during the Init() call.
-  //
-  // It will be NULL before Init() has been called.
+  /// The <code>core</code> function returns the core interface for doing basic

[dmichael (off chromium), 2011/08/11 22:20:50]

core() returns...

[jond, 2011/08/12 16:58:18]

See above. I will talk to Josie about this, but for now I'm going to start
sentences with capitals (even if its a function name). On 2011/08/11 22:20:50,
dmichael wrote:

+  /// global operations. The return value is guaranteed to be non-NULL once the
+  /// module has successfully initialized and during the Init() call.
+  ///
+  /// It will be NULL before Init() has been called.
+  ///
+  /// @return The core interface for doing basic global operations.
   Core* core() { return core_; }
 
-  // Implements GetInterface for the browser to get plugin interfaces. If you
-  // need to provide your own implementations of new interfaces, you can use
-  // AddPluginInterface which this function will use.
+  /// GetPluginInterface() implements <code>GetInterface</code> for the browser
+  /// to get module interfaces. If you need to provide your own implementations
+  /// of new interfaces, use AddPluginInterface() which this function will use.
+  ///
+  /// @param[in] interface_name The module interface for the browser to get.
   const void* GetPluginInterface(const char* interface_name);
 
-  // Returns an interface in the browser.
+  /// GetBrowserInterface() returns an interface in the browser.

[dmichael (off chromium), 2011/08/11 22:20:50]

I think the original wording is confusing. Maybe:
GetBrowserInterface() returns interfaces which the browser implements (i.e., PPB
interfaces).

Or similar.

[jond, 2011/08/12 16:58:18]

Done.

+  ///
+  /// @param[in] interface_name The browser interface for the moduel to get.
   const void* GetBrowserInterface(const char* interface_name);
 
-  // Returns the object associated with this PP_Instance, or NULL if one is
-  // not found.
+  /// InstanceForPPInstance() returns the object associated with this
+  /// <code>PP_Instance</code>, or NULL if one is not found.
+  ///
+  /// @param[in] instance This <code>PP_Instance</code>.
+  ///
+  /// @return The object associated with this <code>PP_Instance</code>,
+  /// or NULL if one is not found.
   Instance* InstanceForPPInstance(PP_Instance instance);
 
-  // Adds a handler for a given interface name. When the browser requests
-  // that interface name, the given |vtable| will be returned.
-  //
-  // In general, plugins will not need to call this directly. Instead, the
-  // C++ wrappers for each interface will register themselves with this
-  // function.
-  //
-  // This function may be called more than once with the same interface name
-  // and vtable with no effect. However, it may not be used to register a
-  // different vtable for an already-registered interface. It will assert for
-  // a different registration for an already-registered interface in debug
-  // mode, and just ignore the registration in release mode.
+  /// AddPluginInterface() adds a handler for a provided interface name. When
+  /// the browser requests that interface name, the provided
+  /// <code>vtable</code> will be returned.
+  ///
+  /// In general, modules will not need to call this directly. Instead, the
+  /// C++ wrappers for each interface will register themselves with this
+  /// function.
+  ///
+  /// This function may be called more than once with the same interface name
+  /// and vtable with no effect. However, it may not be used to register a
+  /// different vtable for an already-registered interface. It will assert for
+  /// a different registration for an already-registered interface in debug
+  /// mode, and just ignore the registration in release mode.
+  ///
+  /// @param[in] interface_name The interface name that will receive a handler.
+  /// @param[in,out] vtable The vtable returned for <code>interface_name</code>.

[dmichael (off chromium), 2011/08/11 22:20:50]

'returned'->'to return'

[jond, 2011/08/12 16:58:18]

Done.

   void AddPluginInterface(const std::string& interface_name,
                           const void* vtable);
 
-  // Sets the browser interface and calls the regular init function that
-  // can be overridden by the base classes.
-  //
-  // TODO(brettw) make this private when I can figure out how to make the
-  // initialize function a friend.
+  // InternalInit() sets the browser interface and calls the regular init

[dmichael (off chromium), 2011/08/11 22:20:50]

init -> Init()

[jond, 2011/08/12 16:58:18]

Done.

+  /// function that can be overridden by the base classes.
+  ///
+  /// TODO(brettw) make this private when I can figure out how to make the
+  /// initialize function a friend.
+  ///
+  /// @param[in] mod A <code>PP_Module</code>.
+  /// @param[in] get_browser_interface The browser interface to set.
+  ///
+  /// @return True if successful, otherwise false.

[dmichael (off chromium), 2011/08/11 22:20:50]

True->true

[jond, 2011/08/12 16:58:18]

Again, I was hoping to start sentences with a capital. But, if you think its
confusing, I'll revisit. On 2011/08/11 22:20:50, dmichael wrote:

[dmichael (off chromium), 2011/08/12 19:13:55]

When case is important, my opinion is it's much better to ignore the
capitalization rule and get the code right. 'True' has no meaning in C or C++,
but 'true' is a keyword. Since we have PP_TRUE in some places, and some
developers might be new to C/C++, it could be confusing. That's my 2 cents.

[jond, 2011/08/12 19:29:24]

Okay, I'll change that one.

On 2011/08/12 19:13:55, dmichael wrote:

[dmichael (off chromium), 2011/08/12 19:42:29]

Thanks!

   bool InternalInit(PP_Module mod,
                     PPB_GetInterface get_browser_interface);
 
-  // Allows iteration over the current instances in the module.
+  /// The <code>current_instance</code> allows iteration over the current

[dmichael (off chromium), 2011/08/11 22:20:50]

/// current_instances() allows...

[jond, 2011/08/12 16:58:18]

Done.

[jond, 2011/08/12 16:58:18]

Done.

+  /// instances in the module.
+  ///
+  /// @return An <code>InstanceMap</code> of all instances in the module.
   const InstanceMap& current_instances() const { return current_instances_; }
 
  protected:
-  // Override to create your own plugin type.
+  /// CreateInstance() should be overridden to create your own module type.
+  ///
+  /// @param[in] instance A <code>PP_Instance</code>.
+  ///
+  /// @return The resulting instance.
   virtual Instance* CreateInstance(PP_Instance instance) = 0;
 
  private:
   friend PP_Bool Instance_DidCreate(PP_Instance pp_instance,
                                     uint32_t argc,
                                     const char* argn[],
                                     const char* argv[]);
   friend void Instance_DidDestroy(PP_Instance instance);
 
   // Unimplemented (disallow copy and assign).
@@ skipping 10 matching lines @@
 
   // All additional interfaces this plugin can handle as registered by
   // AddPluginInterface.
   typedef std::map<std::string, const void*> InterfaceMap;
   InterfaceMap additional_interfaces_;
 };
 
 }  // namespace pp
 
 #endif  // PPAPI_CPP_MODULE_H_