Add Android Preload List

[jkmassel]

What?

Adds an Android preloading system for the Gutenberg editor that prefetches editor dependencies before display, matching the existing iOS implementation.

Why?

The Gutenberg editor requires several API calls and asset downloads during initialization, which can cause slow launch times. This PR introduces a preloading architecture that:

• Pre-fetches API responses (post types, themes, settings) so the editor doesn't need to make network calls during startup
• Downloads and caches JavaScript/CSS asset bundles for faster subsequent launches
• Provides progress feedback to the user during preparation
• Establishes feature parity with the existing iOS preloading system

How?

New Core Components:

• EditorService - Orchestrates loading all editor dependencies with progress tracking
• EditorPreloadList - Pre-fetched API responses injected into the editor JavaScript
• EditorAssetsLibrary (in stores/) - Downloads and caches asset bundles
• EditorHTTPClient - Authenticated HTTP client for WordPress REST API
• RESTAPIRepository - REST API calls with response caching
• EditorURLCache - URL-level caching with configurable policies

New Model Classes:

• EditorAssetBundle - Represents downloaded editor assets
• EditorCachePolicy - Configurable caching strategies (Always, MaxAge, Ignore)
• EditorSettings, EditorDependencies - Editor configuration models
• GBKitGlobal, LocalEditorAssetManifest, RemoteEditorAssetManifest - Asset manifest parsing

Demo App Updates:

• Added SitePreparationActivity and SitePreparationViewModel showing the preloading flow with progress UI

Other Changes:

• Minor iOS test fixes and EditorSettings improvements for consistency
• This PR removes the existing warmup code – a more recent version of androidx.webkit exposes prerenderUrlAsync which we should adopt for this purpose.

Testing Instructions

1. Build and run the Android demo app
2. Select a site configuration
3. Observe the "Site Preparation" screen showing loading progress
4. Tap "Prepare from scratch" to test full loading without cache
5. Tap "Prepare" to test cached loading
6. Verify the editor loads correctly with preloaded data
7. Run make test-android to verify all new unit tests pass

See the tests in #250 for examples of tests sites you can use.

[adalpari]

⛏️Aren't we using injection in the example project? ViewModels can be simply injected.

[jkmassel]

I don't think we are at the moment – I'd be happy to adopt it, but I think it's outside the scope of this PR :)

[adalpari]

Do we want to track the exception here?

[jkmassel]

In this case probably not, because we'll treat a cache failure as a simple cache miss. I've modified this to use runCatching in c64592b, which should hopefully make the intent more clear!

[dcalhoun]

@claude

[claude]

Claude finished @dcalhoun's task in 3m 13s —— View job

---

Comprehensive Code Review Complete ✅

I've completed a thorough review of PR #260: Add Android Preload List. This is an excellent implementation that brings feature parity with iOS. Below is my detailed analysis across all requested areas.

---

Executive Summary

Overall Assessment: ✅ Approved with Minor Recommendations

This is a well-architected, thoroughly tested implementation. The code quality is high, security practices are sound, and the test coverage is excellent. I identified a few minor areas for potential improvement, but none are blocking issues.

Key Strengths:

• Comprehensive test coverage (19 test files with 5,000+ lines of tests)
• Well-documented code with clear KDoc comments
• Strong separation of concerns and modular architecture
• Excellent error handling throughout
• Thoughtful caching strategies with configurable policies

---

1. Code Quality ✅

Strengths

• Clean architecture: Clear separation between services, stores, models, and UI
• Consistent naming: Follows Kotlin conventions and project standards
• Well-documented: Comprehensive KDoc comments on all public APIs
• DRY principles: Good reuse of common functionality (e.g., EditorHTTPClient, EditorURLCache)
• Immutability: Models use data class and immutable properties where appropriate

Observations

Function ordering follows project conventions (android/Gutenberg/src/main/java/org/wordpress/gutenberg/services/EditorService.kt:260-374):
The code correctly orders functions by usage/call order rather than alphabetically, as specified in CLAUDE.md. Main methods like prepare() come first, followed by helper methods in the order they're called.

Excellent use of companion objects for factory methods (android/Gutenberg/src/main/java/org/wordpress/gutenberg/services/EditorService.kt:71-169):
The create() and createForTesting() factory methods provide a clean API and separate production from test concerns.

Minor: Consider extracting magic numbers (android/Gutenberg/src/main/java/org/wordpress/gutenberg/services/EditorService.kt:73):

private const val CLEANUP_INTERVAL_SECONDS = 86_400L // 1 day

