Remove timing limitation of SetOption invocation for PPAPI sockets.
Currently PPAPI has timing limitation for sockets' SetOption.
NODELAY, and BROADCAST need to be before Connect() or Bind(),
while RCVBUF_SIZE and SNFBUF_SIZE need to be after it.
This CL removes such a limitation.
Along with the change, pepper_udp_socket_message_filter starts to use UDPSocket instead of UDPServerSocket, so that the implementation direction gets closer to TCP message filter a little bit.
BUG=425563, 420697
TEST=Ran trybots.
Review URL: https://codereview.chromium.org/690903002
Cr-Commit-Position: refs/heads/master@{#307867}
diff --git a/ppapi/proxy/tcp_socket_private_resource.cc b/ppapi/proxy/tcp_socket_private_resource.cc
index 76ed4b9e1..454c3faf 100644
--- a/ppapi/proxy/tcp_socket_private_resource.cc
+++ b/ppapi/proxy/tcp_socket_private_resource.cc
@@ -101,7 +101,9 @@
case PP_TCPSOCKETOPTION_PRIVATE_INVALID:
return PP_ERROR_BADARGUMENT;
case PP_TCPSOCKETOPTION_PRIVATE_NO_DELAY:
- return SetOptionImpl(PP_TCPSOCKET_OPTION_NO_DELAY, value, callback);
+ return SetOptionImpl(PP_TCPSOCKET_OPTION_NO_DELAY, value,
+ true, // Check connect() state.
+ callback);
default:
NOTREACHED();
return PP_ERROR_BADARGUMENT;
diff --git a/ppapi/proxy/tcp_socket_resource.cc b/ppapi/proxy/tcp_socket_resource.cc
index f8f8f68..5466d0d 100644
--- a/ppapi/proxy/tcp_socket_resource.cc
+++ b/ppapi/proxy/tcp_socket_resource.cc
@@ -115,10 +115,21 @@
CloseImpl();
}
+int32_t TCPSocketResource::SetOption1_1(
+ PP_TCPSocket_Option name,
+ const PP_Var& value,
+ scoped_refptr<TrackedCallback> callback) {
+ return SetOptionImpl(name, value,
+ true, // Check connect() state.
+ callback);
+}
+
int32_t TCPSocketResource::SetOption(PP_TCPSocket_Option name,
const PP_Var& value,
scoped_refptr<TrackedCallback> callback) {
- return SetOptionImpl(name, value, callback);
+ return SetOptionImpl(name, value,
+ false, // Do not check connect() state.
+ callback);
}
PP_Resource TCPSocketResource::CreateAcceptedSocket(
diff --git a/ppapi/proxy/tcp_socket_resource.h b/ppapi/proxy/tcp_socket_resource.h
index 6a8e9fff..6ac09298 100644
--- a/ppapi/proxy/tcp_socket_resource.h
+++ b/ppapi/proxy/tcp_socket_resource.h
@@ -47,6 +47,10 @@
virtual int32_t Accept(PP_Resource* accepted_tcp_socket,
scoped_refptr<TrackedCallback> callback) override;
virtual void Close() override;
+ virtual int32_t SetOption1_1(
+ PP_TCPSocket_Option name,
+ const PP_Var& value,
+ scoped_refptr<TrackedCallback> callback) override;
virtual int32_t SetOption(PP_TCPSocket_Option name,
const PP_Var& value,
scoped_refptr<TrackedCallback> callback) override;
diff --git a/ppapi/proxy/tcp_socket_resource_base.cc b/ppapi/proxy/tcp_socket_resource_base.cc
index 39c9fa3..fb4db5b 100644
--- a/ppapi/proxy/tcp_socket_resource_base.cc
+++ b/ppapi/proxy/tcp_socket_resource_base.cc
@@ -318,11 +318,12 @@
int32_t TCPSocketResourceBase::SetOptionImpl(
PP_TCPSocket_Option name,
const PP_Var& value,
+ bool check_connect_state,
scoped_refptr<TrackedCallback> callback) {
SocketOptionData option_data;
switch (name) {
case PP_TCPSOCKET_OPTION_NO_DELAY: {
- if (!state_.IsConnected())
+ if (check_connect_state && !state_.IsConnected())
return PP_ERROR_FAILED;
if (value.type != PP_VARTYPE_BOOL)
@@ -332,7 +333,7 @@
}
case PP_TCPSOCKET_OPTION_SEND_BUFFER_SIZE:
case PP_TCPSOCKET_OPTION_RECV_BUFFER_SIZE: {
- if (!state_.IsConnected())
+ if (check_connect_state && !state_.IsConnected())
return PP_ERROR_FAILED;
if (value.type != PP_VARTYPE_INT32)
diff --git a/ppapi/proxy/tcp_socket_resource_base.h b/ppapi/proxy/tcp_socket_resource_base.h
index 8835ab97..3274fb9 100644
--- a/ppapi/proxy/tcp_socket_resource_base.h
+++ b/ppapi/proxy/tcp_socket_resource_base.h
@@ -95,6 +95,7 @@
void CloseImpl();
int32_t SetOptionImpl(PP_TCPSocket_Option name,
const PP_Var& value,
+ bool check_connect_state,
scoped_refptr<TrackedCallback> callback);
void PostAbortIfNecessary(scoped_refptr<TrackedCallback>* callback);
diff --git a/ppapi/proxy/udp_socket_private_resource.cc b/ppapi/proxy/udp_socket_private_resource.cc
index af43c10f..60afc6f3 100644
--- a/ppapi/proxy/udp_socket_private_resource.cc
+++ b/ppapi/proxy/udp_socket_private_resource.cc
@@ -41,7 +41,9 @@
NOTREACHED();
return PP_ERROR_BADARGUMENT;
}
- int32_t result = SetOptionImpl(public_name, value, NULL);
+ int32_t result = SetOptionImpl(public_name, value,
+ true, // Check bind() state.
+ NULL);
return result == PP_OK_COMPLETIONPENDING ? PP_OK : result;
}
diff --git a/ppapi/proxy/udp_socket_resource.cc b/ppapi/proxy/udp_socket_resource.cc
index 9ce7c91f..d815b9f 100644
--- a/ppapi/proxy/udp_socket_resource.cc
+++ b/ppapi/proxy/udp_socket_resource.cc
@@ -75,11 +75,22 @@
CloseImpl();
}
+int32_t UDPSocketResource::SetOption1_0(
+ PP_UDPSocket_Option name,
+ const PP_Var& value,
+ scoped_refptr<TrackedCallback> callback) {
+ return SetOptionImpl(name, value,
+ true, // Check bind() state.
+ callback);
+}
+
int32_t UDPSocketResource::SetOption(
PP_UDPSocket_Option name,
const PP_Var& value,
scoped_refptr<TrackedCallback> callback) {
- return SetOptionImpl(name, value, callback);
+ return SetOptionImpl(name, value,
+ false, // Check bind() state.
+ callback);
}
} // namespace proxy
diff --git a/ppapi/proxy/udp_socket_resource.h b/ppapi/proxy/udp_socket_resource.h
index e0d099a..81aad26 100644
--- a/ppapi/proxy/udp_socket_resource.h
+++ b/ppapi/proxy/udp_socket_resource.h
@@ -36,6 +36,10 @@
PP_Resource addr,
scoped_refptr<TrackedCallback> callback) override;
virtual void Close() override;
+ virtual int32_t SetOption1_0(
+ PP_UDPSocket_Option name,
+ const PP_Var& value,
+ scoped_refptr<TrackedCallback> callback) override;
virtual int32_t SetOption(PP_UDPSocket_Option name,
const PP_Var& value,
scoped_refptr<TrackedCallback> callback) override;
diff --git a/ppapi/proxy/udp_socket_resource_base.cc b/ppapi/proxy/udp_socket_resource_base.cc
index 521a6e2..8ed6d029 100644
--- a/ppapi/proxy/udp_socket_resource_base.cc
+++ b/ppapi/proxy/udp_socket_resource_base.cc
@@ -34,6 +34,7 @@
bool private_api)
: PluginResource(connection, instance),
private_api_(private_api),
+ bind_called_(false),
bound_(false),
closed_(false),
read_buffer_(NULL),
@@ -61,6 +62,7 @@
int32_t UDPSocketResourceBase::SetOptionImpl(
PP_UDPSocket_Option name,
const PP_Var& value,
+ bool check_bind_state,
scoped_refptr<TrackedCallback> callback) {
if (closed_)
return PP_ERROR_FAILED;
@@ -69,8 +71,14 @@
switch (name) {
case PP_UDPSOCKET_OPTION_ADDRESS_REUSE:
case PP_UDPSOCKET_OPTION_BROADCAST: {
- if (bound_)
+ if ((check_bind_state || name == PP_UDPSOCKET_OPTION_ADDRESS_REUSE) &&
+ bind_called_) {
+ // SetOption should fail in this case in order to give predictable
+ // behavior while binding. Note that we use |bind_called_| rather
+ // than |bound_| since the latter is only set on successful completion
+ // of Bind().
return PP_ERROR_FAILED;
+ }
if (value.type != PP_VARTYPE_BOOL)
return PP_ERROR_BADARGUMENT;
option_data.SetBool(PP_ToBool(value.value.as_bool));
@@ -78,7 +86,7 @@
}
case PP_UDPSOCKET_OPTION_SEND_BUFFER_SIZE:
case PP_UDPSOCKET_OPTION_RECV_BUFFER_SIZE: {
- if (!bound_)
+ if (check_bind_state && !bound_)
return PP_ERROR_FAILED;
if (value.type != PP_VARTYPE_INT32)
return PP_ERROR_BADARGUMENT;
@@ -111,6 +119,7 @@
if (TrackedCallback::IsPending(bind_callback_))
return PP_ERROR_INPROGRESS;
+ bind_called_ = true;
bind_callback_ = callback;
// Send the request, the browser will call us back via BindReply.
diff --git a/ppapi/proxy/udp_socket_resource_base.h b/ppapi/proxy/udp_socket_resource_base.h
index 5400f6be..d21579b 100644
--- a/ppapi/proxy/udp_socket_resource_base.h
+++ b/ppapi/proxy/udp_socket_resource_base.h
@@ -54,6 +54,7 @@
int32_t SetOptionImpl(PP_UDPSocket_Option name,
const PP_Var& value,
+ bool check_bind_state,
scoped_refptr<TrackedCallback> callback);
int32_t BindImpl(const PP_NetAddress_Private* addr,
scoped_refptr<TrackedCallback> callback);
@@ -106,6 +107,11 @@
PP_Resource* output_addr);
bool private_api_;
+
+ // |bind_called_| is true after Bind() is called, while |bound_| is true,
+ // after Bind() succeeds. Bind() is an asynchronous method, so the timing
+ // on which of these is set is slightly different.
+ bool bind_called_;
bool bound_;
bool closed_;
