-
Notifications
You must be signed in to change notification settings - Fork 163
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
Feature/content filter error architecture #239
base: develop
Are you sure you want to change the base?
Conversation
ContentFilters are one of the mechanisms that the Readium SDK uses for extensibility. However, up to this point, there was not a clear way to report an error inside a ContentFilter. For example, if there is any error inside the FilterData() function, there would be no clear way to report that back to the app. This change creates a new architecture through which the code inside a given ContentFilter will be able to report an error back to the app, and the app will be able to do something about that (like displaying the error to the user).
Recently, I implemented a change in the Readium SDK that allows ContentFilters to report errors. That is important because the ContentFilters are the basic mechanism for extending the Readium SDK, and it is expected that people all over the world will start writing their own ContentFilters. Given that, it is necessary that there is a mechanism for reporting when there is an error in a given ContentFilter. The basic mechanism in the Readium SDK has already been implemented, but this change adds the extra glue code in the JNI layer so that Android apps can receive those error messages.
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. 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. |
Reminder: we need to review this in depth, comparative analysis with LCP's own error handling additions. |
CC @clebeaupin |
*/ | ||
static void HandleContentFilterError(const std::string &filterId, unsigned int errorCode, const std::string &message) | ||
{ | ||
s_contentFilterErrorHandler(filterId, errorCode, message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there should be NULL check here, just in case ResetContentFilterErrorHandler()
was never invoked.
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.