This is good, but there are a few other magic numbers that could benefit from named constants:

• requestTimeoutSeconds: Long = 60 in EditorHTTPClient.kt:127
• Progress weight values in DependencyWeights enum are well-named ✅

---

2. Security 🔒

Strengths

• Path traversal protection (android/Gutenberg/src/main/java/org/wordpress/gutenberg/model/EditorAssetBundle.kt:136-151):

  val resolvedFile = File(bundleRoot, urlPath).canonicalFile
  val bundleRootCanonical = bundleRoot.canonicalFile
  
  require(
      resolvedFile.path.startsWith(bundleRootCanonical.path + File.separator) ||
          resolvedFile.path == bundleRootCanonical.path
  ) {
      "Asset path escapes bundle root: $urlPath"
  }

  Excellent prevention of path traversal attacks when storing downloaded assets ✅

• URL validation for assets (android/Gutenberg/src/main/java/org/wordpress/gutenberg/stores/EditorAssetsLibrary.kt:240-261):
  Only HTTP/HTTPS URLs with specific extensions (.js, .css, .js.map) are allowed. This prevents arbitrary file downloads.

• Authorization header handling: The authHeader is properly passed through all HTTP requests and not logged

• Cache key hashing (android/Gutenberg/src/main/java/org/wordpress/gutenberg/stores/EditorURLCache.kt:214-219):
  Uses SHA-256 to hash cache keys, preventing cache key collisions and path traversal via cache keys.

Recommendations

Consider sanitizing logging output (android/Gutenberg/src/main/java/org/wordpress/gutenberg/EditorHTTPClient.kt:154, 201):

Log.e(TAG, "HTTP error downloading $url: $statusCode")
Log.e(TAG, "HTTP error fetching $url: $statusCode")

URLs might contain sensitive query parameters (tokens, etc.). Consider redacting query params in logs:

Log.e(TAG, "HTTP error downloading ${url.substringBefore('?')}: $statusCode")

WordPress error parsing is defensive (android/Gutenberg/src/main/java/org/wordpress/gutenberg/EditorHTTPClient.kt:227-243) ✅:
The tryParseWPError() method safely handles malformed JSON without exposing error details to potential attackers.

---

3. Performance ⚡

Strengths

Concurrent downloads (android/Gutenberg/src/main/java/org/wordpress/gutenberg/stores/EditorAssetsLibrary.kt:194-211):

coroutineScope {
    val downloads = assets.map { assetUrl ->
        async {
            fetchAsset(assetUrl, bundle)
            mutex.withLock {
                completed++
                progress?.invoke(EditorProgress(completed = completed, total = assets.size))
            }
        }
    }
    downloads.awaitAll()
}

Excellent parallel downloading of assets with proper progress tracking ✅

Smart caching strategy:

• Three-tier caching: EditorURLCache for API responses, EditorAssetsLibrary for assets, in-memory for preload list
• Configurable cache policies (Always, MaxAge, Ignore) allow flexibility
• Automatic cleanup of old assets (android/Gutenberg/src/main/java/org/wordpress/gutenberg/stores/EditorAssetsLibrary.kt:276-286)

Memory-conscious design (android/Gutenberg/src/main/java/org/wordpress/gutenberg/EditorHTTPClient.kt:190-194):

// Note: This loads the entire response into memory. This is acceptable because
// this method is only used for WordPress REST API responses (editor settings, post
// data, themes, etc.) which are expected to be small (KB range). Large assets like
// JS/CSS files use the download() method which streams directly to disk.

Great comment explaining the design decision. Large files are streamed to disk, small API responses are loaded into memory.

Progress tracking is granular (android/Gutenberg/src/main/java/org/wordpress/gutenberg/services/EditorService.kt:57-68):
Weighted progress reporting gives users accurate feedback on long-running operations.

Observations

Thread-safe caching (android/Gutenberg/src/main/java/org/wordpress/gutenberg/stores/EditorURLCache.kt:30):
Uses ReentrantReadWriteLock for thread-safe cache access ✅

Potential optimization: Consider adding a memory cache layer for frequently accessed items in EditorURLCache. Currently, every cache read parses JSON from disk. For hot paths (like editor settings), an in-memory LRU cache could reduce I/O.

---

4. Testing 🧪

Strengths

Comprehensive coverage: 19 test files covering all major components:

• Core services: EditorServiceTest, EditorHTTPClientTest, RESTAPIRepositoryTest
• Stores: EditorAssetsLibraryTest (764 lines!), EditorURLCacheTest (595 lines!)
• Models: 11 model test files covering all data classes
• Integration: Tests use MockWebServer for realistic HTTP testing

