1146564 - Implement a Readability Service

Status: RESOLVED FIXED

(Firefox for iOS :: Reader View, defect)

Description

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

We now load reader view content from a cache. But what happens if the cached content is not available? Content can be unavailable because of a couple of reasons:

* The OS decided to clean up ~/Caches/
* A page was added to the Reading List via the Share Extension. In this case we only have the URL and Title of the page, but no content yet.
* New items have been synced to the Reading List. The server does not provide us with content yet, so we need to grab that content.

What we need is a Readability Service that can run in the background, indepdentn of an actual visible webview.

Then we can queue up pages to be readerized. These pages are then stored in the cache so that they can be read offline or when they are opened from the reading list when content is not available.

Comment 1

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Attachment: PR: https://github.com/mozilla/firefox-ios/pull/255

This patch introduces a `ReadabilityService`. This is a background services that uses the `WKWebView` and a new `ReadabilityBrowserHelper` to readerize pages on demand in the background.

Couple of notes:

* The interstitial that is displayed when you open a page from the reading list for which we have no content yet is very raw. It just shows a `<h1>` with a message. This needs UX. This is all located in `ReaderViewLoading.html`.
* The `ReaderModeService` may need a better way to handle errors. Right now things simply time out after which we display an error. We can probably speed that up so that the user does not have to wait 10 seconds.
* The current `ReaderMode` (which is also a `BrowserHelper`) can be much simplified by using this new `ReadabilityBrowserHelper`. There will be a nicer separation of concerns when we do that.

This is a good first iteration that I would like to expose to our testers sooner than later. This code needs to mature in the coming weeks though.

Comment 2

[Brian Nicholson (:bnicholson)]

Comment on attachment 8581889 [details] [review]
PR: https://github.com/mozilla/firefox-ios/pull/255

I really don't like the approach we're going with here. The polling HTML page feels like an unnecessary hack, and I'm also not convinced it's a good idea to try doing this all in the background.

For one, messing around with WKWebViews off the main thread is likely an unsafe operation as mentioned in the PR. Secondly, AFAICT, the browser isn't going to be doing anything useful on the main thread while it waits for the result -- it's simply waiting for the result to come back. It's not like we'll randomly be parsing pages in the background while the user is browsing, right? I'm just not sure what contention issues this is trying to solve.

Comment 3

[Richard Newman [:rnewman]]

(In reply to Brian Nicholson (:bnicholson) from comment #2)
> Secondly, AFAICT, the browser isn't
> going to be doing anything useful on the main thread while it waits for the
> result -- it's simply waiting for the result to come back. It's not like
> we'll randomly be parsing pages in the background while the user is
> browsing, right?

Yeah, we will. This will happen whenever entries are added to your reading list from other sources, namely from synchronization and from the share extension.

Comment 4

[Brian Nicholson (:bnicholson)]

Oh, that makes sense. But thinking about this a bit more, WKWebView content is already executed out-of-process, so I'm not sure throwing it on a background thread buys us anything regardless.

Comment 5

[Stefan Arentz | :st3fan | ⏰ EST | he/him]

Merged after fixing thread safety issues around WKWebView. Initial setup is now done on the main thread, while the NSOperation waits for readabilty results on it's own (unspecified) thread.