Bug 215610 - Convert runtime flag to setting for lazy image loading
Summary: Convert runtime flag to setting for lazy image loading
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Images (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
: 216007 (view as bug list)
Depends on:
Blocks:
 
Reported: 2020-08-18 09:01 PDT by Rob Buis
Modified: 2020-09-15 04:36 PDT (History)
12 users (show)

See Also:


Attachments
Patch (56.89 KB, patch)
2020-08-18 09:11 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (41.89 KB, patch)
2020-08-27 01:49 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (50.29 KB, patch)
2020-08-27 05:44 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (53.52 KB, patch)
2020-08-27 07:32 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (53.52 KB, patch)
2020-08-27 08:24 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (65.43 KB, patch)
2020-08-28 02:45 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (66.92 KB, patch)
2020-08-28 04:27 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (66.66 KB, patch)
2020-08-28 06:44 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (67.72 KB, patch)
2020-08-29 02:04 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (68.55 KB, patch)
2020-08-31 02:47 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (68.55 KB, patch)
2020-08-31 10:31 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (63.78 KB, patch)
2020-08-31 11:49 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (63.96 KB, patch)
2020-09-01 01:43 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (65.35 KB, patch)
2020-09-10 08:40 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (67.69 KB, patch)
2020-09-14 01:23 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (62.94 KB, patch)
2020-09-14 06:11 PDT, Rob Buis
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (61.94 KB, patch)
2020-09-14 07:34 PDT, Rob Buis
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (65.49 KB, patch)
2020-09-14 08:55 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (68.62 KB, patch)
2020-09-14 11:12 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (67.42 KB, patch)
2020-09-14 12:46 PDT, Rob Buis
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (64.29 KB, patch)
2020-09-14 14:06 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (64.20 KB, patch)
2020-09-15 01:16 PDT, Rob Buis
no flags Details | Formatted Diff | Diff
Patch (61.80 KB, patch)
2020-09-15 02:48 PDT, Rob Buis
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2020-08-18 09:01:24 PDT
Convert runtime flag to setting for lazy image loading.
Comment 1 Rob Buis 2020-08-18 09:11:27 PDT
Created attachment 406789 [details]
Patch
Comment 2 EWS Watchlist 2020-08-18 09:12:13 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 3 Radar WebKit Bug Importer 2020-08-25 09:02:13 PDT
<rdar://problem/67740399>
Comment 4 Rob Buis 2020-08-27 01:49:24 PDT
Created attachment 407384 [details]
Patch
Comment 5 Rob Buis 2020-08-27 05:44:23 PDT
Created attachment 407391 [details]
Patch
Comment 6 Rob Buis 2020-08-27 07:32:31 PDT
Created attachment 407398 [details]
Patch
Comment 7 Rob Buis 2020-08-27 08:24:52 PDT
Created attachment 407400 [details]
Patch
Comment 8 Rob Buis 2020-08-28 02:45:45 PDT
Created attachment 407457 [details]
Patch
Comment 9 Rob Buis 2020-08-28 04:27:43 PDT
Created attachment 407459 [details]
Patch
Comment 10 youenn fablet 2020-08-28 06:04:38 PDT
Comment on attachment 407459 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407459&action=review

> Source/WebKit/Shared/WebPreferences.yaml:-2048
> -  category: experimental

Why removing the experimental feature?
Comment 11 Rob Buis 2020-08-28 06:44:51 PDT
Created attachment 407466 [details]
Patch
Comment 12 EWS 2020-08-28 09:38:57 PDT
Found 1 new test failure: imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-slow.html
Comment 13 Rob Buis 2020-08-29 02:04:13 PDT
Created attachment 407536 [details]
Patch
Comment 14 Rob Buis 2020-08-31 02:47:41 PDT
Created attachment 407595 [details]
Patch
Comment 15 EWS 2020-08-31 04:52:02 PDT
Committed r266350: <https://trac.webkit.org/changeset/266350>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407595 [details].
Comment 16 Hector Lopez 2020-08-31 10:22:31 PDT
Reverted r266350 for reason:

Revision is causing a constant crash on both macOS and iOS

Committed r266358: <https://trac.webkit.org/changeset/266358>
Comment 17 Rob Buis 2020-08-31 10:31:41 PDT
Created attachment 407609 [details]
Patch
Comment 18 Simon Fraser (smfr) 2020-08-31 11:03:34 PDT
Comment on attachment 407609 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=407609&action=review

> Source/WebCore/ChangeLog:8
> +        Convert runtime flag to setting for lazy image loading.

This is missing the "why".
Comment 19 Rob Buis 2020-08-31 11:26:04 PDT
(In reply to Simon Fraser (smfr) from comment #18)
> Comment on attachment 407609 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=407609&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Convert runtime flag to setting for lazy image loading.
> 
> This is missing the "why".

Because Sam asked for it :) But I think Sam (partially) ended up implementing it himself:
https://bug-215981-attachments.webkit.org/attachment.cgi?id=407610
Comment 20 Rob Buis 2020-08-31 11:49:12 PDT
Created attachment 407613 [details]
Patch
Comment 21 Rob Buis 2020-09-01 01:43:30 PDT
Created attachment 407664 [details]
Patch
Comment 22 EWS 2020-09-01 12:24:26 PDT
Committed r266408: <https://trac.webkit.org/changeset/266408>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 407664 [details].
Comment 23 Hector Lopez 2020-09-01 18:56:30 PDT
Reverted r266408 for reason:

Revision caused constant crashes on iOS and macOS

Committed r266446: <https://trac.webkit.org/changeset/266446>
Comment 24 Simon Fraser (smfr) 2020-09-01 20:48:01 PDT
Why did EWS not show crashes?
Comment 25 Rob Buis 2020-09-01 22:37:42 PDT
(In reply to Simon Fraser (smfr) from comment #24)
> Why did EWS not show crashes?

What Simon says. You have to give a bit more information about this, the bots are green and I can't reproduce any problems locally. Is this maybe related to a certain macOs version? Any backtraces?
Comment 26 youenn fablet 2020-09-02 00:10:55 PDT
This is most probably https://bugs.webkit.org/show_bug.cgi?id=216007
Comment 27 Rob Buis 2020-09-02 00:45:04 PDT
(In reply to youenn fablet from comment #26)
> This is most probably https://bugs.webkit.org/show_bug.cgi?id=216007

Thanks Youenn. I was aware of this problem and added a line to skip it for now in the patches that landed:
+imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-element/image-loading-lazy-slow.html [ Skip ]

Is skipping not the right thing? Should it be marked as Crash?
Comment 28 youenn fablet 2020-09-02 00:49:29 PDT
(In reply to Rob Buis from comment #27)
> (In reply to youenn fablet from comment #26)
> > This is most probably https://bugs.webkit.org/show_bug.cgi?id=216007
> 
> Thanks Youenn. I was aware of this problem and added a line to skip it for
> now in the patches that landed:
> +imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-
> element/image-loading-lazy-slow.html [ Skip ]
> 
> Is skipping not the right thing? Should it be marked as Crash?

We should probably first understand why it is crashing before skipping it.
Can you repro the issue for this test?

If some other test is crashing, it might be some similar test.
Hector might be able to provide some more precise information later.
Comment 29 youenn fablet 2020-09-03 01:48:02 PDT
*** Bug 216007 has been marked as a duplicate of this bug. ***
Comment 30 Rob Buis 2020-09-04 07:38:54 PDT
(In reply to youenn fablet from comment #28)
> (In reply to Rob Buis from comment #27)
> > (In reply to youenn fablet from comment #26)
> > > This is most probably https://bugs.webkit.org/show_bug.cgi?id=216007
> > 
> > Thanks Youenn. I was aware of this problem and added a line to skip it for
> > now in the patches that landed:
> > +imported/w3c/web-platform-tests/html/semantics/embedded-content/the-img-
> > element/image-loading-lazy-slow.html [ Skip ]
> > 
> > Is skipping not the right thing? Should it be marked as Crash?
> 
> We should probably first understand why it is crashing before skipping it.
> Can you repro the issue for this test?
> 
> If some other test is crashing, it might be some similar test.
> Hector might be able to provide some more precise information later.

Yes, I was able to repro and fix is here:
https://bugs.webkit.org/show_bug.cgi?id=215998

I think it is cleaner to keep the fix separate since this bug was just about converting, not really changing behaviour.
Comment 31 Rob Buis 2020-09-10 08:40:21 PDT
Created attachment 408448 [details]
Patch
Comment 32 Rob Buis 2020-09-14 01:23:05 PDT
Created attachment 408688 [details]
Patch
Comment 33 Rob Buis 2020-09-14 02:57:50 PDT
@youenn I upstreamed some of the reftest-wait tests changes to:
https://github.com/web-platform-tests/wpt/pull/25513
Comment 34 Rob Buis 2020-09-14 06:11:44 PDT
Created attachment 408704 [details]
Patch
Comment 35 Rob Buis 2020-09-14 07:34:06 PDT
Created attachment 408705 [details]
Patch
Comment 36 Rob Buis 2020-09-14 08:55:56 PDT
Created attachment 408707 [details]
Patch
Comment 37 Rob Buis 2020-09-14 11:12:26 PDT
Created attachment 408727 [details]
Patch
Comment 38 Rob Buis 2020-09-14 12:46:28 PDT
Created attachment 408736 [details]
Patch
Comment 39 Rob Buis 2020-09-14 14:06:19 PDT
Created attachment 408745 [details]
Patch
Comment 40 Rob Buis 2020-09-15 01:16:01 PDT
Created attachment 408800 [details]
Patch
Comment 41 Rob Buis 2020-09-15 02:48:21 PDT
Created attachment 408808 [details]
Patch
Comment 42 EWS 2020-09-15 04:36:11 PDT
Committed r267083: <https://trac.webkit.org/changeset/267083>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408808 [details].