Skip to content

Commit

Permalink
Closes issue mozilla-mobile#8681: Do not retry to update an extension
Browse files Browse the repository at this point in the history 
when an unrecoverable exception happens
  • Loading branch information
@Amejia481
Amejia481 committed Oct 16, 2020
1 parent 283dc92 commit a4d2e8b
Show file tree
Hide file tree
Showing 12 changed files with 309 additions and 12 deletions.
7 changes: 6 additions & 1 deletion ...er/engine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngine.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import mozilla.components.concept.engine.webextension.EnableSource
import mozilla.components.concept.engine.webextension.TabHandler
import mozilla.components.concept.engine.webextension.WebExtension
import mozilla.components.concept.engine.webextension.WebExtensionDelegate
import mozilla.components.concept.engine.webextension.WebExtensionException
import mozilla.components.concept.engine.webextension.WebExtensionRuntime
import mozilla.components.concept.engine.webnotifications.WebNotificationDelegate
import mozilla.components.concept.engine.webpush.WebPushDelegate
Expand All @@ -55,6 +56,8 @@ import org.mozilla.geckoview.GeckoRuntime
import org.mozilla.geckoview.GeckoSession
import org.mozilla.geckoview.GeckoWebExecutor
import org.mozilla.geckoview.WebExtensionController
import org.mozilla.geckoview.WebExtension.InstallException
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_USER_CANCELED

