Skip to content

Add helper functions for encoding and writing strings in src/core/obj_bin_transform_core.js - #22016

Merged
Snuffleupagus merged 3 commits into
mozilla:masterfrom
Snuffleupagus:compile-writeStrings
Sep 29, 2026
Merged

Snuffleupagus merged 3 commits into
mozilla:masterfrom
Snuffleupagus:compile-writeStrings

Conversation

@Snuffleupagus

@Snuffleupagus Snuffleupagus commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Currently compileCssFontInfo, compileSystemFontInfo, and compileFontInfo duplicate the same exact code for encoding and writing strings, which can be avoided with the introduction of two new helpers.

Note: This reduces the size of the gulp mozcentral bundle by 450 bytes, which isn't a lot but still cannot hurt.

@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Reference tests

Results for 8905373 (run, references from mozilla/pdf.js.refs@e990d937ae).

Platform Status Tests Total runtime Errors Different FBF No reference Report
Linux ✅ 1380 10m 58s 0 0 0 0
Windows ✅ 1380 13m 22s 0 0 0 0

The report is kept for 30 days and replaced by the next run.

@codecov-commenter

codecov-commenter commented Sep 24, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.37%. Comparing base (91041fb) to head (8905373).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #22016      +/-   ##
==========================================
+ Coverage   90.36%   90.37%   +0.01%     
==========================================
  Files         269      269              
  Lines       67233    67201      -32     
==========================================
- Hits        60753    60733      -20     
+ Misses       6480     6468      -12     
Flag Coverage Δ
browsertest 66.17% <100.00%> (-0.02%) ⬇️
fonttest 8.98% <0.00%> (+0.01%) ⬆️
integrationtest 69.42% <100.00%> (+<0.01%) ⬆️
unittest 59.51% <100.00%> (-0.02%) ⬇️
unittestcli 58.02% <100.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Sep 24, 2026
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Sep 24, 2026
@Snuffleupagus
Snuffleupagus marked this pull request as draft September 24, 2026 13:07
@Snuffleupagus
Snuffleupagus marked this pull request as ready for review September 24, 2026 13:08
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Sep 24, 2026
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Sep 24, 2026
@mozilla mozilla deleted a comment from moz-tools-bot Sep 24, 2026
@mozilla mozilla deleted a comment from moz-tools-bot Sep 24, 2026
@mozilla mozilla deleted a comment from moz-tools-bot Sep 24, 2026
@mozilla mozilla deleted a comment from moz-tools-bot Sep 24, 2026
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Sep 24, 2026
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Sep 24, 2026
…j_bin_transform_core.js`

Currently `compileCssFontInfo`, `compileSystemFontInfo`, and `compileFontInfo` duplicate the same exact code for encoding and writing strings, which can be avoided with the introduction of two new helpers.

*Note:* This reduces the size of the `gulp mozcentral` bundle by `450` bytes, which isn't a lot but still cannot hurt.
…ings in `compileSystemFontInfo`

*Note:* This reduces the size of the `gulp mozcentral` bundle by `281` bytes, which isn't a lot but still cannot hurt.
After the previous patches the total length of the encoded strings are consistently available *before* writing them, hence we can directly set the length and remove the temporary (i.e. zero) placeholder entry.
@mozilla mozilla deleted a comment from moz-tools-bot Sep 29, 2026
@mozilla mozilla deleted a comment from moz-tools-bot Sep 29, 2026
@mozilla mozilla deleted a comment from moz-tools-bot Sep 29, 2026
@mozilla mozilla deleted a comment from moz-tools-bot Sep 29, 2026
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Sep 29, 2026
@Snuffleupagus
Snuffleupagus requested review from calixteman and removed request for calixteman September 29, 2026 07:43

@calixteman calixteman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM. Thank you.

This branch was successfully deployed

1 active deployment
sync_pdfs — 8905373f Deployed Sep 29, 2026 by Snuffleupagus via request #170
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants