Commit Graph
3 Commits
Author SHA1 Message Date
Flarnie MarchanandGitHub 076bbeace7 Fall back to 'setTimeout' when 'requestAnimationFrame' is not called (#13091)
* Add fixture test for schedule running when tab is backgrounded

**what is the change?:**
Just adding a test to the fixture, where we can easily see whether
scheduled callbacks are called after switching away from the fixture
tab.

**why make this change?:**
We are about to fix the schedule module so that it still runs even when
the tab is in the backround.

**test plan:**
Manually tested the fixture, verified that it works as expected and
right now callbacks are not called when the tab is in the background.

**issue:**
Internal task T30754186

* Fall back to 'setTimeout' when 'requestAnimationFrame' is not called

**what is the change?:**
If 'requestAnimationFrame' is not called for 100ms we fall back to
'setTimeout' to schedule the postmessage.

**why make this change?:**
When you start loading a page, and then switch tabs,
'requestAnimationFrame' is throttled or not called until you come back
to that tab. That means React's rendering, any any other scheduled work,
are paused.

Users expect the page to continue loading, and rendering is part of the
page load in a React app. So we need to continue calling callbacks.

**test plan:**
Manually tested using the new fixture test, observed that the callbacks
were called while switched to another tab. They were called more
slowly, but that seems like a reasonable thing.

**issue:**
Internal task T30754186

* make arguments more explicit
2018-06-22 09:13:47 -07:00
Flarnie MarchanandGitHub 15767a8f8f [scheduler] 5/n Error handling in scheduler (#12920)
* Initial failing unit test for error handling in schedule

**what is the change?:**
see title

**why make this change?:**
Adding tests for the error handling behavior we are about to add. This
test is failing, which gives us the chance to make it pass.

Wrote skeletons of some other tests to add.

Unit testing this way is really hacky, and I'm also adding to the
fixture to test this in the real browser environment.

**test plan:**
Ran new test, saw it fail!

* Add fixture for testing error handling in scheduler

**what is the change?:**
Added a fixture which does the following -
logs in the console to show what happens when you use
`requestAnimationFrame` to schedule a series of callbacks and some of
them throw errors.

Then does the same actions with the `scheduler` and verifies that it
behaves in a similar way.

Hard to really verify the errors get thrown at the proper time without
looking at the console.

**why make this change?:**
We want the most authentic, accurate test of how errors are handled in
the scheduler. That's what this fixture should be.

**test plan:**
Manually verified that this test does what I expect - right now it's
failing but follow up commits will fix that.

* Handle errors in scheduler

**what is the change?:**
We set a flag before calling any callback, and then use a 'try/finally'
block to wrap it. Note that we *do not* catch the error, if one is
thrown. But, we only unset the flag after the callback successfully
finishes.

If we reach the 'finally' block and the flag was not unset, then it
means an error was thrown.

In that case we start a new postMessage callback, to finish calling any
other pending callbacks if there is time.

**why make this change?:**
We need to make sure that an error thrown from one callback doesn't stop
other callbacks from firing, but we also don't want to catch or swallow
the error because we want engineers to still be able to log and debug
errors.

**test plan:**
New tests added are passing, and we verified that they fail without this
change.

* Add more tests for error handling in scheduler

**what is the change?:**
Added tests for more situations where error handling may come up.

**why make this change?:**
To get additional protection against this being broken in the future.

**test plan:**
Ran new tests and verified that they fail when error handling fails.

* callSafely -> callUnsafely

* Fix bugs with error handling in schedule

**what is the change?:**
- ensure that we properly remove the callback from the linked list, even
if it throws an error and is timed out.
- ensure that you can call 'cancelScheduledWork' more than once and it
is idempotent.

**why make this change?:**
To fix bugs :)

**test plan:**
Existing tests pass, and we'll add more tests in a follow up commit.

* Unit tests for error handling with timed out callbacks

**what is the change?:**
More unit tests, to cover behavior which we missed; error handling of
timed out callbacks.

**why make this change?:**
To protect the future!~

**test plan:**
Run the new tests.

* Adds fixture to test timed out callbacks with scheduler

**what is the change?:**
See title

In the other error handling fixture we compare 'scheduleWork' error
handling to 'requestAnimationFrame' and try to get as close as possible.
There is no 'timing out' feature with 'requestAnimationFrame' but
effectively the 'timing out' feature changes the order in which things
are called. So we just changed the order in the 'requestAnimationFrame'
version and that works well for illustrating the behavior we expect in
the 'scheduleWork' test.

**why make this change?:**
We need more test coverage of timed out callbacks.

**test plan:**
Executed the fixture manually in Firefox and Chrome. Results looked
good.

* fix rebase problems

* make fixture compensate for chrome JS speed

* ran prettier

* Remove 'cancelled' flag on callbackConfig in scheduler, add test

**what is the change?:**
- Instead of using a 'cancelled' flag on the callbackConfig, it's easier
to just check the state of the callbackConfig inside
'cancelScheduledWork' to determine if it's already been cancelled. That
way we don't have to remember to set the 'cancelled' flag every time we
call a callback or cancel it. One less thing to remember.
- We added a test for calling 'cancelScheduledWork' more than once,
which would have failed before.

Thanks @acdlite for suggesting this in code review. :)

**why make this change?:**
To increase stability of the schedule module, increase test coverage.

**test plan:**
Existing tests pass and we added a new test to cover this behavior.

* fix typo
2018-05-30 15:38:48 -07:00
Flarnie MarchanandGitHub 76e07071a1 [scheduler] 2/n Adding 'schedule' fixture (#12884)
* Adding 'schedule' fixture

**what is the change?:**
We need to test the `schedule` module against real live browser APIs. As
a quick solution we're writing a fixture for using in manual testing.
Later we plan on adding automated browser testing, using this or a
similar fixture as the test page.

**why make this change?:**
To further solidify test coverage for `schedule` before making further
improvements/refactors to the module.

**test plan:**
`open fixtures/schedule/index.html` and inspect the results. It should
be clear that things pass.

We also temporarily broke the scheduler and verified that this fixture
demonstrates the problems.

**issue:**
Internal task T29442940

* Made fixture tests display red or green border depending on pass/fail

**what is the change?:**
Added red/green solid/dashed border for test results when using the
schedule fixture.

We also tweaked the timing of the last test because it was on the line
in terms of whether it passed or failed.

**why make this change?:**
To make it faster to use the fixture - it takes more time to read
through the results line by line and check that they match what is
expected.

**test plan:**
Looked at the fixture, and also tried modifying a test to show what it
looks like when something fails.
2018-05-24 14:11:25 -07:00