Refactor how padding is calculated.

webrtc/call/bitrate_allocator_unittest.cc

Index: webrtc/call/bitrate_allocator_unittest.cc
diff --git a/webrtc/call/bitrate_allocator_unittest.cc b/webrtc/call/bitrate_allocator_unittest.cc
index 4017645c1d6f022988184fb5ac67b44ccc509649..53f04d3fe67e66f696196002fb419baab55d0aef 100644
--- a/webrtc/call/bitrate_allocator_unittest.cc
+++ b/webrtc/call/bitrate_allocator_unittest.cc
@@ -12,12 +12,22 @@
 #include <memory>
 #include <vector>
 
+#include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "webrtc/call/bitrate_allocator.h"
 #include "webrtc/modules/bitrate_controller/include/bitrate_controller.h"
 
+using testing::NiceMock;
+
 namespace webrtc {
 
+class MockLimitObserver : public BitrateAllocator::LimitObserver {
+ public:
+  MOCK_METHOD2(OnAllocationLimitsChanged,
+               void(uint32_t min_send_bitrate_bps,
+                    uint32_t max_padding_bitrate_bps));
+};
+
 class TestBitrateObserver : public BitrateAllocatorObserver {
  public:
   TestBitrateObserver()
@@ -37,18 +47,24 @@ class TestBitrateObserver : public BitrateAllocatorObserver {
 
 class BitrateAllocatorTest : public ::testing::Test {
  protected:
-  BitrateAllocatorTest() : allocator_(new BitrateAllocator()) {
+  BitrateAllocatorTest() : allocator_(new BitrateAllocator(&limit_observer_)) {
     allocator_->OnNetworkChanged(300000u, 0, 0);
   }
   ~BitrateAllocatorTest() {}
 
+  NiceMock<MockLimitObserver> limit_observer_;
   std::unique_ptr<BitrateAllocator> allocator_;
 };
 
 TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) {
   TestBitrateObserver bitrate_observer;
-  int start_bitrate =
-      allocator_->AddObserver(&bitrate_observer, 100000, 1500000, true);
+  const uint32_t kMinSendBitrateBps = 100000;
+  const uint32_t kPadUpToBitrateBps = 50000;
+
+  EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(kMinSendBitrateBps,
+                                                         kPadUpToBitrateBps));
+  int start_bitrate = allocator_->AddObserver(
+      &bitrate_observer, kMinSendBitrateBps, 1500000, kPadUpToBitrateBps, true);
   EXPECT_EQ(300000, start_bitrate);
   allocator_->OnNetworkChanged(200000, 0, 0);
   EXPECT_EQ(200000u, bitrate_observer.last_bitrate_);
@@ -57,12 +73,18 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) {
   // bitrate for FEC/retransmissions (see todo in BitrateAllocator).
   allocator_->OnNetworkChanged(4000000, 0, 0);
   EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_);
-  start_bitrate =
-      allocator_->AddObserver(&bitrate_observer, 100000, 4000000, true);
+
+  // Expect |max_padding_bitrate_bps| to change to 0 if the observer is updated.
+  EXPECT_CALL(limit_observer_,
+              OnAllocationLimitsChanged(kMinSendBitrateBps, 0));
+  start_bitrate = allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps,
+                                          4000000, 0, true);
   EXPECT_EQ(4000000, start_bitrate);
 
-  start_bitrate =
-      allocator_->AddObserver(&bitrate_observer, 100000, 1500000, true);
+  EXPECT_CALL(limit_observer_,
+              OnAllocationLimitsChanged(kMinSendBitrateBps, 0));
+  start_bitrate = allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps,
+                                          1500000, 0, true);
   EXPECT_EQ(3000000, start_bitrate);
   EXPECT_EQ(3000000u, bitrate_observer.last_bitrate_);
   allocator_->OnNetworkChanged(1500000, 0, 0);
@@ -72,11 +94,13 @@ TEST_F(BitrateAllocatorTest, UpdatingBitrateObserver) {
 TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) {
   TestBitrateObserver bitrate_observer_1;
   TestBitrateObserver bitrate_observer_2;
+  EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(100000, 0));
   int start_bitrate =
-      allocator_->AddObserver(&bitrate_observer_1, 100000, 300000, true);
+      allocator_->AddObserver(&bitrate_observer_1, 100000, 300000, 0, true);
   EXPECT_EQ(300000, start_bitrate);
+  EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(100000 + 200000, 0));
   start_bitrate =
-      allocator_->AddObserver(&bitrate_observer_2, 200000, 300000, true);
+      allocator_->AddObserver(&bitrate_observer_2, 200000, 300000, 0, true);
   EXPECT_EQ(200000, start_bitrate);
 
   // Test too low start bitrate, hence lower than sum of min. Min bitrates will
@@ -107,13 +131,28 @@ TEST_F(BitrateAllocatorTest, TwoBitrateObserversOneRtcpObserver) {
   EXPECT_EQ(0u, bitrate_observer_2.last_bitrate_);
 }
 
