Lazyload iframes

[domfarolino]

Fixes #5496.

• At least two implementers are interested (and none opposed):
  • Chromium
  • Firefox
  • Safari
• Tests are written and can be reviewed and commented upon at:
  • Tentative tests can be found at https://github.com/web-platform-tests/wpt/tree/master/html/semantics/embedded-content/the-iframe-element. Here's what we still need:
  • Test that the a deferred iframe is fetched with the referrer policy computed at load-deferral time
  • Test that a deferred iframe is fetched with a URL that was parsed against
  • Test generic lazy load behavior (i.e., below-viewport loading=lazy iframes do not load)
  • Test generic lazy load eager behavior
  • Test loading=lazy on srcdoc iframes
  • Test that below-viewport loading=lazy iframes can be lazy loaded multiple times, and that a document in an iframe isn't unloaded until the second time the ready to be lazy loaded flag is loaded
  • From discussion with @annevk, test that setting loading=eager and then immediately removing the attribute still continues the load (we need this for images too)
  • Test that setting loading=eager on a deferred iframe correctly sets the ready to be lazy loaded flag and continues the load
  • Test that in-viewport loading=lazy iframes do not block the outer window load event
• Implementation bugs are filed:
  • Chrome: N/A
  • Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1622090
  • Safari: https://bugs.webkit.org/show_bug.cgi?id=196698

Aside: One thing I noticed about #5510 is that I think it didn't fully fix #5236. I think it's still weird that we're waiting in parallel for the ready to be lazy loaded bool to be mutated on the main thread, and then we queue a task from in-parallel to continue. IMO one of the big wins of using an IntersectionObserver is that we can use the callback to just synchronously finish the loading right from there. I'll start another thread with my latest proposed design to fix this, but for now, the iframe spec behaves similar to images.

---

/embedded-content.html ( diff )
/iframe-embed-object.html ( diff )
/images.html ( diff )
/indices.html ( diff )
/urls-and-fetching.html ( diff )

[annevk]

As specified it doesn't account for srcdoc, meaning that if srcdoc specifies further subresources, those all end up delaying the load event. That seems less than ideal. I think a primary use case for this feature is to make the load event fire faster. (It seems fine to not account for when neither srcdoc nor src is specified.)

The other problem I foresee here is that navigation obtains certain state at which point we run into nondeterminism in the sense that it matters when the user scrolls the page. This already exists to some extent, e.g., with the sandbox attribute, and this will enhance that.

cc @clelland

[domenic]

@domfarolino asked me to look at this to get a second set of eyes on the concern

The other problem I foresee here is that navigation obtains certain state at which point we run into nondeterminism in the sense that it matters when the user scrolls the page. This already exists to some extent, e.g., with the sandbox attribute, and this will enhance that.

I tried to get an exhaustive list of all the mutable state that affects navigation. They are:

• Handled in the "otherwise steps"
  • container src="" attribute
  • embedder document base URL (affects URL parsing)
  • ancestor tree active document URLs (to avoid recursion)
  • container referrerpolicy="" attribute
• Handled in the "navigate" algorithm
  • sandboxing flags of the embedder and of the container (impacts both "allowed to navigate" and the creation sandboxing flags of the new document)
• Handled in "process a navigate fetch"
  • CSP of embedder

Given how non-uniformly these are handled right now, my intuition is that the cleanest thing to do is to proceed as this PR does, by making minimal changes to the "navigate" algorithm and instead doing the deferral in the "otherwise steps". That leaves us better equipped to clean these up in the manner suggested in #4926 later.

