Trigger user event when H2 conn preface & SETTINGS frame are sent

Motivation:
Previously client Http2ConnectionHandler trigger a user event
immediately when the HTTP/2 connection preface is sent. Any attempt to
immediately send a new request could cause the server to terminate the
connection, as it might not have received the SETTINGS frame from the
client. Per RFC7540 Section 3.5, the preface "MUST be followed by a
SETTINGS frame (Section 6.5), which MAY be empty."
(https://tools.ietf.org/html/rfc7540#section-3.5)

This event could be made more meaningful if it also indicates that the
initial client SETTINGS frame has been sent to signal that the channel
is ready to send new requests.

Modification:
- Renamed event to Http2ConnectionPrefaceAndSettingsFrameWrittenEvent.
- Modified Http2ConnectionHandler to trigger the user event only if it
  is a client and it has sent both the preface and SETTINGS frame.

Result:
It is now safe to use the event as an indicator that the HTTP/2
connection is ready to send new requests.

codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionHandler.java

@@ -364,15 +364,20 @@ public class Http2ConnectionHandler extends ByteToMessageDecoder implements Http
 
             prefaceSent = true;
 
-            if (!connection().isServer()) {
+            final boolean isClient = !connection().isServer();
+            if (isClient) {
                 // Clients must send the preface string as the first bytes on the connection.
                 ctx.write(connectionPrefaceBuf()).addListener(ChannelFutureListener.CLOSE_ON_FAILURE);
-                ctx.fireUserEventTriggered(Http2ConnectionPrefaceWrittenEvent.INSTANCE);
             }
 
             // Both client and server must send their initial settings.
             encoder.writeSettings(ctx, initialSettings, ctx.newPromise()).addListener(
                     ChannelFutureListener.CLOSE_ON_FAILURE);
+
+            if (isClient) {
+                ctx.fireUserEventTriggered(
+                        Http2ConnectionPrefaceAndSettingsFrameWrittenEvent.INSTANCE);
+            }
         }
     }
 

codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ConnectionPrefaceAndSettingsFrameWrittenEvent.java

@@ -17,14 +17,15 @@ package io.netty.handler.codec.http2;
 import io.netty.util.internal.UnstableApi;
 
 /**
- * Signifies that the <a href="https://tools.ietf.org/html/rfc7540#section-3.5">connection preface</a> has been sent.
+ * Signifies that the <a href="https://tools.ietf.org/html/rfc7540#section-3.5">connection preface</a> and
- * The client sends the preface, and the server receives the preface. The client shouldn't write any data until this
+ * the initial SETTINGS frame have been sent. The client sends the preface, and the server receives the preface.
- * event has been processed.
+ * The client shouldn't write any data until this event has been processed.
  */
 @UnstableApi