/**
* Gecko-based implementation of Engine interface.
Expand Down Expand Up @@ -266,7 +269,7 @@ class GeckoEngine(
onSuccess(updatedExtension)
GeckoResult<Void>()
}, { throwable ->
onError(extension.id, throwable)
onError(extension.id, WebExtensionException(throwable, throwable.isRecoverable()))
GeckoResult<Void>()
})
}
Expand Down Expand Up @@ -675,6 +678,8 @@ class GeckoEngine(
)
}

private fun Throwable.isRecoverable() = this is InstallException && this.code == ERROR_USER_CANCELED

companion object {
private const val PREFERENCE_NAME =
"MOZAC_GECKO_ENGINE"
Expand Down
65 changes: 65 additions & 0 deletions ...ngine-gecko-beta/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import mozilla.components.concept.engine.mediaquery.PreferredColorScheme
import mozilla.components.concept.engine.webextension.Action
import mozilla.components.concept.engine.webextension.WebExtension
import mozilla.components.concept.engine.webextension.WebExtensionDelegate
import mozilla.components.concept.engine.webextension.WebExtensionException
import mozilla.components.support.test.any
import mozilla.components.support.test.argumentCaptor
import mozilla.components.support.test.eq
Expand Down Expand Up @@ -63,12 +64,23 @@ import org.mozilla.geckoview.GeckoWebExecutor
import org.mozilla.geckoview.MockWebExtension
import org.mozilla.geckoview.StorageController
import org.mozilla.geckoview.WebExtensionController
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_CORRUPT_FILE
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_FILE_ACCESS
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_INCORRECT_HASH
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_INCORRECT_ID
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_NETWORK_FAILURE
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_POSTPONED
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_SIGNEDSTATE_REQUIRED
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_UNEXPECTED_ADDON_TYPE
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_USER_CANCELED
import org.mozilla.geckoview.WebPushController
import org.robolectric.Robolectric
import java.io.IOException
import java.lang.Exception
import org.mozilla.geckoview.WebExtension as GeckoWebExtension

typealias GeckoInstallException = org.mozilla.geckoview.WebExtension.InstallException

@RunWith(AndroidJUnit4::class)
class GeckoEngineTest {

Expand Down Expand Up @@ -1212,6 +1224,53 @@ class GeckoEngineTest {
assertNull(result)
}

@Test
fun `failures when updating MUST indicate if they are recoverable`() {
val runtime = mock<GeckoRuntime>()
val extensionController: WebExtensionController = mock()
val engine = GeckoEngine(context, runtime = runtime)

val extension = mozilla.components.browser.engine.gecko.webextension.GeckoWebExtension(
mockNativeExtension(),
runtime
)
val performUpdate: (GeckoInstallException) -> WebExtensionException = { exception ->
val updateExtensionResult = GeckoResult<GeckoWebExtension>()
whenever(extensionController.update(any())).thenReturn(updateExtensionResult)
whenever(runtime.webExtensionController).thenReturn(extensionController)
var throwable: WebExtensionException? = null

engine.updateWebExtension(
extension,
onError = { _, e -> throwable = e as WebExtensionException }
)

updateExtensionResult.completeExceptionally(exception)
throwable!!
}

val unrecoverableExceptions = listOf(
mockGeckoInstallException(ERROR_NETWORK_FAILURE),
mockGeckoInstallException(ERROR_INCORRECT_HASH),
mockGeckoInstallException(ERROR_CORRUPT_FILE),
mockGeckoInstallException(ERROR_FILE_ACCESS),
mockGeckoInstallException(ERROR_SIGNEDSTATE_REQUIRED),
mockGeckoInstallException(ERROR_UNEXPECTED_ADDON_TYPE),
mockGeckoInstallException(ERROR_INCORRECT_ID),
mockGeckoInstallException(ERROR_POSTPONED)
)

unrecoverableExceptions.forEach { exception ->
assertFalse(performUpdate(exception).isRecoverable)
}

val recoverableExceptions = listOf(mockGeckoInstallException(ERROR_USER_CANCELED))

recoverableExceptions.forEach { exception ->
assertTrue(performUpdate(exception).isRecoverable)
}
}

@Test
fun `list web extensions successfully`() {
val bundle = GeckoBundle()
Expand Down Expand Up @@ -1916,4 +1975,10 @@ class GeckoEngineTest {
}
return spy(MockWebExtension(bundle))
}

private fun mockGeckoInstallException(errorCode: Int): GeckoInstallException {
val exception = object : GeckoInstallException() {}
ReflectionUtils.setField(exception, "code", errorCode)
return exception
}
}
7 changes: 6 additions & 1 deletion ...engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngine.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import mozilla.components.concept.engine.webextension.EnableSource
import mozilla.components.concept.engine.webextension.TabHandler
import mozilla.components.concept.engine.webextension.WebExtension
import mozilla.components.concept.engine.webextension.WebExtensionDelegate
import mozilla.components.concept.engine.webextension.WebExtensionException
import mozilla.components.concept.engine.webextension.WebExtensionRuntime
import mozilla.components.concept.engine.webnotifications.WebNotificationDelegate
import mozilla.components.concept.engine.webpush.WebPushDelegate
Expand All @@ -55,6 +56,8 @@ import org.mozilla.geckoview.GeckoResult
import org.mozilla.geckoview.GeckoRuntime
import org.mozilla.geckoview.GeckoSession
import org.mozilla.geckoview.GeckoWebExecutor
import org.mozilla.geckoview.WebExtension.InstallException
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_USER_CANCELED
import org.mozilla.geckoview.WebExtensionController

/**
Expand Down Expand Up @@ -267,7 +270,7 @@ class GeckoEngine(
onSuccess(updatedExtension)
GeckoResult<Void>()
}, { throwable ->
onError(extension.id, throwable)
onError(extension.id, WebExtensionException(throwable, throwable.isRecoverable()))
GeckoResult<Void>()
})
}
Expand Down Expand Up @@ -668,6 +671,8 @@ class GeckoEngine(
)
}

private fun Throwable.isRecoverable() = this is InstallException && this.code == ERROR_USER_CANCELED

companion object {
private const val PREFERENCE_NAME =
"MOZAC_GECKO_ENGINE"
Expand Down
67 changes: 66 additions & 1 deletion ...ne-gecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/GeckoEngineTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import mozilla.components.concept.engine.mediaquery.PreferredColorScheme
import mozilla.components.concept.engine.webextension.Action
import mozilla.components.concept.engine.webextension.WebExtension
import mozilla.components.concept.engine.webextension.WebExtensionDelegate
import mozilla.components.concept.engine.webextension.WebExtensionException
import mozilla.components.support.test.any
import mozilla.components.support.test.argumentCaptor
import mozilla.components.support.test.eq
Expand Down Expand Up @@ -62,13 +63,23 @@ import org.mozilla.geckoview.GeckoSession
import org.mozilla.geckoview.GeckoWebExecutor
import org.mozilla.geckoview.MockWebExtension
import org.mozilla.geckoview.StorageController
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_CORRUPT_FILE
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_FILE_ACCESS
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_INCORRECT_HASH
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_INCORRECT_ID
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_NETWORK_FAILURE
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_POSTPONED
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_SIGNEDSTATE_REQUIRED
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_UNEXPECTED_ADDON_TYPE
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_USER_CANCELED
import org.mozilla.geckoview.WebExtensionController
import org.mozilla.geckoview.WebPushController
import org.robolectric.Robolectric
import java.io.IOException
import java.lang.Exception
import org.mozilla.geckoview.WebExtension as GeckoWebExtension

typealias GeckoInstallException = org.mozilla.geckoview.WebExtension.InstallException

@RunWith(AndroidJUnit4::class)
class GeckoEngineTest {

Expand Down Expand Up @@ -1212,6 +1223,54 @@ class GeckoEngineTest {
assertNull(result)
}

@Test
fun `failures when updating MUST indicate if they are recoverable`() {
val runtime = mock<GeckoRuntime>()
val extensionController: WebExtensionController = mock()
val engine = GeckoEngine(context, runtime = runtime)

val extension = mozilla.components.browser.engine.gecko.webextension.GeckoWebExtension(
mockNativeExtension(),
runtime
)
val performUpdate: (GeckoInstallException) -> WebExtensionException = { exception ->
val updateExtensionResult = GeckoResult<GeckoWebExtension>()
whenever(extensionController.update(any())).thenReturn(updateExtensionResult)
whenever(runtime.webExtensionController).thenReturn(extensionController)
var throwable: WebExtensionException? = null

engine.updateWebExtension(
extension,
onError = { _, e -> throwable = e as WebExtensionException
}
)

updateExtensionResult.completeExceptionally(exception)
throwable!!
}

val unrecoverableExceptions = listOf(
mockGeckoInstallException(ERROR_NETWORK_FAILURE),
mockGeckoInstallException(ERROR_INCORRECT_HASH),
mockGeckoInstallException(ERROR_CORRUPT_FILE),
mockGeckoInstallException(ERROR_FILE_ACCESS),
mockGeckoInstallException(ERROR_SIGNEDSTATE_REQUIRED),
mockGeckoInstallException(ERROR_UNEXPECTED_ADDON_TYPE),
mockGeckoInstallException(ERROR_INCORRECT_ID),
mockGeckoInstallException(ERROR_POSTPONED)
)

unrecoverableExceptions.forEach { exception ->
assertFalse(performUpdate(exception).isRecoverable)
}

val recoverableExceptions = listOf(mockGeckoInstallException(ERROR_USER_CANCELED))

recoverableExceptions.forEach { exception ->
assertTrue(performUpdate(exception).isRecoverable)
}
}

@Test
fun `list web extensions successfully`() {
val bundle = GeckoBundle()
Expand Down Expand Up @@ -1920,4 +1979,10 @@ class GeckoEngineTest {
}
return spy(MockWebExtension(bundle))
}

private fun mockGeckoInstallException(errorCode: Int): GeckoInstallException {
val exception = object : GeckoInstallException() {}
ReflectionUtils.setField(exception, "code", errorCode)
return exception
}
}
7 changes: 6 additions & 1 deletion ...browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/GeckoEngine.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import mozilla.components.concept.engine.webextension.EnableSource
import mozilla.components.concept.engine.webextension.TabHandler
import mozilla.components.concept.engine.webextension.WebExtension
import mozilla.components.concept.engine.webextension.WebExtensionDelegate
import mozilla.components.concept.engine.webextension.WebExtensionException
import mozilla.components.concept.engine.webextension.WebExtensionRuntime
import mozilla.components.concept.engine.webnotifications.WebNotificationDelegate
import mozilla.components.concept.engine.webpush.WebPushDelegate
Expand All @@ -54,6 +55,8 @@ import org.mozilla.geckoview.GeckoResult
import org.mozilla.geckoview.GeckoRuntime
import org.mozilla.geckoview.GeckoSession
import org.mozilla.geckoview.GeckoWebExecutor
import org.mozilla.geckoview.WebExtension.InstallException
import org.mozilla.geckoview.WebExtension.InstallException.ErrorCodes.ERROR_USER_CANCELED
import org.mozilla.geckoview.WebExtensionController

/**
Expand Down Expand Up @@ -266,7 +269,7 @@ class GeckoEngine(
onSuccess(updatedExtension)
GeckoResult<Void>()
}, { throwable ->
onError(extension.id, throwable)
onError(extension.id, WebExtensionException(throwable, throwable.isRecoverable()))
GeckoResult<Void>()
})
}
Expand Down Expand Up @@ -675,6 +678,8 @@ class GeckoEngine(
)
}

private fun Throwable.isRecoverable() = this is InstallException && this.code == ERROR_USER_CANCELED

companion object {
private const val PREFERENCE_NAME =
"MOZAC_GECKO_ENGINE"
Expand Down
Loading

0 comments on commit a4d2e8b

Please sign in to comment.