search

readium / readium-sdk

A C++ ePub renderer SDK
BSD 3-Clause "New" or "Revised" License
385 stars 164 forks source link

Feature/content filter error architecture #239

Open nleme opened 8 years ago

nleme commented 8 years ago

Creating this pull request to track the work to merge the new architecture for ContentFilter errors into develop.

This is a rather old feature branch, so the first thing I have to do is to merge develop into this feature branch, and solve any possible merge conflicts, so that this code is up to date.

nleme commented 8 years ago

Let me attach the conversation that was already going on about the changes in this feature branch:

Email from Daniel Weck:

I am a little concerned about the NoLicenseForDecryption type of ContentFilterError, which seems quite specific. See whole code diff: https://github.com/readium/readium-sdk/compare/develop...feature/contentFilterErrorArchitecture For example, the LCP ContentModule integration handles license-related DRM "bootstrapping" (e.g. certificate + signature checks, user passphrase input) early on, outside of the ContentFilter chain. See: https://github.com/readium/readium-sdk/compare/develop...feature/content-module https://github.com/readium/SDKLauncher-Android/compare/develop...feature/lcp https://github.com/readium/SDKLauncher-iOS/compare/develop...feature/lcp https://github.com/readium/SDKLauncher-OSX/compare/develop...feature/lcp https://github.com/readium/readium-lcp-client/tree/android

Email from me:

First, about the enumeration value "NoLicenseForDecryption". Would you prefer something more generic like "InternalError"?

Second, about the error processing in ContentModule. That's absolutely correct, but that should happen in parallel to the error processing in the ContentFilter. It doesn't matter if you have a ContentModule set up or not, or even if you have any DRM going or not, you can always have a chain of ContentFilter objects, for whatever reasons, and those objects when filtering bytes can have an error. The idea here is how to report when those errors happen.

We should continue the discussion here on the pull request, from now on.

danielweck commented 7 years ago

Reminder: we need to review this in depth, comparative analysis with LCP's own error handling additions. e.g. throw ePub3::ContentModuleException in https://github.com/readium/readium-lcp-client/blob/develop/src/lcp-content-filter/LcpContentModule.cpp See: https://github.com/readium/readium-sdk/blob/feature/content-module/ePub3/ePub/content_module_exception.h defined in feature/content-module branch of readium-sdk

danielweck commented 7 years ago

CC @clebeaupin