Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,56 @@ class MainActivityTests {
onView(withText(R.string.action_contact)).check(matches(isDisplayed()))
}

/** A page the server answers with an error is not a document, whatever the load reported. */
@Test
fun aPageTheServerCannotServeOffersContact() {
val activity = mainActivityActivityTestRule.activity
val documentFragment = loadDocument(activity, requireTestFile("test.odt"))

val partUri = documentFragment.lastDocument?.partUris?.firstOrNull()
Assert.assertNotNull("no page was published", partUri)

val pageView = documentFragment.pageView
Assert.assertNotNull(pageView)

// a path the server has nothing under, which errors the same way a page it cannot render
InstrumentationRegistry.getInstrumentation().runOnMainSync {
pageView!!.loadUrl("$partUri.missing")
}

// the webview reports the error long after loadUrl returns, and nothing idles on it
Assert.assertTrue(
"the page that failed was not given up on",
waitFor(10000) { documentFragment.lastDocument == null },
)

onView(withText(R.string.dialog_broken_file)).check(matches(isDisplayed()))
onView(withText(R.string.action_contact)).check(matches(isDisplayed()))
}

/** A link that fails is not the document failing, and must not be reported as one. */
@Test
fun aLinkThatFailsLeavesTheDocumentOpen() {
val activity = mainActivityActivityTestRule.activity
val documentFragment = loadDocument(activity, requireTestFile("test.odt"))

val pageView = documentFragment.pageView
Assert.assertNotNull(pageView)

// what the webview is left to try when shouldOverrideUrlLoading finds no app for a link
InstrumentationRegistry.getInstrumentation().runOnMainSync {
pageView!!.loadUrl("nosuchscheme://example")
}

// a plain wait: this asserts something does not happen, after a webview round trip
// that nothing idles on
SystemClock.sleep(3000)
InstrumentationRegistry.getInstrumentation().waitForIdleSync()

Assert.assertNotNull("the document was given up on", documentFragment.lastDocument)
onView(withText(R.string.dialog_broken_file)).check(doesNotExist())
}

@Test
fun testODTEditMode() {
val activity = mainActivityActivityTestRule.activity
Expand Down Expand Up @@ -330,6 +380,17 @@ class MainActivityTests {
return fragment
}

private fun waitFor(timeoutMs: Long, condition: () -> Boolean): Boolean {
val startMs = SystemClock.elapsedRealtime()
do {
if (condition()) {
return true
}
SystemClock.sleep(100)
} while (SystemClock.elapsedRealtime() - startMs < timeoutMs)
return false
}

private fun waitForDocumentFragment(
activity: MainActivity,
timeoutMs: Long,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,41 @@ class DocumentFragment : Fragment(), DocumentLoader.Listener {
state.endLoadIdling()
}

/**
* The page [PageView] was given cannot be shown after all, so this ends where a document that
* would not open ends: back on the landing screen, offering to tell us about it.
*
* It arrives after [onLoadSuccess] rather than instead of it, which is why it undoes it instead
* of going through [isActivityReadyForOutcome].
*/
fun onPageFailed() {
// already given up on, or a second view of the same failure
if (state.lastDocument == null) {
return
}

if (activity == null || isStateSaved) {
replayOnStart = { onPageFailed() }

return
}

val activity = requireActivity()

analyticsManager.report(
"page_failed",
AnalyticsConstants.PARAM_CONTENT_TYPE,
state.lastFile?.mimeType,
)

unload()
dismissProgress()

giveUp(activity)

offerContact(activity)
}

override fun onEncrypted(request: DocumentRequest, file: IdentifiedFile) {
if (!isActivityReadyForOutcome(request, file) { onEncrypted(request, file) }) {
return
Expand Down
44 changes: 40 additions & 4 deletions app/src/main/java/app/opendocument/droid/ui/widget/PageView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import android.util.Base64InputStream
import android.webkit.JavascriptInterface
import android.webkit.WebResourceError
import android.webkit.WebResourceRequest
import android.webkit.WebResourceResponse
import android.webkit.WebView
import android.webkit.WebViewClient
import androidx.annotation.Keep
Expand Down Expand Up @@ -101,10 +102,32 @@ constructor(context: Context, attributeSet: AttributeSet?) :
return
}

crashManager.log(
RuntimeException(
"loading ${request.url} failed: ${error.errorCode} ${error.description}"
)
failPage(
request.url,
"loading ${request.url} failed: ${error.errorCode} ${error.description}",
)
Comment thread
andiwand marked this conversation as resolved.
}

/**
* An error status is the only shape a rendering failure has here: the core
* translates a page on the server thread, long after `CoreLoader` reported success.
* [onReceivedError] never sees it - the server did answer.
*/
override fun onReceivedHttpError(
view: WebView,
request: WebResourceRequest,
errorResponse: WebResourceResponse,
) {
super.onReceivedHttpError(view, request, errorResponse)

if (!request.isForMainFrame) {
return
}

failPage(
request.url,
"serving ${request.url} failed: " +
"${errorResponse.statusCode} ${errorResponse.reasonPhrase}",
)
}

Expand Down Expand Up @@ -180,6 +203,19 @@ constructor(context: Context, attributeSet: AttributeSet?) :
super.destroy()
}

/** Reports a page that will never appear. [description] is all there is of the cause. */
private fun failPage(url: Uri, description: String) {
crashManager.log(RuntimeException(description))

// only the document is the app's to give up on. a link shouldOverrideUrlLoading could not
// hand to another app is left to the webview, and fails here as a main frame load too
if (!isOwnContent(url.toString())) {
return
}

documentFragment.onPageFailed()
}

/** Whether [url] is a document we produced: a cached file, or the core's own http server. */
private fun isOwnContent(url: String): Boolean =
url.startsWith("file://") || url.startsWith(LOCAL_SERVER_URL_PREFIX)
Expand Down
Loading