Make 0 call sequence numbers be invalid.

This was part of a previous change I abandoned but this part still seems like
a good idea, since if you store sequence numbers for some reason, it's
impossible to store one indicating you haven't made a request.

I also worked to define the wraparound behavior since from a recent
Chromium-dev thread, I learned that signed overflow is not defined in C++. My
previous patch did this by using unsigned sequence numbers, but this is
discouraged by the style guide and is kind of difficult to change at this point
anyway.

BUG=

Review URL: https://codereview.chromium.org/11375003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@166776 0039d316-1c4b-4281-b951-d872f2087c98
diff --git a/ppapi/proxy/plugin_resource.cc b/ppapi/proxy/plugin_resource.cc
index 22eba26..3a02309 100644
--- a/ppapi/proxy/plugin_resource.cc
+++ b/ppapi/proxy/plugin_resource.cc
@@ -4,6 +4,8 @@
 
 #include "ppapi/proxy/plugin_resource.h"
 
+#include <limits>
+
 #include "ppapi/proxy/ppapi_messages.h"
 
 namespace ppapi {
@@ -12,7 +14,7 @@
 PluginResource::PluginResource(Connection connection, PP_Instance instance)
     : Resource(OBJECT_IS_PROXY, instance),
       connection_(connection),
-      next_sequence_number_(0),
+      next_sequence_number_(1),
       sent_create_to_browser_(false),
       sent_create_to_renderer_(false) {
 }
@@ -74,13 +76,13 @@
     DCHECK(!sent_create_to_browser_);
     sent_create_to_browser_ = true;
   }
-  ResourceMessageCallParams params(pp_resource(), next_sequence_number_++);
+  ResourceMessageCallParams params(pp_resource(), GetNextSequence());
   GetSender(dest)->Send(
       new PpapiHostMsg_ResourceCreated(params, pp_instance(), msg));
 }
 
 void PluginResource::Post(Destination dest, const IPC::Message& msg) {
-  ResourceMessageCallParams params(pp_resource(), next_sequence_number_++);
+  ResourceMessageCallParams params(pp_resource(), GetNextSequence());
   SendResourceCall(dest, params, msg);
 }
 
@@ -95,7 +97,7 @@
 int32_t PluginResource::GenericSyncCall(Destination dest,
                                         const IPC::Message& msg,
                                         IPC::Message* reply) {
-  ResourceMessageCallParams params(pp_resource(), next_sequence_number_++);
+  ResourceMessageCallParams params(pp_resource(), GetNextSequence());
   params.set_has_callback();
   ResourceMessageReplyParams reply_params;
   bool success = GetSender(dest)->Send(new PpapiHostMsg_ResourceSyncCall(
@@ -105,5 +107,17 @@
   return PP_ERROR_FAILED;
 }
 
+int32_t PluginResource::GetNextSequence() {
+  // Return the value with wraparound, making sure we don't make a sequence
+  // number with a 0 ID. Note that signed wraparound is undefined in C++ so we
+  // manually check.
+  int32_t ret = next_sequence_number_;
+  if (next_sequence_number_ == std::numeric_limits<int32_t>::max())
+    next_sequence_number_ = 1;  // Skip 0 which is invalid.
+  else
+    next_sequence_number_++;
+  return ret;
+}
+
 }  // namespace proxy
 }  // namespace ppapi
diff --git a/ppapi/proxy/plugin_resource.h b/ppapi/proxy/plugin_resource.h
index 3b564b4..33d1996a 100644
--- a/ppapi/proxy/plugin_resource.h
+++ b/ppapi/proxy/plugin_resource.h
@@ -81,13 +81,13 @@
   // still be invoked but with the default values of the message parameters.
   //
   // Returns the new request's sequence number which can be used to identify
-  // the callback.
+  // the callback. This value will never be 0, which you can use to identify
+  // an invalid callback.
   //
-  // Note: 1) all integers (including 0 and -1) are valid request IDs.
-  //       2) when all plugin references to this resource are gone or the
+  // Note: 1) When all plugin references to this resource are gone or the
   //          corresponding plugin instance is deleted, all pending callbacks
   //          are abandoned.
-  //       3) it is *not* recommended to let |callback| hold any reference to
+  //       2) It is *not* recommended to let |callback| hold any reference to
   //          |this|, in which it will be stored. Otherwise, this object will
   //          live forever if we fail to clean up the callback. It is safe to
   //          use base::Unretained(this) or a weak pointer, because this object
@@ -135,8 +135,11 @@
                           const IPC::Message& msg,
                           IPC::Message* reply_msg);
 
+  int32_t GetNextSequence();
+
   Connection connection_;
 
+  // Use GetNextSequence to retrieve the next value.
   int32_t next_sequence_number_;
 
   bool sent_create_to_browser_;