That is, if we go mucking around inside the navigation algorithm (by e.g. moving the deferral until right before "process a navigate fetch"), that will make future refactoring into an up-front policy-container harder. (Or at the very least we'd have to move the deferral back up into the otherwise steps.) And we'd end up having to duplicate a lot of the checks in "navigate" after the deferral runs its course, since they are important for security.

So if we go with this PR as-is, this would give us the following behavior for images vs. iframes. Here I used "loading-started" to refer to the beginning of the relevant loading algorithm (usually right after the element gets inserted into the document), and "loading-committed" to refer to the point after which we're about to fetch from the network (e.g., once the user has scrolled down enough).

• <img>:
  • Snapshotted at loading-started: embedder document base URL, referrerpolicy="", crossorigin=""
  • Causes a re-fetch: src="", referrerPolicy="", crossOrigin="", all other relevant mutations
  • Consulted after loading-committed: embedder CSP
• <iframe>:
  • Snapshotted at loading-started: embedder document base URL, referrerpolicy=""
  • Causes a re-fetch: src=""
  • Consulted after loading-committed: embedder CSP, embedder sandbox, container sandbox=""

This seems like an acceptable intermediate state for me. If I were to change anything, it would be to find out if we can uniformize the behavior of referrerpolicy="". And maybe try to make the container sandbox="" behave like referrerpolicy="", but that's a stretch.

Now, what is the ultimate desired state? We should probably discuss that in another issue, perhaps #4926 or perhaps a dedicated one. #4926 gestures at moving everything into "snapshotted at loading-started". But getting there will be pretty difficult, especially given the extensive dependency on browsing contexts in CSP and sandboxing. And the discussion might impact other fetches, such as <img> or perhaps even <a>.

[domfarolino]

Thanks a ton for the detailed feedback @domenic! I have a couple of corrections:

This seems like an acceptable intermediate state for me. If I were to change anything, it would be to find out if we can uniformize the behavior of referrerpolicy="". And maybe try to make the container sandbox="" behave like referrerpolicy="", but that's a stretch.

I'll go ahead and edit your comment, but actually the image loading process is interesting: We arguably "snapshot" every property of the request before load-committed, but both referrerpolicy and crossorigin mutations cause-refetches, along with all other relevant mutations.

Given this, I think uniformizing these attributes would require introducing something like a relevant mutations equivalent for the iframe element, which would basically involve:

• Creating this definition for iframes
• Including things like iframe-7, iframe-9, and iframe-10 in this list
• Also mentioning referrerpolicy mutations in this list

But yeah I'm fine with that being done separately, if it is desirable.

Given all this, I think this is ready for review!

[zcorpan]

This is mostly an editorial review. I haven't reviewed the tests or tried to dig deep, yet. I'll be able to review in more detail later this month.

[domfarolino]

Requesting review from @domenic as well given his feedback, and the current state of the PR.

[domenic]

but both referrerpolicy and crossorigin mutations cause-refetches, along with all other relevant mutations.

That isn't quite true, right? Because of the image cache, which is not keyed by referrerpolicy="", changing referrerpolicy="" will not cause a re-fetch.

[domenic]

LGTM with nit and my comment about potentially increasing the clarity for both img and iframe about "the rest of this algorithm".

[domfarolino]

The main unresolved question I think we have is should loading=lazy have an effect on <iframe srcdoc="...">? Currently I believe Chrome does not support this. I'm indifferent on what the spec should say / do.

[domenic]

Changes LGTM. I think the other way would be slightly nicer but I'm happy to leave that for a future refactoring.

Since <iframe srcdoc>s are all inline, I don't think lazy-loading them is particularly helpful---at least, it doesn't save bandwidth---but it's hard for me to say. If it has no effect, then we should make it a document conformance error to use the loading attribute when the srcdoc attribute is present.

Does anyone else have opinions or insights?

[annevk]

I think it's a little more complicated. A srcdoc document can include subresources and those will block the load event of that document which in turn blocks the load event of the parent document. Not blocking the load event of the parent document seems very useful and a core aspect of "lazy loading". (Edit: I already said this above, but maybe this is clearer?)

[domenic]

So I think this was discussed a bit in IRC, but it'd be good to record the conclusion in the thread. As I see it we have three options:

1. The PR as-is, with the understanding that for now lazy doesn't do anything on srcdoc, but we'll add it in a followup.

2. No lazy srcdocs: in this case the PR should add an authoring conformance requirement prohibiting the combination.

3. Support lazy srcdocs now: in this case the PR needs updating to support that.

What's the plan?

[domfarolino]

I believe the plan is (3), to support srcdoc in this PR and test it. I'll try and do at least one of those today or tomorrow.

[domfarolino]

The latest commit(s) add loading=lazy support for srcdoc iframes, PTAL @emilio and others! It does so by adding a little duplication between the srcdoc steps and the "otherwise" steps, but it's pretty minor so I think it's OK.

I also fixed what I consider a small bug that I added in the otherwise frame steps. When we run the will lazy load element steps, we assumed the element was an iframe, however we share these steps with the frameset tag. I fixed this, so now we check to see if the element is an iframe before invoking the will lazy load image steps, and don't do so otherwise.

[emilio]

This looks sensible to me :)

[domenic]

Excellent, LGTM too. I'll wait until next week to merge in case @annevk or @zcorpan want to take another look.

[domfarolino]

Can someone sanity check me here? I want to make sure that loading=lazy iframes are not accidentally blocking their containing document's load event when we're waiting for their resumption steps to be called. When we're waiting, I don't believe the potentially delays the load event conditions are met, which means we're good, but I'd like someone to verify. My rationale for the potentially delays the load event conditions not being met:

• element's nested browsing context's active document is not ready for post-load tasks.

  • When the iframe creates a new nested browsing context, we create a new browsing context, and the document created here is immediately ready for post-load tasks, so this condition is not met

• Anything is delaying the load event of element's nested browsing context's active document.

  • Nope

• element's nested browsing context is in the delaying load events mode.

  • Not yet, but it will be later in the Navigate algorithm

So I think we're good. Buttttt, this brings me to a difference I spotted between iframe and image lazy loading. For image lazy loading, even if the request is resumed before the window load event is fired, the request doesn't delay the window load event. This is due to #updating-the-image-data-step21. However for iframes, even if the load is resumed before the containing document's window load event is fired, then eventually we'll end up in the Navigation algorithm, and the nested browsing context will be put into the delaying load events mode, and the other potentially delays the load event conditions may be met as well.

I think one non-invasive way to fix this would be to keep some state that is persistent per-iframe-request that basically says current navigation was lazy loaded. This only gets reset once the "otherwise steps" or srcdoc navigation steps are re-invoked. We reference this new state / bool in the following way. We change:

The iframe element potentially delays the load event.

...to say...

Iframe elements whose current navigation was lazy loaded bool is false potentially delay the load event.

This gives us a per-request state independent of the loading attribute after resumption, just like the image #updating-the-image-data's delay load event boolean. Not sure if this is the best way. Thoughts?

Either way, we'll need a test like so:

• You have an in-viewport loading=lazy iframe
• The containing document's window load event is delayed by ~1 second or so
• After first-render the in-viewport iframe request is resumed, and the request is made before the window load event
• The request takes ~2 seconds or so
• The outer window load event is fired
• The iframe eventually finishes load, and its load event is fired (after the outer window one obviously)

[domfarolino]

All tests have landed, PTAL @emilio @annevk

[domenic]

Alright, this has accumulated quite a set of approvals by now. I'll merge this tomorrow unless anyone else wants to take a final look.

[emilio]

Looks sane to me.