fix: Prevent multiple Android WebView starts #175
Conversation
This load did not consider the editor configuration, meaning it often loaded the incorrect URL. Additionally, because the editor reveal logic currently only runs on first load, this preload intermittently led to an invisible editor; where the first load revealed the editor and the second, configured load hid the editor. 1. Preload 2. Hide editor 3. Load URL 4. Reveal editor 5. Start editor with configuration 6. Hide editor 7. Load URL
Load and cache not only the manifest, but the editor assets themselves.
It appears we added multiple listeners for the same event at some point. Also, we did not clean up one of the two listeners during tear down.
There was a problem hiding this comment.
Pull Request Overview
This PR prevents multiple Android WebView starts that were causing an invisible editor. The issue occurred when the WebView loaded a URL multiple times during initialization, hiding the editor during the second load without proper reveal logic.
- Removes duplicate editor load listeners and simplifies the initialization flow
- Repurposes the
warmuputility to load WebView and fetch editor assets similar to iOS approach - Eliminates rudimentary preloading from
createAndPreloadWebViewmethod
Comments suppressed due to low confidence (1)
android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt:1
- The warmup method creates a WebView that's automatically recycled after 5 seconds, which may not provide the intended performance benefit if the actual WebView usage happens after this timeout. Consider making the timeout configurable or implementing a more sophisticated lifecycle management approach.
package org.wordpress.gutenberg
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt
Outdated
Show resolved
Hide resolved
| private var configuration: EditorConfiguration = EditorConfiguration.builder().build() | ||
|
|
||
| private val handler = Handler(Looper.getMainLooper()) | ||
| private var editorDidBecomeAvailable: ((GutenbergView) -> Unit)? = null |
There was a problem hiding this comment.
editorDidBecomeAvailable was, at some point, duplicated by editorDidBecomeAvailableListener. We rely upon the latter, so I removed the former.
| private fun createAndPreloadWebView(context: Context): GutenbergView { | ||
| val webView = GutenbergView(context) | ||
| webView.initializeWebView() | ||
| webView.loadUrl(ASSET_URL) |
There was a problem hiding this comment.
This rudimentary URL loading was problematic. First, it did not take into consideration the editor configuration, often resulting in loading the incorrect URL. Second, its loading triggered the editor reveal logic, the editor was then reverted to an invisible state when once start was invoked by the host app.
the benefits of pooling WebViews became moot once we stopped loading a URL during the initialization sequence. It now provides no benefit. Additionally, the pooled WebViews conflicted with the new warmup utilities. It was possible for an editor launch to reuse an in-progress warmup WebView. Once the warmup destroyed its WebView, the editor began failing.
| fun warmup(context: Context, configuration: EditorConfiguration) { | ||
| if (configuration.enableAssetCaching) { | ||
| val library = EditorAssetsLibrary(context, configuration) | ||
| // Preload manifest in background | ||
| warmupScope.launch { | ||
| try { | ||
| library.manifestContentForEditor() | ||
| } catch (e: Exception) { | ||
| Log.e("GutenbergView", "Failed to warmup manifest", e) | ||
| } | ||
| } | ||
| // Cancel any existing warmup | ||
| cancelWarmup() | ||
|
|
||
| // Create dedicated warmup WebView | ||
| val webView = GutenbergView(context) | ||
| webView.initializeWebView() | ||
| webView.start(configuration) | ||
| warmupWebView = webView | ||
|
|
||
| // Schedule cleanup after assets are loaded | ||
| warmupHandler = Handler(Looper.getMainLooper()) | ||
| warmupRunnable = Runnable { | ||
| cleanupWarmup() | ||
| } | ||
| warmupHandler?.postDelayed(warmupRunnable!!, ASSET_LOADING_TIMEOUT_MS) | ||
| } |
There was a problem hiding this comment.
Repurposed this unused warmup utility with an implementation that matches that found for iOS. We rely upon a hidden WebView to perform all the editor requests, allowing the caching mechanism to occur as it normally would—both the manifest and individual assets.
What?
Prevent the WebView loading a URL multiple times during initialization.
Why?
In service of wordpress-mobile/WordPress-Android#22185.
This intermittently led to an invisible editor. The editor was hidden during
the second load, but not revealed again after the second load, as the reveal
logic is only run once.
How?
createAndPreloadWebView.warmuputility to load WebView and fetch editor assets, similar tothe approach found for iOS.
Testing Instructions
See wordpress-mobile/WordPress-Android#22185.
Accessibility Testing Instructions
N/A, no user-facing changes.
Screenshots or screencast
N/A, no user-facing changes.