From 7b2f55ec2fa83873c4eeacb7fbccaecfedece63e Mon Sep 17 00:00:00 2001
From: Scott Mitchell <scott_mitchell@apple.com>
Date: Fri, 18 Dec 2015 13:35:37 -0800
Subject: [PATCH] HTTP/2 Remove RemoteFlowController.streamWritten

Motivation:
RemoteFlowController.streamWritten is not currently required. We should remove it to keep interfaces minimal.

Modifications:
- Remove RemoteFlowController.streamWritten

Result:
1 Less method in RemoteFlowController interface.
Fixes https://github.com/netty/netty/issues/4600
---
 .../DefaultHttp2RemoteFlowController.java     | 41 ++++---------------
 .../http2/Http2RemoteFlowController.java      | 12 ------
 .../DefaultHttp2RemoteFlowControllerTest.java | 21 ----------
 3 files changed, 9 insertions(+), 65 deletions(-)

diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java
index 2db3e70865..c99f86d627 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowController.java
@@ -205,7 +205,7 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
 
     @Override
     public void listener(Listener listener) {
-        monitor = listener == null ? new DefaultWritabilityMonitor() : new ListenerWritabilityMonitor(listener);
+        monitor = listener == null ? new WritabilityMonitor() : new ListenerWritabilityMonitor(listener);
     }
 
     @Override
@@ -627,13 +627,14 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
     /**
      * Abstract class which provides common functionality for writability monitor implementations.
      */