+TEST_F(BitrateAllocatorTest, RemoveObserverTriggersLimitObserver) {
+  TestBitrateObserver bitrate_observer;
+  const uint32_t kMinSendBitrateBps = 100000;
+  const uint32_t kPadUpToBitrateBps = 50000;
+
+  EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(kMinSendBitrateBps,
+                                                         kPadUpToBitrateBps));
+  allocator_->AddObserver(&bitrate_observer, kMinSendBitrateBps, 1500000,
+                          kPadUpToBitrateBps, true);
+  EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0));
+  allocator_->RemoveObserver(&bitrate_observer);
+}
+
 class BitrateAllocatorTestNoEnforceMin : public ::testing::Test {
  protected:
-  BitrateAllocatorTestNoEnforceMin() : allocator_(new BitrateAllocator()) {
+  BitrateAllocatorTestNoEnforceMin()
+      : allocator_(new BitrateAllocator(&limit_observer_)) {
     allocator_->OnNetworkChanged(300000u, 0, 0);
   }
   ~BitrateAllocatorTestNoEnforceMin() {}
 
+  NiceMock<MockLimitObserver> limit_observer_;
   std::unique_ptr<BitrateAllocator> allocator_;
 };
 
@@ -121,8 +160,11 @@ class BitrateAllocatorTestNoEnforceMin : public ::testing::Test {
 // as intended.
 TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) {
   TestBitrateObserver bitrate_observer_1;
+  // Expect OnAllocationLimitsChanged with |min_send_bitrate_bps| = 0 since
+  // AddObserver is called with |enforce_min_bitrate| = false.
+  EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0));
   int start_bitrate =
-      allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, false);
+      allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false);
   EXPECT_EQ(300000, start_bitrate);
 
   // High REMB.
@@ -133,6 +175,7 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, OneBitrateObserver) {
   allocator_->OnNetworkChanged(10000, 0, 0);
   EXPECT_EQ(10000u, bitrate_observer_1.last_bitrate_);
 
+  EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(0, 0));
   allocator_->RemoveObserver(&bitrate_observer_1);
 }
 
@@ -142,16 +185,16 @@ TEST_F(BitrateAllocatorTestNoEnforceMin, ThreeBitrateObservers) {
   TestBitrateObserver bitrate_observer_3;
   // Set up the observers with min bitrates at 100000, 200000, and 300000.
   int start_bitrate =
-      allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, false);
+      allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, false);
   EXPECT_EQ(300000, start_bitrate);
 
   start_bitrate =
-      allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, false);
+      allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, false);
   EXPECT_EQ(200000, start_bitrate);
   EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_);
 
   start_bitrate =
-      allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, false);
+      allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, 0, false);
   EXPECT_EQ(0, start_bitrate);
   EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_);
   EXPECT_EQ(200000u, bitrate_observer_2.last_bitrate_);
@@ -198,16 +241,16 @@ TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowRembEnforceMin) {
   TestBitrateObserver bitrate_observer_2;
   TestBitrateObserver bitrate_observer_3;
   int start_bitrate =
-      allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, true);
+      allocator_->AddObserver(&bitrate_observer_1, 100000, 400000, 0, true);
   EXPECT_EQ(300000, start_bitrate);
 
   start_bitrate =
-      allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, true);
+      allocator_->AddObserver(&bitrate_observer_2, 200000, 400000, 0, true);
   EXPECT_EQ(200000, start_bitrate);
   EXPECT_EQ(100000u, bitrate_observer_1.last_bitrate_);
 
   start_bitrate =
-      allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, true);
+      allocator_->AddObserver(&bitrate_observer_3, 300000, 400000, 0, true);
   EXPECT_EQ(300000, start_bitrate);
   EXPECT_EQ(100000, static_cast<int>(bitrate_observer_1.last_bitrate_));
   EXPECT_EQ(200000, static_cast<int>(bitrate_observer_2.last_bitrate_));
@@ -225,8 +268,9 @@ TEST_F(BitrateAllocatorTest, ThreeBitrateObserversLowRembEnforceMin) {
 
 TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) {
   TestBitrateObserver bitrate_observer_1;
+  EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(50000, 0));
   int start_bitrate =
-      allocator_->AddObserver(&bitrate_observer_1, 50000, 400000, true);
+      allocator_->AddObserver(&bitrate_observer_1, 50000, 400000, 0, true);
   EXPECT_EQ(300000, start_bitrate);
 
   // Set network down, ie, no available bitrate.
@@ -235,8 +279,10 @@ TEST_F(BitrateAllocatorTest, AddObserverWhileNetworkDown) {
   EXPECT_EQ(0u, bitrate_observer_1.last_bitrate_);
 
   TestBitrateObserver bitrate_observer_2;
+  // Adding an observer while the network is down should not affect the limits.
+  EXPECT_CALL(limit_observer_, OnAllocationLimitsChanged(50000 + 50000, 0));
   start_bitrate =
-      allocator_->AddObserver(&bitrate_observer_2, 50000, 400000, true);
+      allocator_->AddObserver(&bitrate_observer_2, 50000, 400000, 0, true);
 
   // Expect the start_bitrate to be set as if the network was still up but that
   // the new observer have been notified that the network is down.