Repository navigation
Editor performance, a data-loss fix, and fixes from a full-app review - #184
Merged
Merged
Conversation
…acters The note body had android:maxLength="20000". A length filter also applies to setText, so a longer note (shared into the app, written in the extended editor, restored from a backup or synced) opened cut short, and the first edit autosaved the cut text over the note: on a device, deleting one character from a 109 KB note left 20,000 characters in the database. Input is no longer limited; SimpleEditorLengthTest pins a 109,000-character note.
…eaner undo Measured on a Pixel 7a with a 109 KB note, typing in the extended editor cost the WebView renderer 48% of a core; a 1 KB note cost 12%. - A plain-text note (from the simple editor, shared into the app, synced) opened as ONE paragraph with a <br> per line, so every keystroke made the browser lay out the whole note again. Each line of text is now a paragraph, as if typed with Enter; blank lines go in front of the next line as <br>. Joined back with newlines the blocks give exactly the text that came in (save.test.mjs, PlainTextBlocksRoundTripTest). - The document went to Android on every Editor.js change: a full save, sanitize and stringify of the note and a copy over the bridge. It now goes once typing pauses (700 ms), at least every 3 s while typing goes on, and at once through flushContent when the screen stops. Android only saves after two quiet seconds anyway. - Undo states kept every document twice, parsed and as JSON, for up to 100 states. They keep the JSON only and parse it when a step is applied.
A block per line made a 1,300-line note take 70 MB more in the WebView renderer (121 MB instead of 50): every Editor.js block holds DOM and a mutation observer of its own. Notes up to 200 lines of text keep a block per line; longer ones are cut into blocks of about 1,000 characters, only between two lines that hold text, so joined back they give exactly the same text and no block starts or ends with a blank line. Pixel 7a, 109 KB / 1,300-line note, mean of three runs, WebView renderer: typing 49.0% -> 17.5% of a core, opening 2.50 s -> 1.96 s of CPU, memory 49.6 MB -> 58.3 MB.
…eystroke The note body is an AppCompatEditText, so its text is an emoji2 SpannableBuilder. After every edit EmojiCompat processes the change and ends its batch with a text-changed report for the WHOLE text, and DynamicLayout then lays out the whole note again. A simpleperf profile of typing put 70% of the main thread in DynamicLayout.reflow. Pixel 7a, typing in the simple editor, app process: 109 KB note: 104% -> 13% of a core 27 KB note: 44% -> 10% of a core EmojiCompat is turned off for the note body (app:emojiCompatEnabled); the system emoji font draws emoji without it. SimpleEditorEmojiCompatTest pins the setting.
… path, client ended on destroy - Play Billing answers are handled on the main thread, so the batch counter and the record of thanked purchases are never touched from two threads. - One path consumes leftover purchases (at start, before a purchase, after ITEM_ALREADY_OWNED). Whether a purchase was thanked is read when its consume finishes, so a purchase the purchase flow thanks meanwhile is not thanked twice, and none is consumed without its thanks. - SupportActivity ends the billing client in onDestroy: after a rotation the old client kept the activity alive and handled purchase updates for it. - A failed product query reports its real response code (billing unavailable, no network) instead of "Product not found". - Tokens of purchases that are gone (refunded, revoked) leave the record; checking a token no longer copies the whole set. - EditableLinkMovementMethod resets the meta state when it swallows the platform's selection crash, so Shift does not stay held. - The notification volume slider stops retrying for the rest of a drag once Do Not Disturb refuses the change, and snaps back when the drag ends.
Keep exports a checklist note's items in listContent and leaves textContent out, so every checklist arrived as an empty note. The items are now written the way the app turns a checklist into plain text, [ ] and [x] per line.
The batched send (700 ms idle, 3 s max wait) widened the window in which a Back press or a screen change could act on an older document, and measured no benefit. flushContent and finishPage keep working as before.
syncNow switched sync back on from the worker thread right before running it, so the isSyncEnabled check SyncService makes under its lock could never see a disconnect that landed while the sync was queued. Enable it on the calling thread instead, where disconnect also runs.
… alive DeleteTagDialog, NameTagDialog and TagOptionsBottomSheet only had argument constructors, so the system could not recreate them after a rotation or process death. They now take their tag via arguments and the options sheet finds its listener on the host. The delete/rename/create writes no longer belong to the dialog's disposables, which dismiss() cleared before the io thread ran them. The 'delete tag and notes' row now appears when the note count arrives, and a failing count no longer crashes.
updateNoteContent touched the sync metadata even when the content UPDATE matched no row because a sync had just applied another device's deletion. That cleared deletedAt, leaving a live record with no note behind it and an updatedAt that outranked the remote tombstone forever. Check for the row in the same transaction, write nothing when it is gone, and return false.
…turning them sideways A 48-108 MP photo was decoded at full size as ARGB_8888, 190-430 MB, and the OutOfMemoryError was not caught. The bounds are read first and the photo is subsampled to at most 4096 px on its long side; running out of memory keeps the original bytes. Re-encoding dropped the EXIF block, and with it the Orientation tag, so portrait photos were stored sideways. The orientation is now applied to the pixels before compressing.
plainTextBlocks put each line into a paragraph unescaped, and the paragraph tool renders it with innerHTML, so a shared note such as <img src=x onerror=...> ran script in a page that can call Android. Escape &, < and > per line. The editor page also no longer needs file access or mixed content: the page comes from file:///android_asset (not governed by setAllowFileAccess) and attachments from the editorjs:// interceptor.
cleanText only decoded , so text like "a < b && c" came back from the extended editor as "a < b && c", and header text was not cleaned at all. Decode entities in one pass for paragraphs, headers and list items.
The sheet's buttons start a write and dismiss in the same click, and dismissing disposed the presenter's subscriptions and dropped the activity, so pin, trash and tag changes could be cancelled before they ran and Copy never opened the copy (its callback also ran off the main thread). The writes now run on their own, and Copy delivers the new id to the activity on the main thread.
The query feeding the cap on published settled versions was a plain UNION, so the cap kept whatever order SQLite's de-duplication left and could drop the newest settlements, which were then offered again. Order it by resolution time, interleave local and remote settlements under the cap, and delete settled rows beyond the number that can ever be published.
The page break was checked once per source line, so a paragraph that wrapped to many lines ran off the bottom of the page, and a word wider than the line was never drawn at all. The break is now checked per drawn line and an overlong word is broken between characters. The PDF, TXT and HTML streams are closed when a write fails.
…e spot Every space or Enter closed the history batch at once, which saves and serializes the whole document: on a long note, on every finished word. A finished word or line now closes its batch after 300 ms; a brief pause still makes it its own step, and Editor.js's late change report does not push the close back.
Deleting a task left its alarm armed, and the receiver posted from the alarm's extras and re-armed repeating reminders, so a deleted task kept notifying every N minutes. Deleting now cancels the alarm, and the receiver loads the task and stops when it is gone, done or has no reminder, re-arming one that sync moved later. The receiver now writes the database inside goAsync() before finishing, so a killed process cannot leave a fired one-time reminder stored as pending, and re-arming after boot skips one-time task reminders missed by more than a day, as for notes.
deleteAttachmentBlockRequest pasted the block id into evaluated JavaScript as-is and escaped the URL only for single quotes. Pass both as JSON string literals.
…reading it The exported VIEW handler read any URI: a missing one threw on a bare thread and crashed the app, and a file:// URI or one of the app's own providers copied its private files into a note. Refuse those, and report any failure while reading instead of crashing.
BackupOptionsDialog only had a constructor taking its callback, so restoring it after a rotation or process death threw InstantiationException. It now keeps its mode in its arguments and asks the hosting activity for the callback when the user chooses. The gathered backup is kept in a process-wide cache, so saving to Drive or to a file picked after a rotation still finds it; when the process was recreated the save reports a failure instead of crashing.
…apply The apply skips a record that moved on locally after the snapshot build read it, but two things defeated that guard: - buildSnapshot read a note's content and only then re-read its updatedAt, so an edit saved in between paired the old content with the new timestamp; the guard let the apply through and the edit was reverted on every device. The timestamp is now read right before the content. - the guard compared the local updatedAt with the merged record's, which is another device's clock whenever the remote version wins. A remote clock ahead let a local edit be overwritten; one behind made an untouched record refuse every newer remote version. It now compares with the updatedAt the build saw for that record.
PDF and HTML exports were built and written on the main thread in the file picker's result, which for a data export of every note could hang the app; the TXT export ran in the background but used the dialog's context after it was dismissed, and toasted off the main thread. All three now build and write on the io thread with the application context and report the outcome on the main thread. The notes lived only in fields, so a dialog recreated while the picker was open wrote an empty '***' file and reported success. The note ids and flags are now arguments, the notes are read back when needed, and an export with nothing left to write reports an error.
For every attachment hash, every sync listed the Drive root for it, read and SHA-256'd the whole local file (bypassing AttachmentHashCache), then listed the root again to verify the remote copy. - SyncStore gains hasVerifiedAttachment; RoomSyncStore answers it from the hash cache the snapshot build already uses, so an unchanged blob is not re-hashed. A full read still produces the precise error when the local copy is bad and there is no remote one to repair it from. - GoogleDriveSyncBackend remembers each candidate listing for the sync and drops it for any root and hash it uploads to, so existence and verification share one listing.
…back The reminder pickers wrote the reminder first and armed the alarm in a main-thread callback that the sheet's onDestroyView cancelled, which could leave a stored reminder with no alarm; dismiss() after onSaveInstanceState also threw. The alarm is now armed (or cancelled) right after the write on the io thread, outside the sheet's disposables, and the sheet closes with dismissAllowingStateLoss() if it is still there.
Saving read every attachment whole into memory to zip it, then read the finished archive whole again to copy it to the picked document. Restoring copied the whole file into a byte array only to look at its first two bytes. Attachments, the JSON and the copy to the document are now streamed in chunks, the archive is sniffed from a stream, and the ZIP stream is closed and the temporary archive removed even when a write fails.
… a rotation Restoring read and unpacked the backup on the main thread after a 3 s timer, and saving built and copied the archive on the main thread from the activity-result callback; both could freeze the screen into an ANR on a large library. Both now run on the io scheduler. A restore wrote notes, tags and trash as three transactions tied to the screen's disposables, so a rotation part-way through left notes restored and tags or trash not, without a word. They are now one transaction, followed by the preferences, and neither a save nor a restore is cancelled by the screen going away: they report to the screen that is current when they end, or to the next one. The Google Keep import dialog could not be cancelled while scanning, and after a rotation the restored dialog spun forever with Back blocked, while a late result hit a null field. It now keeps the file in its arguments, can always be cancelled, and the recreated screen scans again for it.
Every tombstone went into every bundle forever, and the schema caps a bundle at 10,000 records, live and deleted together. A library that had seen enough deletions failed every publish from then on, permanently. A tombstone is now published while its deletion is under 90 days old, and after that only when this sync read a live version of the record from Drive or an open conflict alternative for it is carried along. The rows stay in sync_metadata and are still merged and applied locally, so a device that slept through the horizon and republishes a stale copy gets the tombstone back from the next device that syncs, and the merge deletes the copy as it always would have. The rule is documented on SyncService.forPublication.
NoteActivity put the whole unsaved title and body into the saved-state bundle. With the 20,000-character limit gone, a long note could crash the app with TransactionTooLargeException. Above 64 K characters the draft now stays in memory (RetainedNoteDraft) for the recreated screen instead, as the extended editor does for its document.
…k all of it Clear My Notes deleted whatever the database held when the button was pressed, so a note written or synced in after the hand-off archive was made was deleted although it never reached Encly. The ids the archive carried are now remembered (and kept in the saved state, since the process can be killed while Encly is in front) and only those records and their attachment folders are deleted. The clear was also offered on any successful result, even when Encly reported skipped records that exist nowhere but here; it is now offered only when nothing was skipped.
… hidden The list and the search each subscribed to the full notes query, so every change (each editor autosave included) read every note body twice; the query is now shared. Choosing a tag or a sort order ran the filter and sort of every note on the main thread; the combiner now runs on the computation scheduler. Search and the drawer statistics stop while the main screen is hidden and catch up when it is back. The list itself stays live so it already shows the edit when the editor's return transition starts.
# Conflicts: # app/src/main/java/com/pasich/mynotes/utils/file/FileExportUtils.java
…kes content:// only BackupActivity used the deprecated ShareOptionsDialog constructor. ShareActivity's VIEW filter still advertised file:// URIs, which ShareProcessor now refuses.
pasichDev
changed the title
A declined or failed authorization used to leave sync switched on. The flag is now set from the authorization callback, and only when no disconnect happened since Sync now was tapped (tracked by a process-wide disconnect generation), so a disconnect is still never undone.
…unning Picking a backup or importing from Keep while another restore ran was dropped silently behind the running restore's progress dialog. It now shows a message (new string restore_already_running, all locales).
A long unsaved draft was put into RetainedNoteDraft on every onSaveInstanceState, also when the screen only went to the background. It stayed there, and the next rotation applied it over newer saved content, which autosave then wrote back. The draft is now kept only while the screen is changing configurations, a save of state without a recreation forgets any draft left over, a document the old page hands over after it has finished is ignored, and the handed-over document is kept with the title held at that moment.
updateNote completed successfully when the note row had been deleted meanwhile (a sync applying another device's delete), so the editor showed Saved and every later edit was silently discarded. updateNote now fails with NoteDeletedException when nothing was written, and the note presenter saves the content as a new note and carries on editing that one; a lock keeps two overlapping saves from creating it twice.
…at started it A result that found no attached screen waited in a static for the life of the process and was shown on any later visit to the Backup screen, snackbar and theme reapply included. It is now kept for the starting screen only, identified by a token kept in its saved state, and dropped when that screen is finished rather than recreated.
…was sent retainOnly pruned the thanked record against a queryPurchases answer that can predate a purchase thanked meanwhile; after a failed consume that purchase came back and was thanked a second time. Only tokens recorded when the query was sent are now judged by its answer.
buildSnapshot re-read every metadata row with its own point query after getAll(), thousands of SELECTs per sync. Rows are now re-read 500 at a time with an IN query per record type, still right before that batch's content is read, so a timestamp never postdates the content it pairs with.
clearExported deleted the carried notes, tasks, tags and task lists by id only, so a record edited after the archive was written, here or by a background sync, was deleted with content Encly never received, and its tombstone synced everywhere. The hand-off now records a content fingerprint per record, kept across process death with the ids, and the clear deletes only records whose content still matches; changed ones stay and are counted in the log.
… upload The listing was dropped before the upload, after it, and again in the catch. Nothing can cache it again in between (the upload never lists and callers hold the backend lock), so the single drop up front is enough.
…refused The document picker creates the target file before the app is called back, so refusing a save while another one runs left a 0-byte "backup" behind without a word. Delete it (best effort) and say a save is already running.
An older app version keeps re-adding expired tombstones to every bundle it writes. Compared with those, the pruned snapshot never matched, so both devices republished the whole bundle on every sync. Compare against the remote as this version would publish it instead.
…hase hasVerifiedAttachment flushed the whole hash cache after every attachment, rewriting it N times on a cold cache. The store now flushes once when the sync's attachment phase ends, through a new finishAttachmentVerification hook.
syncNow switched sync on before handing the work to the screen's executor. When the executor had already shut down the sync never ran, yet the flag stayed on. Restore the previous value when the submission is refused; the refusal is reported to the caller as before.
Saving the open note as a new row left the screen's intent, saved state and draft on the deleted id, so a rotation reopened a blank editor, and its attachments still pointed into note_<oldId>. The note is now put back under the same id with the editor's content, and its sync metadata is touched past the tombstone so the edit wins on the next sync.
Keyboard, D-pad and TalkBack changes never start or stop tracking, so a refusal under Do Not Disturb left the slider at a volume not in effect and latched every later change as refused.
An id and a fingerprint for every record went into the saved instance state, which a large library could push past the Binder limit. The snapshot is written to no-backup storage and only its token is saved.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Profiling of both editors on a Pixel 7a, then a review of the whole app (sync, data/backup, UI, editors/security). Every finding below was checked against the code and has a test where practical. Commits are grouped by area; review commit by commit.
Also brings in
e4b10fe(third billing review round), which was pushed to #183 after it was merged.Editors
maxLength="20000"cut longer notes on open and the first edit saved the cut text. Removed; a long draft now goes to a retained ViewModel instead of the saved-state bundle (noTransactionTooLargeException).<img onerror>ran script withwindow.Android); now escaped. File access and mixed content off in the WebView. Block id/URL quoted in evaluated JS. SharedVIEWURIs: only other apps'content://.Sync
SyncService.forPublication.Backup, import, files
[ ]/[x]lines. PDF export breaks pages per drawn line and breaks overlong words.UI
goAsyncand the 24 h grace.Verification
./gradlew spotlessCheck test lint assembleReleaseandnpm test(24) pass locally. Not exercised on a device: the activity-level flows (Back/switch/rotate in the editors, backup restore, dialog recreation).