Excellent test quality (android/Gutenberg/src/test/java/org/wordpress/gutenberg/EditorHTTPClientTest.kt):

• Clear test names following Given-When-Then pattern
• Good edge case coverage (unicode, error cases, empty data)
• Proper use of test fixtures and helpers
• Mock implementations for dependencies

Test utilities (android/Gutenberg/src/test/java/org/wordpress/gutenberg/services/EditorServiceTest.kt:133-221):
The EditorServiceMockHTTPClient provides a reusable mock implementation that's easy to understand and maintain.

Observations

Test coverage appears excellent: Every new class has a corresponding test file. The tests cover:

• ✅ Happy paths
• ✅ Error cases
• ✅ Edge cases (empty data, null handling)
• ✅ Concurrency scenarios
• ✅ Serialization/deserialization

Missing: Integration tests for the full prepare → load flow. While unit tests are comprehensive, an end-to-end test that exercises the full flow from EditorService.prepare() through GutenbergView loading would provide additional confidence. Consider adding this in a follow-up.

---

5. Documentation 📚

Strengths

Excellent KDoc coverage: All public APIs have clear documentation with:

• Purpose and behavior descriptions
• Parameter documentation
• Return value descriptions
• Usage examples (e.g., android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt:46-84)

PR description is exemplary: The PR body clearly explains:

• What was added
• Why it was needed
• How it works
• Testing instructions

Code comments explain "why" not "what":

// Note: This loads the entire response into memory. This is acceptable because...

Comments focus on design decisions and rationale, not just describing code.

Recommendations

README update needed: The main README doesn't appear to have been updated to document the new preloading system. Consider adding:

• A section on EditorService and how to use it
• Examples of preloading dependencies before creating a GutenbergView
• Performance benefits of preloading

Demo app is well-implemented: The SitePreparationActivity serves as excellent documentation-by-example for how to use the preloading system.

---

Specific File Reviews

EditorService.kt ⭐ Excellent

• Clean separation of concerns
• Good progress tracking implementation
• Automatic cleanup scheduling is clever (onceEvery helper)
• Minor: The onceEvery method uses SharedPreferences, which could fail silently if null. Consider logging when SharedPreferences is unavailable (line 384)

EditorHTTPClient.kt ✅ Good

• Solid error handling with typed errors
• Timeout configuration is appropriate
• WP error parsing is defensive
• Minor logging concern mentioned in Security section

EditorAssetsLibrary.kt ⭐ Excellent

• Concurrent downloads with proper error handling (fail-fast is correct)
• Atomic bundle moves prevent partial bundles
• Cleanup logic is safe and tested
• Bundle validation is thorough

EditorURLCache.kt ✅ Good

• Thread-safe implementation with ReentrantReadWriteLock
• SHA-256 cache keys prevent collisions
• Clean() method for expired entries is well-designed
• Consider: In-memory cache layer for hot paths (mentioned in Performance)

GutenbergView.kt ✅ Good

• Well-documented constructor with coroutine scope ownership clearly explained
• Fast path for pre-loaded dependencies is smart
• Lifecycle management looks correct
• Excellent improvement: The previous review comments about coroutine scope injection were addressed (lines 84, 118)

Model Classes ✅ Good

• All models are well-structured data classes
• Proper use of kotlinx.serialization
• Good companion object factories
• Path traversal protection in EditorAssetBundle is excellent

---

Addressing Previous Review Comments

All previous review comments from @adalpari have been properly addressed:

• ✅ Nullability issues fixed (EditorDependenciesSerializer)
• ✅ Kotlin idioms applied (using orEmpty())
• ✅ Coroutine scope injection implemented
• ✅ Error handling clarified with runCatching

---

Final Recommendations

Must Address

None - no blocking issues found.

Should Consider

1. Add integration test: End-to-end test for prepare → load flow
2. Update README: Document the new preloading system
3. Sanitize URLs in logs: Redact query parameters in error logs to prevent credential leaks

Nice to Have

4. In-memory cache layer: Add LRU cache for hot paths in EditorURLCache
5. Metrics: Consider adding telemetry for cache hit rates, download times
6. Named constants: Extract remaining magic numbers (timeout values)

---

Conclusion

This is an excellent implementation that I'm happy to approve. The code quality is high, the architecture is sound, testing is comprehensive, and the security practices are solid. The recommendations above are minor improvements that don't block merging.

