Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve work-around for importScripts bug. #6913

Merged
merged 1 commit into from
Feb 4, 2016

Conversation

Rob--W
Copy link
Member

@Rob--W Rob--W commented Jan 22, 2016

Reverts "Hack to avoid intermidiate Chrome failures during tests." (2b2c521).

Updates require.js to the latest version (pre-release), which includes an alternative work-around for the bug that was worked around by the reverted commit. The advantage of the new work-around is that it allows the runtime to garbage-collect idle Workers.

References:

Addresses #6877

@Rob--W
Copy link
Member Author

Rob--W commented Jan 22, 2016

Aw. requirejs's package.json is invalid (no version), so this PR does not work. We'll have to wait until the next version of RequireJS is released, and then update package.json to point to the new version instead of a specific hash.

@Rob--W
Copy link
Member Author

Rob--W commented Feb 3, 2016

I updated the PR, to include the patch directly in worker_loader.js instead of waiting for the next requirejs release.

@@ -15,6 +15,17 @@

'use strict';

//#if !PRODUCTION
// Patch importScripts to work around a bug in WebKit and Chrome 48-.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a space or second // here

Reverts "Hack to avoid intermidiate Chrome failures during tests."
(2b2c521).

require.js uses importScript asynchronously, which activates the worker
GC bug in WebKit. This patch works around a bug in a way that is similar
in the upcoming (but not yet released) require.js 2.1.23

The advantage of the new work-around is that it allows the runtime to
garbage-collect idle Workers.

References:
- https://crbug.com/572225
- https://webkit.org/b/153317
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

pdfjsbot commented Feb 4, 2016

From: Bot.io (Linux)


Received

Command cmd_preview from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/e4abe24a3a6a4d5/output.txt

@timvandermeij
Copy link
Contributor

/botio test

@pdfjsbot
Copy link

pdfjsbot commented Feb 4, 2016

From: Bot.io (Linux)


Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.21.233.14:8877/68ff34a546de900/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 4, 2016

From: Bot.io (Windows)


Received

Command cmd_test from @timvandermeij received. Current queue size: 0

Live output at: http://107.22.172.223:8877/b866eacbde60aa7/output.txt

@pdfjsbot
Copy link

pdfjsbot commented Feb 4, 2016

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/b866eacbde60aa7/output.txt

Total script time: 20.26 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@pdfjsbot
Copy link

pdfjsbot commented Feb 4, 2016

From: Bot.io (Linux)


Success

Full output at http://107.21.233.14:8877/68ff34a546de900/output.txt

Total script time: 21.69 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: Passed

@timvandermeij
Copy link
Contributor

Landing with r=me and r=yurydelendik (see http://logs.glob.uno/?c=mozilla%23pdfjs&s=3+Feb+2016&e=3+Feb+2016#c46063). Thank you for the patch!

timvandermeij added a commit that referenced this pull request Feb 4, 2016
Improve work-around for importScripts bug.
@timvandermeij timvandermeij merged commit a0aa781 into mozilla:master Feb 4, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants