1901525 - Simplify DecryptingInputStream implementation

Status: RESOLVED FIXED

(Core :: Storage: Quota Manager, task, P3)

Description

[Jan Varga [:janv]]

There are several things which can be removed or simplified:

1. The extra Seek and ParseNextChunk in DecryptingInputStream::Seek (when readBytes) is zero can be removed:
   See https://searchfox.org/mozilla-central/rev/46d0387f0b582f00a5722c20d4e6b8693793631b/dom/quota/DecryptingInputStream_impl.h#419-437

2. mLastBlockLength can be removed and then all related code can be simplified

3. DecryptingInputStream::Tell can be simplified to not use mPlainBytes in the position calculation

Comment 1

[Jan Varga [:janv]]

Attachment: Bug 1901525 - Remove extra Seek and ParseNextChunk from DecryptingInputStream::Seek; r=#dom-storage

Now when we have proper boundary checks, the extra Seek and ParseNextChunk can
be safely removed.

Comment 2

[Jan Varga [:janv]]

Attachment: Bug 1901525 - Add extra Tell check to the end of every seek operation test; r=#dom-storage

Comment 3

[Jan Varga [:janv]]

Attachment: Bug 1901525 - Synchronize mPlainBytes/mNextByte/mLastBlockLength handling in DecryptingInputStream; r=#dom-storage

DecryptingInputStream::ReadSegments resets mPlainBytes and mNextByte to zero
when the position in the plain buffer reaches the end.
DecryptingInputStream::Seek doesn't do that and keeps mPlainBytes and mNextByte
having the same value. It's not strictly required for
DecryptingInputStream::Seek to do the resetting, but the code will read better
if the handling is the same (and will help with understanding of the next
patch).

Comment 4

[Jan Varga [:janv]]

Attachment: Bug 1901525 - Fix incorrect comments in DecryptingInputStream; r=#dom-storage

Comment 5

[Jan Varga [:janv]]

Attachment: Bug 1901525 - Simplify current position handling in DecryptingInputStream; r=#dom-storage

DecryptingInputStream::ReadSegments and DecryptingInputStream::Seek currently
reset mPlainBytes and mNextByte to zero when the position in the plain buffer
reaches the end. The current position calculation then requires extra member
variable mLastBlockLength which complicates the logic even more. It turns out
that mLastBlockLength can be removed when the resetting of mPlainBytes and
mNextByte is avoided.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f69e403c8f98
Remove extra Seek and ParseNextChunk from DecryptingInputStream::Seek; r=hsingh
https://hg.mozilla.org/integration/autoland/rev/8a4dbb3e2c03
Add extra Tell check to the end of every seek operation test; r=hsingh
https://hg.mozilla.org/integration/autoland/rev/637f8eff63e1
Synchronize mPlainBytes/mNextByte/mLastBlockLength handling in DecryptingInputStream; r=hsingh
https://hg.mozilla.org/integration/autoland/rev/1833678d1f85
Fix incorrect comments in DecryptingInputStream; r=hsingh
https://hg.mozilla.org/integration/autoland/rev/55e6d2b6042a
Simplify current position handling in DecryptingInputStream; r=hsingh

Comment 7

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/f69e403c8f98
https://hg.mozilla.org/mozilla-central/rev/8a4dbb3e2c03
https://hg.mozilla.org/mozilla-central/rev/637f8eff63e1
https://hg.mozilla.org/mozilla-central/rev/1833678d1f85
https://hg.mozilla.org/mozilla-central/rev/55e6d2b6042a