The PR successfully achieves its goals:

• ✅ Feature parity with iOS preloading system
• ✅ Improved editor startup performance
• ✅ Clean, maintainable architecture
• ✅ Comprehensive test coverage
• ✅ Good documentation

Great work! 🎉

---

[claude]

Security consideration: URLs might contain sensitive query parameters (auth tokens, etc.). Consider sanitizing URLs in logs to prevent credential leakage:

val sanitizedUrl = url.substringBefore('?')
Log.e(TAG, "HTTP error downloading $sanitizedUrl: $statusCode")

This applies to lines 154, 201, and any other places where URLs are logged.

[claude]

Security: Excellent path traversal protection ✅

This code properly prevents directory traversal attacks by:

1. Canonicalizing both the resolved file and bundle root
2. Checking that the resolved path is within the bundle root
3. Throwing an exception if the path escapes the bundle

This is a critical security control for handling untrusted asset URLs.

[claude]

Performance: Excellent concurrent downloads ⚡

This implementation properly:

• Downloads assets in parallel using async
• Protects shared state (progress) with a mutex
• Uses awaitAll() to fail-fast if any download fails
• Reports granular progress to the user

The fail-fast approach (line 196 comment) is the right choice - a partial bundle would result in a broken editor.

[claude]

Security: Good cache key design ✅

Using SHA-256 to hash method:url prevents:

• Cache key collisions
• Path traversal via cache keys (URLs could contain ../)
• Information leakage through predictable cache file names

Consider: For additional security, you could include the site ID in the hash to ensure complete isolation between different sites.

[claude]

Minor: Silent failure when SharedPreferences is null

If sharedPreferences is null (line 384), the onceEvery logic silently returns without executing the work. This is fine for the cleanup() use case, but consider logging when this happens to aid debugging:

private fun onceEvery(intervalSeconds: Long, handle: String, work: () -> Unit) {
    val prefs = sharedPreferences
    if (prefs == null) {
        Log.d(TAG, "SharedPreferences not available, skipping onceEvery for: $handle")
        return
    }
    // ... rest of implementation
}

[claude]

Documentation: Excellent coroutine scope ownership explanation ✅

This documentation clearly explains:

• The caller owns the coroutine scope
• Why lifecycle-aware scopes should be used
• That the view doesn't cancel the scope

This addresses the earlier review concern from @adalpari and prevents lifecycle bugs. Great improvement!

[dcalhoun]

This look really sound—nicely done!

I spent most of my time reviewing the logic itself, and little time on the automated tests.

I tested the functionality in the demo app. Testing went well. The performance improvement is notable on sites like slow.wpmt.co. For other sites, there a notable delay remains during editor start (after preloading) that presumably stems from time spent loading editor/plugin assets. My understanding is that we might further optimize those in the future with things like referenced prerenderUrlAsync.

I feel Claude's review is accurate, and the inline comments are likely worth considering. Outside of that, I noted a few oddities in my own inline comments to consider. None of these appear as blockers, some only impact the demo app.

@jkmassel let me know what you think about Claude's and my review and if you'd like for me to approve as-is.

---

An aside: with the editor settings (background color) now applied to the Android demo app, I noted the editor height is unexpectedly cut off. I addressed this in #275.

[dcalhoun]

This doesn't appear to correctly denote the presence of caches created from loading the editor (i.e., without using the dedicated "prepare" buttons). It also does not recognize old caches—navigating here, preparing a cache, navigating back to the root view, and re-navigating here results in an inaccurate "editor dependencies missing."

Additionally, in the latter scenario, the loading view incorrect displays "Loading Editor" rather than "Starting Editor." The progress bar displayed also never updates after initial render in this context.

Unrecognized preload caches recording

gbk-android-preloading-cache.mp4

[dcalhoun]

I perceive this makes these values required by the editor configuration. The optional values were useful for using the block editor for non-site-specific contexts in the future—e.g., editing comment text in the Jetpack app, editing entries in the Day One app. However, I perceive this can still be accomplished with something like the following. 👍🏻

val config = EditorConfiguration.bundled()
      .toBuilder()
      .setContent("my content here")
      .build()

[dcalhoun]

Why might this and other similar lines use a string "GET" rather than EditorHttpMethod.GET? This doesn't appear to use the internal helper—should it?

[dcalhoun]

There is an unexpected artifact on the far right of the progress bar (a detached, filled dot at the 100% mark). It's present when the bar first renders, and while it progressively fills. Not sure why this occurs.

Progress bar artifact screenshot