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

Replace the AnnotationStorage.lastModified-getter with a proper hash-method #14873

Merged
merged 1 commit into from
May 4, 2022

Conversation

Snuffleupagus
Copy link
Collaborator

The current lastModified-getter, which only contains a time-stamp, is a fairly crude way of detecting if the stored data has actually been changed. In particular, when the getRawValue-method is used, the lastModified-getter doesn't cope with data being modified from the "outside".

To fix these issues[1], and to prevent any future bugs in this code, this patch introduces a new AnnotationStorage.hash-getter which computes a hash of the currently stored data. To simplify things this re-uses the existing MurmurHash3_64-implementation, which required moving that file into the src/shared/-folder, since its performance should be good enough here.


[1] Given how the AnnotationStorage.lastModified-getter was used, this would have been limited to printing of forms.

@Snuffleupagus Snuffleupagus force-pushed the AnnotationStorage-hash branch 2 times, most recently from 690b7f5 to 1541d42 Compare May 4, 2022 13:20
…h-method

The current `lastModified`-getter, which only contains a time-stamp, is a fairly crude way of detecting if the stored data has actually been changed. In particular, when the `getRawValue`-method is used, the `lastModified`-getter doesn't cope with data being modified from the "outside".

To fix these issues[1], and to prevent any future bugs in this code, this patch introduces a new `AnnotationStorage.hash`-getter which computes a hash of the currently stored data. To simplify things this re-uses the existing `MurmurHash3_64`-implementation, which required moving that file into the `src/shared/`-folder, since its performance should be good enough here.

---
[1] Given how the `AnnotationStorage.lastModified`-getter was used, this would have been limited to *printing* of forms.
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

pdfjsbot commented May 4, 2022

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/0a8a42fb1131385/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 4, 2022

From: Bot.io (Windows)


Received

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

Live output at: http://54.193.163.58:8877/3cce35d1d30c114/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 4, 2022

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/0a8a42fb1131385/output.txt

Total script time: 26.15 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 5
  different first/second rendering: 1

Image differences available at: http://54.241.84.105:8877/0a8a42fb1131385/reftest-analyzer.html#web=eq.log

@pdfjsbot
Copy link

pdfjsbot commented May 4, 2022

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/3cce35d1d30c114/output.txt

Total script time: 30.92 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 2131

Image differences available at: http://54.193.163.58:8877/3cce35d1d30c114/reftest-analyzer.html#web=eq.log

@Snuffleupagus
Copy link
Collaborator Author

/botio-windows browsertest

@pdfjsbot
Copy link

pdfjsbot commented May 4, 2022

From: Bot.io (Windows)


Received

Command cmd_browsertest from @Snuffleupagus received. Current queue size: 0

Live output at: http://54.193.163.58:8877/e29f2b328d6bfef/output.txt

@pdfjsbot
Copy link

pdfjsbot commented May 4, 2022

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/e29f2b328d6bfef/output.txt

Total script time: 23.69 mins

  • Regression tests: FAILED
  different ref/snapshot: 2131

Image differences available at: http://54.193.163.58:8877/e29f2b328d6bfef/reftest-analyzer.html#web=eq.log

@timvandermeij timvandermeij merged commit 8b8a09e into mozilla:master May 4, 2022
@timvandermeij
Copy link
Contributor

Looks good; thanks!

@Snuffleupagus Snuffleupagus deleted the AnnotationStorage-hash branch May 5, 2022 08:19
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.

None yet

3 participants