-public final class Http2ConnectionPrefaceWrittenEvent {
+public final class Http2ConnectionPrefaceAndSettingsFrameWrittenEvent {
-    static final Http2ConnectionPrefaceWrittenEvent INSTANCE = new Http2ConnectionPrefaceWrittenEvent();
+    static final Http2ConnectionPrefaceAndSettingsFrameWrittenEvent INSTANCE =
+            new Http2ConnectionPrefaceAndSettingsFrameWrittenEvent();
 
-    private Http2ConnectionPrefaceWrittenEvent() {
+    private Http2ConnectionPrefaceAndSettingsFrameWrittenEvent() {
     }
 }

codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java

@@ -231,7 +231,7 @@ public class Http2FrameCodec extends Http2ConnectionHandler {
      */
     @Override
     public final void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
-        if (evt instanceof Http2ConnectionPrefaceWrittenEvent) {
+        if (evt == Http2ConnectionPrefaceAndSettingsFrameWrittenEvent.INSTANCE) {
             // The user event implies that we are on the client.
             tryExpandConnectionFlowControlWindow(connection());
         } else if (evt instanceof UpgradeEvent) {

codec-http2/src/test/java/io/netty/handler/codec/http2/DataCompressionHttp2Test.java

@@ -347,7 +347,7 @@ public class DataCompressionHttp2Test {
                 p.addLast(clientHandler);
                 p.addLast(new ChannelInboundHandlerAdapter() {
                     public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
-                        if (evt instanceof Http2ConnectionPrefaceWrittenEvent) {
+                        if (evt == Http2ConnectionPrefaceAndSettingsFrameWrittenEvent.INSTANCE) {
                             prefaceWrittenLatch.countDown();
                             ctx.pipeline().remove(this);
                         }

codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionHandlerTest.java

@@ -44,6 +44,7 @@ import org.mockito.stubbing.Answer;
 
 import java.util.List;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import static io.netty.buffer.Unpooled.copiedBuffer;
 import static io.netty.handler.codec.http2.Http2CodecUtil.connectionPrefaceBuf;
@@ -240,6 +241,34 @@ public class Http2ConnectionHandlerTest {
         }
     }
 
+    @Test
+    public void clientShouldveSentPrefaceAndSettingsFrameWhenUserEventIsTriggered() throws Exception {
+        when(connection.isServer()).thenReturn(false);
+        when(channel.isActive()).thenReturn(false);
+        handler = newHandler();
+        when(channel.isActive()).thenReturn(true);
+
+        final Http2ConnectionPrefaceAndSettingsFrameWrittenEvent evt =
+                Http2ConnectionPrefaceAndSettingsFrameWrittenEvent.INSTANCE;
+
+        final AtomicBoolean verified = new AtomicBoolean(false);
+        final Answer verifier = new Answer() {
+            @Override
+            public Object answer(final InvocationOnMock in) throws Throwable {
+                assertTrue(in.getArgument(0).equals(evt));  // sanity check...
+                verify(ctx).write(eq(connectionPrefaceBuf()));
+                verify(encoder).writeSettings(eq(ctx), any(Http2Settings.class), any(ChannelPromise.class));
+                verified.set(true);
+                return null;
+            }
+        };
+
+        doAnswer(verifier).when(ctx).fireUserEventTriggered(evt);
+
+        handler.channelActive(ctx);
+        assertTrue(verified.get());
+    }
+
     @Test
     public void clientShouldSendClientPrefaceStringWhenActive() throws Exception {
         when(connection.isServer()).thenReturn(false);

codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ConnectionRoundtripTest.java

@@ -933,7 +933,7 @@ public class Http2ConnectionRoundtripTest {
                 p.addLast(new ChannelInboundHandlerAdapter() {
                     @Override
                     public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
-                        if (evt instanceof Http2ConnectionPrefaceWrittenEvent) {
+                        if (evt == Http2ConnectionPrefaceAndSettingsFrameWrittenEvent.INSTANCE) {
                             prefaceWrittenLatch.countDown();
                             ctx.pipeline().remove(this);
                         }

codec-http2/src/test/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandlerTest.java

@@ -61,7 +61,6 @@ import static io.netty.handler.codec.http.HttpMethod.POST;
 import static io.netty.handler.codec.http.HttpVersion.HTTP_1_1;
 import static io.netty.handler.codec.http2.Http2TestUtil.of;
 import static io.netty.util.CharsetUtil.UTF_8;
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -541,7 +540,7 @@ public class HttpToHttp2ConnectionHandlerTest {
                 p.addLast(new ChannelInboundHandlerAdapter() {
                     @Override
                     public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
-                        if (evt instanceof Http2ConnectionPrefaceWrittenEvent) {
+                        if (evt == Http2ConnectionPrefaceAndSettingsFrameWrittenEvent.INSTANCE) {
                             prefaceWrittenLatch.countDown();
                             ctx.pipeline().remove(this);
                         }

codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java

@@ -60,7 +60,6 @@ import static io.netty.handler.codec.http2.Http2CodecUtil.getEmbeddedHttp2Except
 import static io.netty.handler.codec.http2.Http2Exception.isStreamError;
 import static io.netty.handler.codec.http2.Http2TestUtil.of;
 import static io.netty.handler.codec.http2.Http2TestUtil.runInChannel;
-import static java.util.concurrent.TimeUnit.MILLISECONDS;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
@@ -725,7 +724,7 @@ public class InboundHttp2ToHttpAdapterTest {
                 });
                 p.addLast(new ChannelInboundHandlerAdapter() {
                     public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception {
-                        if (evt instanceof Http2ConnectionPrefaceWrittenEvent) {
+                        if (evt == Http2ConnectionPrefaceAndSettingsFrameWrittenEvent.INSTANCE) {
                             prefaceWrittenLatch.countDown();
                             ctx.pipeline().remove(this);
                         }