feat: Recover content on WebView refresh via pull model #283
Conversation
|
Claude finished @dcalhoun's task in 1m 53s —— View job Code Review Complete ✅This PR implements a solid pull-based content recovery mechanism. The implementation is well-structured across all three platforms with comprehensive test coverage. Below is my detailed review. Overall AssessmentStrengths:
Architecture: The pull model is the right approach here. Having the web layer request content rather than push it eliminates timing issues and makes the recovery mechanism more reliable. Detailed Findings1. Code Quality
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
android/Gutenberg/src/main/java/org/wordpress/gutenberg/GutenbergView.kt
Show resolved
Hide resolved
dcalhoun
force-pushed
the
feat/restore-content-via-pull
branch
from
55471c3 to
e8dd9ff
Compare
| // Since we don't use the auto-save functionality, draft posts need to have an ID. | ||
| // We assign a temporary ID of -1. |
There was a problem hiding this comment.
It remains true we need a -1 fallback ID for draft posts, but this appears unrelated to auto-save. We now use the AutosaveMonitor in the GutenbergKit project.
For posterity, this comment was first added in 444de49.
dcalhoun
force-pushed
the
feat/restore-content-via-pull
branch
from
3703a0c to
dd58448
Compare
| return { | ||
| id: -1, | ||
| type: 'post', | ||
| status: 'auto-draft', |
There was a problem hiding this comment.
The auto-draft post status is specific to the WordPress server and not recognized/utilized by the WordPress mobile apps. Also, using auto-draft results in emptying populated post titles. Therefore, a more sensible default is draft.
|
@dcalhoun I can approve this from the Android site of things once the conflicts are resolved. |
Expose function for retrieving the latest content from the native host. This is useful for ensuring the content is updated after WebView refresh or re-initialization.
Enable the editor to request the latest content from the host.
Enable the editor to request the latest content from the host.
Ensure the editor always uses the latest content. This particularly important for subsequent initialization events--e.g., WebView refresh.
When crossing actor boundaries, Swift requires type to conform to `Sendable`, which `Any?` cannot. Constructing the dictionary outside of the `MainActor` run avoids this incompatibility.
Improve visibility of critical errors.
Mitigate crashes from unexpected characters.
There isn't an explicit preference, but a result of arbitrary code ordering.
Note the draft post fallback.
Ensure the fallback status matches across the various scenarios for sourcing post content.
The `auto-draft` post status is unique to the WordPress server; it represents saves not performed by the user, but by the auto-save functionality. This concept is not present in the WordPress mobile apps. Using auto-draft resulted in emptying of post title content due to client-side filters in Gutenberg core.
The name is more explicit and matches other fields like `postId`. This also improves cross-platform alignment.
Structure the `postStatus` field in the same manner across platforms for consistency.
dcalhoun
force-pushed
the
feat/restore-content-via-pull
branch
from
3110b00 to
fa4ac86
Compare
|
@nbradbury I rebased this addressing merge conflicts and added three additional commits to update this branch to match the latest GBKit configuration structure and improve alignment across platforms: |
There was a problem hiding this comment.
Seems reasonable.
How does the web view know when to reload if the process crashes? The other parts of the app use the following delegate method: webViewWebContentProcessDidTerminate in which the native sides triggers the reload and has control over it. There is no other ways to trigger the page refresh in production AFAIK.
|
|
||
| /// Hiding the conformances, and breaking retain cycles. | ||
| private final class GutenbergEditorController: NSObject, WKNavigationDelegate, WKScriptMessageHandler { | ||
| private final class GutenbergEditorController: NSObject, WKNavigationDelegate, WKScriptMessageHandler, WKScriptMessageHandlerWithReply { |
There was a problem hiding this comment.
TIL WKScriptMessageHandlerWithReply – its' really nice.
jkmassel
left a comment
jkmassel
left a comment
There was a problem hiding this comment.
Tested this alongside iOS and it worked great.
@kean you are correct. GutenbergKit doesn't currently implement Do you feel we should also add
JavaScript from plugins could call |
I'm not sure if that's what happens when
It might happen automatically if
That's a fair point. Doesn't web view retain the memory state in the case? Does it need to call the native side? |
Align with Swift naming practices.
Improve the robustness of content restoration in the event the `WKWebView` is terminated on iOS due to memory pressure. In testing by manually terminating the `com.apple.WebKit.WebContent` task in the Activity Monitor, the editor recovered without these changes, at least the first time. However, terminating the task more than once led to an empty WebView for the editor. Implementing `webViewWebContentProcessDidTerminate` seems to improve the robustness. Terminating the `com.apple.WebKit.WebContent` task multiple times now always leads to the editor restoring the latest content.
I've been unable to identify an approach to test that specific scenario. That is why I used the terminate the That said, I did find now that terminating the process multiple times will lead to an empty WebView. Implementing
No. It seems the React app runs again, populating the editor with the stale |
Given the two approvals received and having addressed @kean's initial feedback quoted above in ba98f11, I plan to merge this and publish a release for integrating into the next WordPress-iOS release. I'm happy to follow up on further feedback in new PRs. 🙇🏻♂️ |
What?
Adds a pull-based content recovery mechanism that allows the web editor to request the latest content from the native host during initialization.
Why?
Ref CMM-1123.
When the WebView reinitializes (due to OS memory pressure or page refresh), the editor previously loaded stale content from
window.GBKit.postwhich was injected at original load time. The host apps have fresher content from autosave events (~1s intervals), but this wasn't being used on WebView recovery.Related WordPress app PRs:
How?
Implements a pull model where the web editor always requests content from the native host during initialization:
src/utils/bridge.js): AddedrequestLatestContent()function that calls the appropriate native bridge methodEditorViewController.swift): ImplementsWKScriptMessageHandlerWithReplyto respond to content requests via a new delegate methodeditorRequestsLatestContentGutenbergView.kt): AddsLatestContentProviderinterface and@JavascriptInterfacemethod that returns JSON contentsrc/utils/editor.jsx):getPost()now requests content from native host first, falling back towindow.GBKit.postonly when bridge is unavailableAlternatives considered:
Testing Instructions
chrome://inspectAccessibility Testing Instructions
No UI changes - this is a data recovery mechanism that happens during initialization.
Screenshots or screencast
N/A - No visual changes
🤖 Generated with Claude Code