Skip to content

Commit

Permalink
Feature/performance tweaks (duckduckgo#481)
Browse files Browse the repository at this point in the history
* Add utility class to walkback url subdomains

* Remove unused methods on Terms of Service store

* Add new entityMapping.entityForUrl implementation

* Stop pre-loading entity list into memory

* Add method to query DB directly for matching entity

* Updating gradle and AGP

* Merging in latest develop

* [WIP] compiling and tests passing

* Update test to ensure it is called from a coroutine

* Restore logic of PrivacyPractices to precompute values for network

* Minor code tidy

* Ensure correct prevalence score

* Some code tidy; preparing for PR

* Code tidy

* code tidy

* Code tidy

* Code tidy

* Code tidy

* Remove production calls to measureExecution

* Wrap new tab opening in a launch as it's now a suspend method

* Code tidy

* Rename subdomain remover method, and better rely on standard functions

* Code tidy; rename some methods and remove nullability of a param

* Rename class for better clarity

* Fix style of instantiating test object

* Rename `Initializer`

* Rename SiteDetails to SitePrivacyData

* Clarify comment
  • Loading branch information
CDRussell authored May 3, 2019
1 parent 2b82331 commit bfb6e52
Show file tree
Hide file tree
Showing 60 changed files with 1,010 additions and 432 deletions.
6 changes: 3 additions & 3 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ ext {
androidKtx = "1.0.1"
fragmentKtx = "1.0.0"
constraintLayout = "2.0.0-alpha3"
lifecycle = "2.0.0"
lifecycle = "2.1.0-alpha04"
room = "2.1.0-alpha05"
workManager = "2.0.0"
legacySupport = "1.0.0"
Expand Down Expand Up @@ -113,8 +113,6 @@ ext {

dependencies {
implementation "androidx.legacy:legacy-support-v4:$legacySupport"
implementation 'androidx.lifecycle:lifecycle-extensions:2.0.0-beta01'
implementation 'androidx.lifecycle:lifecycle-viewmodel-ktx:2.0.0'
debugImplementation "com.squareup.leakcanary:leakcanary-android:$leakCanary"
releaseImplementation "com.squareup.leakcanary:leakcanary-android-no-op:$leakCanary"

Expand Down Expand Up @@ -147,6 +145,8 @@ dependencies {

// ViewModel and LiveData
implementation "androidx.lifecycle:lifecycle-extensions:$lifecycle"
implementation "androidx.lifecycle:lifecycle-viewmodel-ktx:$lifecycle"

kapt "androidx.lifecycle:lifecycle-compiler:$lifecycle"
testImplementation "androidx.arch.core:core-testing:$coreTesting"
androidTestImplementation "androidx.arch.core:core-testing:$coreTesting"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
* limitations under the License.
*/

@file:Suppress("RemoveExplicitTypeArguments")

package com.duckduckgo.app.browser

import android.content.Context
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
* limitations under the License.
*/

@file:Suppress("RemoveExplicitTypeArguments")

package com.duckduckgo.app.browser

import android.view.MenuItem
Expand All @@ -36,11 +38,11 @@ import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.DownloadFile
import com.duckduckgo.app.browser.LongPressHandler.RequiredAction.OpenInNewTab
import com.duckduckgo.app.browser.addtohome.AddToHomeCapabilityDetector
import com.duckduckgo.app.browser.favicon.FaviconDownloader
import com.duckduckgo.app.browser.model.BasicAuthenticationCredentials
import com.duckduckgo.app.browser.model.BasicAuthenticationRequest
import com.duckduckgo.app.browser.model.LongPressTarget
import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter
import com.duckduckgo.app.browser.session.WebViewSessionStorage
import com.duckduckgo.app.browser.model.BasicAuthenticationCredentials
import com.duckduckgo.app.browser.model.BasicAuthenticationRequest
import com.duckduckgo.app.cta.db.DismissedCtaDao
import com.duckduckgo.app.cta.ui.CtaViewModel
import com.duckduckgo.app.global.db.AppConfigurationDao
Expand All @@ -64,6 +66,7 @@ import com.duckduckgo.app.trackerdetection.model.TrackingEvent
import com.duckduckgo.app.usage.search.SearchCountDao
import com.duckduckgo.app.widget.ui.WidgetCapabilities
import com.nhaarman.mockitokotlin2.*
import kotlinx.coroutines.runBlocking
import org.junit.After
import org.junit.Assert.*
import org.junit.Before
Expand Down Expand Up @@ -178,9 +181,11 @@ class BrowserTabViewModelTest {
)

val siteFactory = SiteFactory(mockPrivacyPractices, mockTrackerNetworks, prevalenceStore = mockPrevalenceStore)
whenever(mockTabsRepository.retrieveSiteData(any())).thenReturn(MutableLiveData())
whenever(mockPrivacyPractices.privacyPracticesFor(any())).thenReturn(PrivacyPractices.UNKNOWN)
whenever(mockAppInstallStore.installTimestamp).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(1))
runBlocking<Unit> {
whenever(mockTabsRepository.retrieveSiteData(any())).thenReturn(MutableLiveData())
whenever(mockPrivacyPractices.privacyPracticesFor(any())).thenReturn(PrivacyPractices.UNKNOWN)
whenever(mockAppInstallStore.installTimestamp).thenReturn(System.currentTimeMillis() - TimeUnit.DAYS.toMillis(1))
}

testee = BrowserTabViewModel(
statisticsUpdater = mockStatisticsUpdater,
Expand Down Expand Up @@ -237,7 +242,7 @@ class BrowserTabViewModelTest {
}

@Test
fun whenOpenInNewBackgroundRequestedThenTabRepositoryUpdatedAndCommandIssued() {
fun whenOpenInNewBackgroundRequestedThenTabRepositoryUpdatedAndCommandIssued() = runBlocking<Unit> {
val url = "http://www.example.com"
testee.openInNewBackgroundTab(url)

Expand Down Expand Up @@ -553,13 +558,13 @@ class BrowserTabViewModelTest {
}

@Test
fun whenLoadingStartedThenPrivacyGradeIsCleared() {
fun whenLoadingStartedThenPrivacyGradeIsCleared() = runBlocking<Unit> {
testee.loadingStarted("http://example.com")
assertNull(testee.privacyGrade.value)
}

@Test
fun whenUrlChangedThenPrivacyGradeIsReset() {
fun whenUrlChangedThenPrivacyGradeIsReset() = runBlocking<Unit> {
val grade = testee.privacyGrade.value
changeUrl("https://example.com")
assertNotEquals(grade, testee.privacyGrade.value)
Expand Down Expand Up @@ -984,7 +989,7 @@ class BrowserTabViewModelTest {
}

@Test
fun whenOnSiteAndBrokenSiteSelectedThenBrokenSiteFeedbackCommandSentWithUrl() {
fun whenOnSiteAndBrokenSiteSelectedThenBrokenSiteFeedbackCommandSentWithUrl() = runBlocking<Unit> {
isBrowsing(true)
changeUrl("foo.com")
testee.onBrokenSiteSelected()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import com.duckduckgo.app.privacy.ui.PrivacyDashboardActivity
import com.duckduckgo.app.tabs.model.TabEntity
import com.duckduckgo.app.tabs.model.TabRepository
import com.nhaarman.mockitokotlin2.*
import kotlinx.coroutines.runBlocking
import org.junit.After
import org.junit.Assert.assertEquals
import org.junit.Assert.assertTrue
Expand All @@ -50,10 +51,10 @@ class BrowserViewModelTest {
var instantTaskExecutorRule = InstantTaskExecutorRule()

@Mock
private lateinit var mockCommandObserver: Observer<BrowserViewModel.Command>
private lateinit var mockCommandObserver: Observer<Command>

@Captor
private lateinit var commandCaptor: ArgumentCaptor<BrowserViewModel.Command>
private lateinit var commandCaptor: ArgumentCaptor<Command>

@Mock
private lateinit var mockTabRepository: TabRepository
Expand Down Expand Up @@ -86,8 +87,11 @@ class BrowserViewModelTest {
appEnjoymentUserEventRecorder = mockAppEnjoymentUserEventRecorder
)
testee.command.observeForever(mockCommandObserver)
whenever(mockTabRepository.add()).thenReturn(TAB_ID)
whenever(mockOmnibarEntryConverter.convertQueryToUrl(any())).then { it.arguments.first() }

runBlocking<Unit> {
whenever(mockTabRepository.add()).thenReturn(TAB_ID)
whenever(mockOmnibarEntryConverter.convertQueryToUrl(any())).then { it.arguments.first() }
}
}

@After
Expand All @@ -96,26 +100,26 @@ class BrowserViewModelTest {
}

@Test
fun whenNewTabRequestedThenTabAddedToRepository() {
fun whenNewTabRequestedThenTabAddedToRepository() = runBlocking<Unit> {
testee.onNewTabRequested()
verify(mockTabRepository).add()
}

@Test
fun whenOpenInNewTabRequestedThenTabAddedToRepository() {
fun whenOpenInNewTabRequestedThenTabAddedToRepository() = runBlocking<Unit> {
val url = "http://example.com"
testee.onOpenInNewTabRequested(url)
verify(mockTabRepository).add(url)
}

@Test
fun whenTabsUpdatedAndNoTabsThenNewTabAddedToRepository() {
fun whenTabsUpdatedAndNoTabsThenNewTabAddedToRepository() = runBlocking<Unit> {
testee.onTabsUpdated(ArrayList())
verify(mockTabRepository).add(null, false, true)
}

@Test
fun whenTabsUpdatedWithTabsThenNewTabNotLaunched() {
fun whenTabsUpdatedWithTabsThenNewTabNotLaunched() = runBlocking {
testee.onTabsUpdated(asList(TabEntity(TAB_ID, "", "", false, true, 0)))
verify(mockCommandObserver, never()).onChanged(any())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import com.duckduckgo.app.statistics.store.StatisticsDataStore
import com.nhaarman.mockitokotlin2.any
import com.nhaarman.mockitokotlin2.mock
import com.nhaarman.mockitokotlin2.verify
import kotlinx.coroutines.runBlocking
import org.junit.Before
import org.junit.Test

Expand Down Expand Up @@ -68,7 +69,7 @@ class BrowserWebViewClientTest {

@UiThreadTest
@Test
fun whenOnPageFinishedCalledThenListenerNotified() {
fun whenOnPageFinishedCalledThenListenerNotified() = runBlocking {
testee.onPageFinished(webView, EXAMPLE_URL)
verify(listener).loadingFinished(EXAMPLE_URL)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
* limitations under the License.
*/

@file:Suppress("RemoveExplicitTypeArguments")

package com.duckduckgo.app.browser

import android.net.Uri
Expand All @@ -29,6 +31,7 @@ import com.duckduckgo.app.surrogates.SurrogateResponse
import com.duckduckgo.app.trackerdetection.TrackerDetector
import com.duckduckgo.app.trackerdetection.model.TrackingEvent
import com.nhaarman.mockitokotlin2.*
import kotlinx.coroutines.runBlocking
import org.junit.Assert.*
import org.junit.Before
import org.junit.Test
Expand Down Expand Up @@ -65,7 +68,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenUrlShouldBeUpgradedThenUpgraderInvoked() {
fun whenUrlShouldBeUpgradedThenUpgraderInvoked() = runBlocking<Unit> {
configureShouldUpgrade()
testee.shouldIntercept(
request = mockRequest,
Expand All @@ -78,7 +81,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenUrlShouldBeUpgradedThenCancelledResponseReturned() {
fun whenUrlShouldBeUpgradedThenCancelledResponseReturned() = runBlocking<Unit> {
configureShouldUpgrade()
val response = testee.shouldIntercept(
request = mockRequest,
Expand All @@ -91,7 +94,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenUrlShouldBeUpgradedButNotOnMainFrameThenNotUpgraded() {
fun whenUrlShouldBeUpgradedButNotOnMainFrameThenNotUpgraded() = runBlocking<Unit> {
configureShouldUpgrade()
whenever(mockRequest.isForMainFrame).thenReturn(false)
testee.shouldIntercept(
Expand All @@ -105,7 +108,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenUrlShouldBeUpgradedButUrlIsNullThenNotUpgraded() {
fun whenUrlShouldBeUpgradedButUrlIsNullThenNotUpgraded() = runBlocking<Unit> {
configureShouldUpgrade()
whenever(mockRequest.url).thenReturn(null)
testee.shouldIntercept(
Expand All @@ -119,7 +122,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenUrlShouldNotBeUpgradedThenUpgraderNotInvoked() {
fun whenUrlShouldNotBeUpgradedThenUpgraderNotInvoked() = runBlocking<Unit> {
whenever(mockHttpsUpgrader.shouldUpgrade(any())).thenReturn(false)
testee.shouldIntercept(
request = mockRequest,
Expand All @@ -132,7 +135,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenCurrentUrlIsNullThenShouldContinueToLoad() {
fun whenCurrentUrlIsNullThenShouldContinueToLoad() = runBlocking<Unit> {
configureShouldNotUpgrade()
val response = testee.shouldIntercept(
request = mockRequest,
Expand All @@ -144,7 +147,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenIsTrustedSite_DuckDuckGo_ThenShouldContinueToLoad() {
fun whenIsTrustedSite_DuckDuckGo_ThenShouldContinueToLoad() = runBlocking<Unit> {
configureShouldNotUpgrade()
val response = testee.shouldIntercept(
request = mockRequest,
Expand All @@ -157,7 +160,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenIsTrustedSite_DontTrack_ThenShouldContinueToLoad() {
fun whenIsTrustedSite_DontTrack_ThenShouldContinueToLoad() = runBlocking<Unit> {
configureShouldNotUpgrade()
val response = testee.shouldIntercept(
request = mockRequest,
Expand All @@ -170,7 +173,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenIsTrustedSite_SpreadPrivacy_ThenShouldContinueToLoad() {
fun whenIsTrustedSite_SpreadPrivacy_ThenShouldContinueToLoad() = runBlocking<Unit> {
configureShouldNotUpgrade()
val response = testee.shouldIntercept(
request = mockRequest,
Expand All @@ -183,7 +186,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenIsTrustedSite_DuckDuckHack_ThenShouldContinueToLoad() {
fun whenIsTrustedSite_DuckDuckHack_ThenShouldContinueToLoad() = runBlocking<Unit> {
configureShouldNotUpgrade()
val response = testee.shouldIntercept(
request = mockRequest,
Expand All @@ -196,7 +199,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenIsTrustedSite_PrivateBrowsingMyths_ThenShouldContinueToLoad() {
fun whenIsTrustedSite_PrivateBrowsingMyths_ThenShouldContinueToLoad() = runBlocking<Unit> {
configureShouldNotUpgrade()
val response = testee.shouldIntercept(
request = mockRequest,
Expand All @@ -209,7 +212,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenIsTrustedSite_DuckDotCo_ThenShouldContinueToLoad() {
fun whenIsTrustedSite_DuckDotCo_ThenShouldContinueToLoad() = runBlocking<Unit> {
configureShouldNotUpgrade()
val response = testee.shouldIntercept(
request = mockRequest,
Expand All @@ -222,7 +225,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenIsHttpRequestThenHttpRequestListenerCalled() {
fun whenIsHttpRequestThenHttpRequestListenerCalled() = runBlocking<Unit> {
configureShouldNotUpgrade()
whenever(mockRequest.url).thenReturn(Uri.parse("http://example.com"))
val mockListener = mock<WebViewClientListener>()
Expand All @@ -239,7 +242,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenIsHttpsRequestThenHttpRequestListenerNotCalled() {
fun whenIsHttpsRequestThenHttpRequestListenerNotCalled() = runBlocking<Unit> {
configureShouldNotUpgrade()
whenever(mockRequest.url).thenReturn(Uri.parse("https://example.com"))
val mockListener = mock<WebViewClientListener>()
Expand All @@ -257,7 +260,7 @@ class WebViewRequestInterceptorTest {


@Test
fun whenRequestShouldBlockAndNoSurrogateThenCancellingResponseReturned() {
fun whenRequestShouldBlockAndNoSurrogateThenCancellingResponseReturned() = runBlocking<Unit> {
whenever(mockResourceSurrogates.get(any())).thenReturn(SurrogateResponse(responseAvailable = false))

configureShouldNotUpgrade()
Expand All @@ -273,7 +276,7 @@ class WebViewRequestInterceptorTest {
}

@Test
fun whenRequestShouldBlockButThereIsASurrogateThen() {
fun whenRequestShouldBlockButThereIsASurrogateThen() = runBlocking<Unit> {
val availableSurrogate = SurrogateResponse(
responseAvailable = true,
mimeType = "application/javascript",
Expand Down Expand Up @@ -308,13 +311,13 @@ class WebViewRequestInterceptorTest {
whenever(mockTrackerDetector.evaluate(any(), any(), any())).thenReturn(blockTrackingEvent)
}

private fun configureShouldUpgrade() {
private fun configureShouldUpgrade() = runBlocking<Unit> {
whenever(mockHttpsUpgrader.shouldUpgrade(any())).thenReturn(true)
whenever(mockRequest.url).thenReturn(validUri())
whenever(mockRequest.isForMainFrame).thenReturn(true)
}

private fun configureShouldNotUpgrade() {
private fun configureShouldNotUpgrade() = runBlocking<Unit> {
whenever(mockHttpsUpgrader.shouldUpgrade(any())).thenReturn(false)

whenever(mockRequest.url).thenReturn(validUri())
Expand Down
Loading

0 comments on commit bfb6e52

Please sign in to comment.