Updated operation of progress notifications to not send report for em…

…pty progress

src/realm/object-store/sync/sync_session.cpp

@@ -1585,10 +1585,16 @@ void SyncProgressNotifier::set_local_version(uint64_t snapshot_version)
 util::UniqueFunction<void()>
 SyncProgressNotifier::NotifierPackage::create_invocation(Progress const& current_progress, bool& is_expired)
 {
-    uint64_t transfered = is_download ? current_progress.downloaded : current_progress.uploaded;
+    uint64_t transferred = is_download ? current_progress.downloaded : current_progress.uploaded;
     uint64_t transferable = is_download ? current_progress.downloadable : current_progress.uploadable;
     double estimate = is_download ? current_progress.download_estimate : current_progress.upload_estimate;
 
+    // If this is an "empty" progress, then there is no need to notify the progress
+    // callback. Wait until there is something "real" to report.
+    if (transferable == 0 && transferred == 0) {
+        return [] {};
+    }
+
     if (!is_streaming) {
         // If the sync client has not yet processed all of the local
         // transactions then the uploadable data is incorrect and we should
@@ -1617,16 +1623,16 @@ SyncProgressNotifier::NotifierPackage::create_invocation(Progress const& current
         // the total number of uploadable bytes available rather than the number of
         // bytes this NotifierPackage was waiting to upload.
         if (!is_download) {
-            estimate = transferable > 0 ? std::min(transfered / double(transferable), 1.0) : 0.0;
+            estimate = std::min(transferred / double(transferable), 1.0);
         }
     }
 
     // A notifier is expired if at least as many bytes have been transferred
     // as were originally considered transferrable.
     is_expired =
-        !is_streaming && (transfered >= transferable && (!is_download || !pending_query_version || estimate >= 1.0));
+        !is_streaming && (transferred >= transferable && (!is_download || !pending_query_version || estimate >= 1.0));
     return [=, notifier = notifier] {
-        notifier(transfered, transferable, estimate);
+        notifier(transferred, transferable, estimate);
     };
 }
 

test/object-store/sync/session/progress_notifications.cpp

@@ -160,12 +160,42 @@ TEST_CASE("progress notification", "[sync][session][progress]") {
         REQUIRE_FALSE(callback_was_called);
     }
 
+    SECTION("callback is not invoked for empty progress") {
+        bool is_streaming = GENERATE(true, false);
+        auto direction = GENERATE(NotifierType::upload, NotifierType::download);
+        bool callback_was_called = false;
+        SECTION("registered when latest progress was empty") {
+            progress.update(0, 0, 0, 0, 1, 1.0, 1.0, 0);
+            progress.register_callback(
+                [&](auto, auto, double) {
+                    callback_was_called = true;
+                },
+                direction, is_streaming, 0);
+            REQUIRE_FALSE(callback_was_called);
+        }
+
+        SECTION("received empty progress update") {
+            progress.register_callback(
+                [&](auto, auto, double) {
+                    callback_was_called = true;
+                },
+                direction, is_streaming, 0);
+            REQUIRE_FALSE(callback_was_called);
+            progress.update(0, 0, 0, 0, 1, 0.0, 0.0, 0);
+            REQUIRE_FALSE(callback_was_called);
+        }
+
+        // Verify notifications are received if not empty
+        progress.update(0, 20, 0, 10, 1, 0.0, 0.0, 0);
+        REQUIRE(callback_was_called);
+    }
+
     SECTION("callback is invoked immediately when a progress update has already occurred") {
         progress.set_local_version(1);
-        progress.update(0, 0, 0, 0, 1, 0.0, 0.0, 0);
+        progress.update(100, 200, 50, 100, 1, 0.5, 0.5, 0);
 
         bool callback_was_called = false;
-        SECTION("for upload notifications, with no data transfer ongoing") {
+        SECTION("for upload notifications") {
             double estimate = 0.0;
             progress.register_callback(
                 [&](auto, auto, double ep) {
@@ -174,18 +204,18 @@ TEST_CASE("progress notification", "[sync][session][progress]") {
                 },
                 NotifierType::upload, false, 0);
             REQUIRE(callback_was_called);
-            REQUIRE(estimate == 0.0);
+            REQUIRE(estimate == 0.5);
         }
 
-        SECTION("for download notifications, with no data transfer ongoing") {
+        SECTION("for download notifications") {
             double estimate = 0.0;
             progress.register_callback(
                 [&](auto, auto, double ep) {
                     callback_was_called = true;
                     estimate = ep;
                 },
                 NotifierType::download, false, 0);
-            REQUIRE(estimate == 0.0);
+            REQUIRE(estimate == 0.5);
             REQUIRE(callback_was_called);
         }
 
@@ -206,7 +236,7 @@ TEST_CASE("progress notification", "[sync][session][progress]") {
     }
 
     SECTION("callback is invoked after each update for streaming notifiers") {
-        progress.update(0, 0, 0, 0, 1, 0.0, 0.0, 0);
+        progress.update(0, 100, 100, 100, 1, 0.0, 1.0, 0);
 
         bool callback_was_called = false;
         uint64_t transferred = 0;
@@ -893,7 +923,6 @@ TEST_CASE("progress notification", "[sync][session][progress]") {
                 TestInputValue{2, 0.6, 900, 1000},
                 TestInputValue{2, 1, 1000, 1000},
             }, {
-                ProgressEntry{0, 0, 0},
                 ProgressEntry{200, 200, 1},
                 ProgressEntry{300, 600, 0.2},
                 ProgressEntry{400, 600, 0.4},