[incfs] Fix the mount state callbacks processing

- use a never-existing storage ID as a job key
- order the jobs in the map to not skip them on changes, or,
  worse, never hang in a loop
- clear the local callbacks vector before moving to the next
  storage ID
- try to resume from the closest place on the next processing
  iteration

Bug: 183435580
Test: atest service.incremental_test
Change-Id: I36cd5d30c656bed62c20bd7a7f84fb58046a0933

1 files changed, 25 insertions, 18 deletions

diff --git a/services/incremental/IncrementalService.cpp b/services/incremental/IncrementalService.cpp
index 217b621e54ab..9bb2f041556a 100644
--- a/services/incremental/IncrementalService.cpp
+++ b/services/incremental/IncrementalService.cpp
@@ -2272,7 +2272,7 @@ void IncrementalService::addIfsStateCallback(StorageId storageId, IfsStateCallba
         mIfsStateCallbacks[storageId].emplace_back(std::move(callback));
     }
     if (wasEmpty) {
-        addTimedJob(*mTimedQueue, kMaxStorageId, Constants::progressUpdateInterval,
+        addTimedJob(*mTimedQueue, kAllStoragesId, Constants::progressUpdateInterval,
                     [this]() { processIfsStateCallbacks(); });
     }
 }
@@ -2288,29 +2288,36 @@ void IncrementalService::processIfsStateCallbacks() {
             }
             IfsStateCallbacks::iterator it;
             if (storageId == kInvalidStorageId) {
-                // First entry, initialize the it.
+                // First entry, initialize the |it|.
                 it = mIfsStateCallbacks.begin();
             } else {
-                // Subsequent entries, update the storageId, and shift to the new one.
-                it = mIfsStateCallbacks.find(storageId);
+                // Subsequent entries, update the |storageId|, and shift to the new one (not that
+                // it guarantees much about updated items, but at least the loop will finish).
+                it = mIfsStateCallbacks.lower_bound(storageId);
                 if (it == mIfsStateCallbacks.end()) {
-                    // Was removed while processing, too bad.
+                    // Nothing else left, too bad.
                     break;
                 }
-
-                auto& callbacks = it->second;
-                if (callbacks.empty()) {
-                    std::swap(callbacks, local);
+                if (it->first != storageId) {
+                    local.clear(); // Was removed during processing, forget the old callbacks.
                 } else {
-                    callbacks.insert(callbacks.end(), local.begin(), local.end());
-                }
-                if (callbacks.empty()) {
-                    it = mIfsStateCallbacks.erase(it);
-                    if (mIfsStateCallbacks.empty()) {
-                        return;
+                    // Put the 'surviving' callbacks back into the map and advance the position.
+                    auto& callbacks = it->second;
+                    if (callbacks.empty()) {
+                        std::swap(callbacks, local);
+                    } else {
+                        callbacks.insert(callbacks.end(), std::move_iterator(local.begin()),
+                                         std::move_iterator(local.end()));
+                        local.clear();
+                    }
+                    if (callbacks.empty()) {
+                        it = mIfsStateCallbacks.erase(it);
+                        if (mIfsStateCallbacks.empty()) {
+                            return;
+                        }
+                    } else {
+                        ++it;
                     }
-                } else {
-                    ++it;
                 }
             }
 
@@ -2330,7 +2337,7 @@ void IncrementalService::processIfsStateCallbacks() {
         processIfsStateCallbacks(storageId, local);
     }
 
-    addTimedJob(*mTimedQueue, kMaxStorageId, Constants::progressUpdateInterval,
+    addTimedJob(*mTimedQueue, kAllStoragesId, Constants::progressUpdateInterval,
                 [this]() { processIfsStateCallbacks(); });
 }