Add at least *some* test-coverage for the RenderTask.onContinue functionality
#12036
Conversation
This file contains 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
Snuffleupagus
force-pushed
the
test-renderTaskOnContinue
branch
from
30a601b to
ee403dc
Compare
Snuffleupagus
changed the title
RenderTask.onContinue functionalityRenderTask.onContinue functionality
Snuffleupagus
force-pushed
the
test-renderTaskOnContinue
branch
from
ee403dc to
3c95f58
Compare
…ctionality The default viewer, and thus Firefox, depends on the `RenderTask.onContinue` functionality to pause/continue rendering (such that the most visible page always renders first). Despite this functionality thus being very important, it has however never actually been tested *at all* as far as I can tell. Hence this patch which adds a new boolean `renderTaskOnContinue` parameter (`false` by default), that can be used to force a reference-test to use the `RenderTask.onContinue` code-path in the `InternalRenderTask` class. Note that I purposely made this new reference-test behaviour *optional*, since I didn't want to negatively affect the general runtime of the tests (given that there's a slight delay added to the rendering). Also, for e.g. benchmarking you'd most likely want to stay away from the `RenderTask.onContinue` functionality for similar reasons.
Snuffleupagus
force-pushed
the
test-renderTaskOnContinue
branch
from
3c95f58 to
4a5b68e
Compare
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.67.70.0:8877/498390b32731073/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.215.176.217:8877/22e39e3c5110b72/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.67.70.0:8877/498390b32731073/output.txt Total script time: 25.93 mins
Image differences available at: http://54.67.70.0:8877/498390b32731073/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.215.176.217:8877/22e39e3c5110b72/output.txt Total script time: 28.89 mins
Image differences available at: http://54.215.176.217:8877/22e39e3c5110b72/reftest-analyzer.html#web=eq.log |
timvandermeij
approved these changes
Jul 1, 2020
|
Looks good! |
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.
The default viewer, and thus Firefox, depends on the
RenderTask.onContinuefunctionality to pause/continue rendering (such that the most visible page always renders first).Despite this functionality thus being very important, it has however never actually been tested at all as far as I can tell. Hence this patch which adds a new boolean
renderTaskOnContinueparameter (falseby default), that can be used to force a reference-test to use theRenderTask.onContinuecode-path in theInternalRenderTaskclass.Note that I purposely made this new reference-test behaviour optional, since I didn't want to negatively affect the general runtime of the tests (given that there's a slight delay added to the rendering). Also, for e.g. benchmarking you'd most likely want to stay away from the
RenderTask.onContinuefunctionality for similar reasons.