[web] Preload encrypted media
ClosedPublic
Actions

Authored by bartek on May 31 2023, 11:58 PM.

Details

Reviewers

atul
ashoat

Commits

rCOMMd392c7c61165: [web] Preload encrypted media

Summary

Resolves ENG-3679.
Used fetch to preload encrypted media. The current preloadImage function used to preload them by creating a dummy <img /> element and setting the src property, which was not suitable for encrypted media.

The introduced function in fact preloads any request, including images, but I left the original because browser might have some optimizations for images. Also it is used in other places in the codebase e.g. to get the dimensions.

Test Plan

Entered an encrypted media chat and selected a photo to send. Observed the Dev Tools Network tab to see that the encrypted media was preloaded:

Note the two subsequent requests to b281...3d58. The first one is sent from the preload function (media-utils.js, 251ms), while the other is read from disk-cache. Remember to uncheck the "Disable cache" option in devtools.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

bartek created this revision.May 31 2023, 11:58 PM

bartek held this revision as a draft.

Herald added a subscriber: tomek. · View Herald TranscriptMay 31 2023, 11:58 PM

bartek added a child revision: D8051: [web] Fix preloading blob service URIs.Jun 1 2023, 12:03 AM

bartek edited the test plan for this revision. (Show Details)Jun 1 2023, 12:07 AM

bartek attached a referenced file: F567178: Screenshot 2023-06-01 at 08.35.07.png. (Show Details)Jun 1 2023, 12:12 AM

bartek published this revision for review.Jun 1 2023, 12:17 AM

bartek added inline comments.

web/media/media-utils.js
72 ↗	(On Diff #27335)	Typo, I'll fix this before landing

Harbormaster completed remote builds in B19947: Diff 27335.Jun 1 2023, 12:18 AM

Two concerns:

Unlike native, it doesn't appear that we are waiting on the preload for anything. On native, we show the local URI until the preload completes. On web, for non-encrypted media we delete the local URI when the preload completes... but we don't wait for the preload to start showing the new URI, do we? It seems like we're likely to have a "flicker" still after landing this diff.
It seems like there are two parts we could consider for a "preload": download and decrypt. To perfectly avoid a "flicker", it feels like we'd also probably want to wait for the decrypt before showing the remote URI (but this would require 1 to be done first).

Assuming you agree with my assessment, could you create a task (or multiple tasks) for these before landing?

web/media/media-utils.js
61 ↗	(On Diff #27335)	Typo

This revision is now accepted and ready to land.Jun 1 2023, 7:58 AM

Fix typos. Rebase before landing

In D8050#238830, @ashoat wrote:

Two concerns:

Unlike native, it doesn't appear that we are waiting on the preload for anything. On native, we show the local URI until the preload completes. On web, for non-encrypted media we delete the local URI when the preload completes... but we don't wait for the preload to start showing the new URI, do we? It seems like we're likely to have a "flicker" still after landing this diff.

It seems like there are two parts we could consider for a "preload": download and decrypt. To perfectly avoid a "flicker", it feels like we'd also probably want to wait for the decrypt before showing the remote URI (but this would require 1 to be done first).

Assuming you agree with my assessment, could you create a task (or multiple tasks) for these before landing?