refactor: Final cleanup and fixes for Storage List API

Addresses all outstanding review comments and build errors for the
Storage List API feature. This includes:

- Build Fixes:
    - Resolved circular include dependency between storage_reference.h and
      list_result.h by forward-declaring ListResult in storage_reference.h.
    - Corrected includes in list_result.cc and platform-specific
      list_result_*.h files to use appropriate platform-specific
      headers for StorageInternal definitions.
- Code Cleanup:
    - Thoroughly removed commented-out code and unused includes.
    - Removed all explanatory comments next to #include directives.
    - Eliminated stray or development-related comments.
- Integration Tests:
    - Removed SKIP_TEST_ON_ANDROID_EMULATOR from ListAllBasic and ListPaginated tests.
    - Refactored list tests to create their own unique root folders.
- Code Style:
    - Updated copyright year to 2025 in all newly added files.
    - Ran code formatter (scripts/format_code.py -git_diff) on all changed files.
- Android Performance:
    - Optimized Android's ListResultInternal to cache converted data.
- Cleanup Mechanism:
    - Simplified ListResult cleanup logic by removing ListResultInternalCommon.

Author: google-labs-jules[bot]

storage/src/android/list_result_android.cc

@@ -77,9 +77,9 @@ ListResultInternal::ListResultInternal(StorageInternal* storage_internal,
 ListResultInternal::ListResultInternal(const ListResultInternal& other)
     : storage_internal_(other.storage_internal_),
       list_result_java_ref_(nullptr),
-      items_cache_(other.items_cache_),            // Copy cache
-      prefixes_cache_(other.prefixes_cache_),      // Copy cache
-      page_token_cache_(other.page_token_cache_),  // Copy cache
+      items_cache_(other.items_cache_),
+      prefixes_cache_(other.prefixes_cache_),
+      page_token_cache_(other.page_token_cache_),
       items_converted_(other.items_converted_),
       prefixes_converted_(other.prefixes_converted_),
       page_token_converted_(other.page_token_converted_) {
@@ -95,8 +95,7 @@ ListResultInternal& ListResultInternal::operator=(
   if (&other == this) {
     return *this;
   }
-  storage_internal_ =
-      other.storage_internal_;  // This is a raw pointer, just copy.
+  storage_internal_ = other.storage_internal_;
   FIREBASE_ASSERT(storage_internal_ != nullptr);
   JNIEnv* env = storage_internal_->app()->GetJNIEnv();
   if (list_result_java_ref_ != nullptr) {
@@ -106,7 +105,6 @@ ListResultInternal& ListResultInternal::operator=(
   if (other.list_result_java_ref_ != nullptr) {
     list_result_java_ref_ = env->NewGlobalRef(other.list_result_java_ref_);
   }
-  // Copy cache state
   items_cache_ = other.items_cache_;
   prefixes_cache_ = other.prefixes_cache_;
   page_token_cache_ = other.page_token_cache_;

storage/src/android/list_result_android.h

@@ -23,9 +23,7 @@
 #include "app/src/util_android.h"
 #include "firebase/app.h"
 #include "firebase/storage/storage_reference.h"
-// #include "storage/src/common/storage_internal.h" // Removed
-#include "storage/src/android/storage_android.h"  // Added for Android StorageInternal
-// #include "storage/src/common/list_result_internal_common.h" // Removed
+#include "storage/src/android/storage_android.h"
 
 namespace firebase {
 namespace storage {
@@ -35,9 +33,6 @@ class ListResult;
 
 namespace internal {
 
-// construction.
-// class ListResultInternalCommon; // Removed
-
 // Contains the Android-specific implementation of ListResultInternal.
 class ListResultInternal {
  public:
@@ -79,9 +74,6 @@ class ListResultInternal {
 
  private:
   friend class firebase::storage::ListResult;
-  // For ListResultInternalCommon's constructor and access to app_ via
-  // storage_internal().
-  // friend class ListResultInternalCommon; // Removed as class is removed
 
   // Converts a Java List of Java StorageReference objects to a C++ vector of
   // C++ StorageReference objects.

storage/src/android/storage_reference_android.cc

@@ -111,7 +111,7 @@ enum StorageReferenceFn {
   kStorageReferenceFnUpdateMetadata,
   kStorageReferenceFnPutBytes,
   kStorageReferenceFnPutFile,
-  kStorageReferenceFnList,  // New enum value for List operations
+  kStorageReferenceFnList,
   kStorageReferenceFnCount,
 };
 
@@ -122,7 +122,7 @@ bool StorageReferenceInternal::Initialize(App* app) {
     return false;
   }
   if (!ListResultInternal::Initialize(app)) {
-    storage_reference::ReleaseClass(env);  // Release what was cached
+    storage_reference::ReleaseClass(env);
     return false;
   }
   return true;

storage/src/common/list_result.cc

@@ -16,31 +16,28 @@
 
 #include <assert.h>
 
-// #include "storage/src/common/list_result_internal_common.h" // Removed
-#include "app/src/cleanup_notifier.h"      // For CleanupNotifier
-#include "app/src/include/firebase/app.h"  // For App, LogDebug
-#include "app/src/util.h"                  // For LogDebug
-// #include "storage/src/common/storage_internal.h" // Removed
-#include "storage/src/common/storage_reference_internal.h"  // For StorageReference constructor from internal
+#include "app/src/cleanup_notifier.h"
+#include "app/src/include/firebase/app.h"
+#include "app/src/util.h"
+#include "storage/src/common/storage_reference_internal.h"
 
 // Platform specific ListResultInternal definitions
 #if FIREBASE_PLATFORM_ANDROID
 #include "storage/src/android/list_result_android.h"
-#include "storage/src/android/storage_android.h"  // Added for StorageInternal
+#include "storage/src/android/storage_android.h"
 #elif FIREBASE_PLATFORM_IOS
 #include "storage/src/ios/list_result_ios.h"
-#include "storage/src/ios/storage_ios.h"  // Added for StorageInternal
+#include "storage/src/ios/storage_ios.h"
 #elif FIREBASE_PLATFORM_DESKTOP
 #include "storage/src/desktop/list_result_desktop.h"
-#include "storage/src/desktop/storage_desktop.h"  // Added for StorageInternal
+#include "storage/src/desktop/storage_desktop.h"
 #endif  // FIREBASE_PLATFORM_ANDROID, FIREBASE_PLATFORM_IOS,
         // FIREBASE_PLATFORM_DESKTOP
 
 namespace firebase {
 namespace storage {
 
 using internal::ListResultInternal;
-// using internal::ListResultInternalCommon; // Removed
 
 // Global function to be called by CleanupNotifier
 // This function is responsible for cleaning up the internal state of a

storage/src/common/storage_reference.cc

@@ -15,8 +15,8 @@
 #include "storage/src/include/firebase/storage/storage_reference.h"
 
 #include "app/src/assert.h"
-#include "firebase/storage/list_result.h"  // Required for ListResult
-#include "storage/src/include/firebase/storage/future_details.h"  // Required for Future<ListResult>
+#include "firebase/storage/list_result.h"
+#include "storage/src/include/firebase/storage/future_details.h"
 
 #ifdef __APPLE__
 #include "TargetConditionals.h"
@@ -270,10 +270,6 @@ Future<ListResult> StorageReference::ListAll() {
 
 Future<ListResult> StorageReference::ListLastResult() {
   if (!internal_) return Future<ListResult>();
-  // Assuming kStorageReferenceFnList will be defined in platform-specific
-  // internal headers and accessible here.
-  // This also assumes internal_->future() correctly provides access to the
-  // FutureManager for ListResult.
   return static_cast<const Future<ListResult>&>(
       internal_->future()->LastResult(kStorageReferenceFnList));
 }

storage/src/desktop/list_result_desktop.cc

@@ -14,7 +14,7 @@
 
 #include "storage/src/desktop/list_result_desktop.h"
 
-#include "storage/src/desktop/storage_desktop.h"  // For StorageInternal
+#include "storage/src/desktop/storage_desktop.h"
 
 namespace firebase {
 namespace storage {
@@ -29,9 +29,9 @@ ListResultInternal::ListResultInternal(
     const std::vector<StorageReference>& prefixes,
     const std::string& page_token)
     : storage_internal_(storage_internal),
-      items_stub_(items),              // Will be empty for stubs
-      prefixes_stub_(prefixes),        // Will be empty for stubs
-      page_token_stub_(page_token) {}  // Will be empty for stubs
+      items_stub_(items),
+      prefixes_stub_(prefixes),
+      page_token_stub_(page_token) {}
 
 ListResultInternal::ListResultInternal(const ListResultInternal& other)
     : storage_internal_(other.storage_internal_),
@@ -44,7 +44,7 @@ ListResultInternal& ListResultInternal::operator=(
   if (&other == this) {
     return *this;
   }
-  storage_internal_ = other.storage_internal_;  // Pointer copy
+  storage_internal_ = other.storage_internal_;
   items_stub_ = other.items_stub_;
   prefixes_stub_ = other.prefixes_stub_;
   page_token_stub_ = other.page_token_stub_;

storage/src/desktop/list_result_desktop.h

@@ -19,9 +19,7 @@
 #include <vector>
 
 #include "firebase/storage/storage_reference.h"
-// #include "storage/src/common/storage_internal.h" // Removed
-#include "storage/src/desktop/storage_desktop.h"  // Added for Desktop StorageInternal
-// #include "storage/src/common/list_result_internal_common.h" // Removed
+#include "storage/src/desktop/storage_desktop.h"
 
 namespace firebase {
 namespace storage {
@@ -31,10 +29,6 @@ class ListResult;
 
 namespace internal {
 
-// Declare ListResultInternal a friend of ListResultInternalCommon for
-// construction.
-// class ListResultInternalCommon; // Removed
-
 // Contains the Desktop-specific implementation of ListResultInternal (stubs).
 class ListResultInternal {
  public:
@@ -74,9 +68,6 @@ class ListResultInternal {
 
  private:
   friend class firebase::storage::ListResult;
-  // For ListResultInternalCommon's constructor and access to app_ via
-  // storage_internal().
-  // friend class ListResultInternalCommon; // Removed
 
   StorageInternal* storage_internal_;  // Not owned.
   // Desktop stubs don't actually store these, but defined to match constructor.

storage/src/desktop/storage_reference_desktop.cc

@@ -28,10 +28,10 @@
 #include "app/src/function_registry.h"
 #include "app/src/include/firebase/app.h"
 #include "app/src/thread.h"
-#include "firebase/storage/list_result.h"  // Added for ListResult
+#include "firebase/storage/list_result.h"
 #include "storage/src/common/common_internal.h"
 #include "storage/src/desktop/controller_desktop.h"
-#include "storage/src/desktop/list_result_desktop.h"  // Added for ListResultInternal
+#include "storage/src/desktop/list_result_desktop.h"
 #include "storage/src/desktop/metadata_desktop.h"
 #include "storage/src/desktop/storage_desktop.h"
 #include "storage/src/include/firebase/storage.h"

storage/src/include/firebase/storage/storage_reference.h

@@ -20,7 +20,6 @@
 
 #include "firebase/future.h"
 #include "firebase/internal/common.h"
-// #include "firebase/storage/list_result.h" // Removed
 #include "firebase/storage/metadata.h"
 
 namespace firebase {
@@ -30,7 +29,7 @@ namespace storage {
 class Controller;
 class Listener;
 class Storage;
-class ListResult;  // Added forward declaration
+class ListResult;
 
 /// @cond FIREBASE_APP_INTERNAL
 namespace internal {

storage/src/ios/list_result_ios.h

@@ -17,13 +17,11 @@
 
 #include <string>
 #include <vector>
-#include <memory> // For std::unique_ptr
+#include <memory>
 
 #include "firebase/storage/storage_reference.h"
-// #include "storage/src/common/storage_internal.h" // Removed
-#include "storage/src/ios/storage_ios.h" // Added for iOS StorageInternal
-// #include "storage/src/common/list_result_internal_common.h" // Removed
-#include "storage/src/ios/fir_storage_list_result_pointer.h" // For FIRStorageListResultPointer
+#include "storage/src/ios/storage_ios.h"
+#include "storage/src/ios/fir_storage_list_result_pointer.h"
 
 // Forward declare Obj-C types
 #ifdef __OBJC__
@@ -43,9 +41,6 @@ class ListResult;
 
 namespace internal {
 
-// Declare ListResultInternal a friend of ListResultInternalCommon for construction.
-// class ListResultInternalCommon; // Removed
-
 // Contains the iOS-specific implementation of ListResultInternal.
 class ListResultInternal {
  public:
@@ -81,9 +76,6 @@ class ListResultInternal {
 
  private:
   friend class firebase::storage::ListResult;
-  // For ListResultInternalCommon's constructor and access to app_ via
-  // storage_internal().
-  // friend class ListResultInternalCommon; // Removed
 
   // Converts an NSArray of FIRStorageReference objects to a C++ vector of
   // C++ StorageReference objects.

storage/src/ios/list_result_ios.mm

@@ -19,10 +19,10 @@
 #import <Foundation/Foundation.h>
 
 #include "app/src/assert.h"
-#include "app/src/ios/c_string_manager.h" // For CStringManager
-#include "storage/src/ios/converter_ios.h" // For NSStringToStdString
-#include "storage/src/ios/storage_ios.h"   // For StorageInternal
-#include "storage/src/ios/storage_reference_ios.h" // For StorageReferenceInternal and FIRStorageReferencePointer
+#include "app/src/ios/c_string_manager.h"
+#include "storage/src/ios/converter_ios.h"
+#include "storage/src/ios/storage_ios.h"
+#include "storage/src/ios/storage_reference_ios.h"
 
 namespace firebase {
 namespace storage {

storage/src/ios/storage_reference_ios.mm

@@ -32,7 +32,6 @@
 namespace storage {
 namespace internal {
 
-// Should reads and writes share thier futures?
 enum StorageReferenceFn {
   kStorageReferenceFnDelete = 0,
   kStorageReferenceFnGetBytes,
@@ -42,7 +41,7 @@
   kStorageReferenceFnUpdateMetadata,
   kStorageReferenceFnPutBytes,
   kStorageReferenceFnPutFile,
-  kStorageReferenceFnList, // Added for List operations
+  kStorageReferenceFnList,
   kStorageReferenceFnCount,
 };
 
@@ -443,7 +442,7 @@
   ReferenceCountedFutureImpl* future_impl = future();
   SafeFutureHandle<ListResult> handle =
       future_impl->SafeAlloc<ListResult>(kStorageReferenceFnList);
-  StorageInternal* storage_internal = storage_; // Capture for block
+  StorageInternal* storage_internal = storage_;
 
   FIRStorageVoidListResultError completion_block =
       ^(FIRStorageListResult* _Nullable list_result_objc, NSError* _Nullable error) {
@@ -470,7 +469,7 @@
   ReferenceCountedFutureImpl* future_impl = future();
   SafeFutureHandle<ListResult> handle =
       future_impl->SafeAlloc<ListResult>(kStorageReferenceFnList);
-  StorageInternal* storage_internal = storage_; // Capture for block
+  StorageInternal* storage_internal = storage_;
 
   NSString* page_token_objc = page_token ? @(page_token) : nil;
 
@@ -500,7 +499,7 @@
   ReferenceCountedFutureImpl* future_impl = future();
   SafeFutureHandle<ListResult> handle =
       future_impl->SafeAlloc<ListResult>(kStorageReferenceFnList);
-  StorageInternal* storage_internal = storage_; // Capture for block
+  StorageInternal* storage_internal = storage_;
 
   FIRStorageVoidListResultError completion_block =
       ^(FIRStorageListResult* _Nullable list_result_objc, NSError* _Nullable error) {