-    private abstract class WritabilityMonitor {
+    private class WritabilityMonitor {
         private long totalPendingBytes;
-        private final Writer writer;
-
-        protected WritabilityMonitor(Writer writer) {
-            this.writer = writer;
-        }
+        private final Writer writer = new StreamByteDistributor.Writer() {
+            @Override
+            public void write(Http2Stream stream, int numBytes) {
+                state(stream).writeAllocatedBytes(numBytes);
+            }
+        };
 
         /**
          * Called when the writability of the underlying channel changes.
@@ -728,20 +729,6 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
         }
     }
 
-    /**
-     * Provides no notification or tracking of writablity changes.
-     */
-    private final class DefaultWritabilityMonitor extends WritabilityMonitor {
-        DefaultWritabilityMonitor() {
-            super(new StreamByteDistributor.Writer() {
-                @Override
-                public void write(Http2Stream stream, int numBytes) {
-                    state(stream).writeAllocatedBytes(numBytes);
-                }
-            });
-        }
-    }
-
     /**
      * Writability of a {@code stream} is calculated using the following:
      * <pre>
@@ -763,17 +750,7 @@ public class DefaultHttp2RemoteFlowController implements Http2RemoteFlowControll
             }
         };
 
-        ListenerWritabilityMonitor(final Listener listener) {
-            super(new StreamByteDistributor.Writer() {
-                @Override
-                public void write(Http2Stream stream, int numBytes) {
-                    AbstractState state = state(stream);
-                    int written = state.writeAllocatedBytes(numBytes);
-                    if (written != -1) {
-                        listener.streamWritten(state.stream(), written);
-                    }
-                }
-            });
+        ListenerWritabilityMonitor(Listener listener) {
             this.listener = listener;
         }
 
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2RemoteFlowController.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2RemoteFlowController.java
index 279129c363..83fe96da84 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2RemoteFlowController.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2RemoteFlowController.java
@@ -138,18 +138,6 @@ public interface Http2RemoteFlowController extends Http2FlowController {
      * Listener to the number of flow-controlled bytes written per stream.
      */
     interface Listener {
-
-        /**
-         * Report the number of {@code writtenBytes} for a {@code stream}. Called after the
-         * flow-controller has flushed bytes for the given stream.
-         * <p>
-         * This method should not throw. Any thrown exceptions are considered a programming error and are ignored.
-         * @param stream that had bytes written.
-         * @param writtenBytes the number of bytes written for a stream, can be 0 in the case of an
-         *                     empty DATA frame.
-         */
-        void streamWritten(Http2Stream stream, int writtenBytes);
-
         /**
          * Notification that {@link Http2RemoteFlowController#isWritable(Http2Stream)} has changed for {@code stream}.
          * <p>
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java
index c98dfd0b7d..58742311dd 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/DefaultHttp2RemoteFlowControllerTest.java
@@ -170,7 +170,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         verifyZeroInteractions(listener);
         controller.writePendingBytes();
         data.assertFullyWritten();
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 5);
         verifyZeroInteractions(listener);
     }
 
@@ -181,7 +180,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         data.assertNotWritten();
         controller.writePendingBytes();
         data.assertFullyWritten();
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 0);
         verifyZeroInteractions(listener);
     }
 
@@ -210,7 +208,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         controller.writePendingBytes();
         data1.assertFullyWritten();
         data2.assertNotWritten();
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 15);
         verify(listener, times(1)).writabilityChanged(stream(STREAM_A));
         assertFalse(controller.isWritable(stream(STREAM_A)));
     }
@@ -267,7 +264,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         controller.writePendingBytes();
         // Verify that a partial frame of 5 remains to be sent
         data.assertPartiallyWritten(5);
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 5);
         verify(listener, times(1)).writabilityChanged(stream(STREAM_A));
         assertFalse(controller.isWritable(stream(STREAM_A)));
         verifyNoMoreInteractions(listener);
@@ -286,7 +282,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         controller.writePendingBytes();
         data.assertPartiallyWritten(10);
         moreData.assertNotWritten();
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 10);
         verify(listener, times(1)).writabilityChanged(stream(STREAM_A));
         assertFalse(controller.isWritable(stream(STREAM_A)));
         reset(listener);
@@ -302,7 +297,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
 
         data.assertFullyWritten();
         moreData.assertPartiallyWritten(5);
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 15);
         verify(listener, never()).writabilityChanged(stream(STREAM_A));
         assertFalse(controller.isWritable(stream(STREAM_A)));
 
@@ -331,7 +325,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         // Verify that the entire frame was sent.
         controller.initialWindowSize(10);
         data.assertFullyWritten();
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 10);
         assertWritabilityChanged(0, false);
     }
 
@@ -356,7 +349,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         dataA.assertPartiallyWritten(8);
         assertEquals(65527, window(STREAM_A));
         assertEquals(0, window(CONNECTION_STREAM_ID));
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 8);
         assertWritabilityChanged(0, false);
         reset(listener);
 
@@ -376,11 +368,9 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         // Verify the rest of A is written.
         dataA.assertFullyWritten();
         assertEquals(65525, window(STREAM_A));
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 2);
 
         dataB.assertFullyWritten();
         assertEquals(65525, window(STREAM_B));
-        verify(listener, times(1)).streamWritten(stream(STREAM_B), 10);
         verifyNoMoreInteractions(listener);
     }
 
@@ -399,7 +389,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         sendData(STREAM_A, data1);
         controller.writePendingBytes();
         data1.assertFullyWritten();
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 20);
         assertTrue(window(CONNECTION_STREAM_ID) > 0);
         verify(listener, times(1)).writabilityChanged(stream(STREAM_A));
         verify(listener, never()).writabilityChanged(stream(STREAM_B));
@@ -513,8 +502,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
 
         data.assertFullyWritten();
         data2.assertFullyWritten();
-
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 10);
     }
 
     @Test
@@ -540,7 +527,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         assertTrue(controller.isWritable(stream(STREAM_D)));
 
         data.assertPartiallyWritten(5);
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 5);
     }
 
     @Test
@@ -565,7 +551,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
 
         controller.writePendingBytes();
         data.assertFullyWritten();
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 10);
         assertWritabilityChanged(0, false);
         assertEquals(0, window(CONNECTION_STREAM_ID));
         assertEquals(DEFAULT_WINDOW_SIZE - 10, window(STREAM_A));
@@ -594,7 +579,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
 
         controller.writePendingBytes();
         data.assertPartiallyWritten(5);
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 5);
         assertWritabilityChanged(0, false);
         assertEquals(0, window(CONNECTION_STREAM_ID));
         assertEquals(DEFAULT_WINDOW_SIZE - 5, window(STREAM_A));
@@ -637,7 +621,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         data.assertNotWritten();
         controller.writePendingBytes();
         data.assertFullyWritten();
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 10);
         verify(listener, never()).writabilityChanged(stream(STREAM_A));
         verify(listener, never()).writabilityChanged(stream(STREAM_B));
         verify(listener, never()).writabilityChanged(stream(STREAM_C));
@@ -687,7 +670,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         data.assertNotWritten();
         controller.writePendingBytes();
         data.assertPartiallyWritten(5);
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 5);
         assertEquals(DEFAULT_WINDOW_SIZE - 5, window(CONNECTION_STREAM_ID));
         assertEquals(0, window(STREAM_A));
         assertEquals(DEFAULT_WINDOW_SIZE, window(STREAM_B));
@@ -717,7 +699,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         verify(flowControlled, never()).writeComplete();
 
         assertEquals(90, windowBefore - window(STREAM_A));
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 90);
         assertWritabilityChanged(0, true);
     }
 
@@ -794,7 +775,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         verify(flowControlled).writeComplete();
 
         assertEquals(150, windowBefore - window(STREAM_A));
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 150);
         assertWritabilityChanged(0, true);
     }
 
@@ -820,7 +800,6 @@ public abstract class DefaultHttp2RemoteFlowControllerTest {
         verify(flowControlled).write(any(ChannelHandlerContext.class), anyInt());
         verify(flowControlled).error(any(ChannelHandlerContext.class), any(Throwable.class));
         verify(flowControlled, never()).writeComplete();
-        verify(listener, times(1)).streamWritten(stream(STREAM_A), 0);
         verify(listener, times(1)).writabilityChanged(stream(STREAM_A));
         verify(listener, never()).writabilityChanged(stream(STREAM_B));
         verify(listener, never()).writabilityChanged(stream(STREAM_C));