Skip to content

Commit 6979032

Browse files
jonalmeidamergify[bot]
jonalmeida
authored and
mergify[bot]
committed
Re-land: "Closes mozilla-mobile#8693: Integrate GeckoWebExecutor#FETCH_FLAG_PRIVATE."
This re-lands commit 2d1842d.
1 parent 799e6f8 commit 6979032

File tree

11 files changed

+106
-37
lines changed

11 files changed

+106
-37
lines changed

‎components/browser/engine-gecko-beta/src/main/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchClient.kt

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,7 @@ class GeckoViewFetchClient(
5454
}
5555

5656
return try {
57-
var fetchFlags = 0
58-
if (request.cookiePolicy == Request.CookiePolicy.OMIT) {
59-
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_ANONYMOUS
60-
}
61-
if (request.redirect == Request.Redirect.MANUAL) {
62-
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_NO_REDIRECTS
63-
}
64-
val webResponse = executor.fetch(webRequest, fetchFlags).poll(readTimeOutMillis)
57+
val webResponse = executor.fetch(webRequest, request.fetchFlags).poll(readTimeOutMillis)
6558
webResponse?.toResponse() ?: throw IOException("Fetch failed with null response")
6659
} catch (e: TimeoutException) {
6760
throw SocketTimeoutException()
@@ -70,6 +63,21 @@ class GeckoViewFetchClient(
7063
}
7164
}
7265

66+
private val Request.fetchFlags: Int
67+
get() {
68+
var fetchFlags = 0
69+
if (cookiePolicy == Request.CookiePolicy.OMIT) {
70+
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_ANONYMOUS
71+
}
72+
if (private) {
73+
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_PRIVATE
74+
}
75+
if (redirect == Request.Redirect.MANUAL) {
76+
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_NO_REDIRECTS
77+
}
78+
return fetchFlags
79+
}
80+
7381
companion object {
7482
const val MAX_READ_TIMEOUT_MINUTES = 5L
7583
}

‎components/browser/engine-gecko-beta/src/test/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchUnitTestCases.kt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,19 @@ class GeckoViewFetchUnitTestCases : FetchTestCases() {
222222
verify(geckoWebExecutor)!!.fetch(any(), eq(GeckoWebExecutor.FETCH_FLAGS_ANONYMOUS))
223223
}
224224

225+
@Test
226+
fun performPrivateRequest() {
227+
mockResponse(200)
228+
229+
val request = mock<Request>()
230+
whenever(request.url).thenReturn("https://mozilla.org")
231+
whenever(request.method).thenReturn(Request.Method.GET)
232+
whenever(request.private).thenReturn(true)
233+
createNewClient().fetch(request)
234+
235+
verify(geckoWebExecutor)!!.fetch(any(), eq(GeckoWebExecutor.FETCH_FLAGS_PRIVATE))
236+
}
237+
225238
@Test
226239
override fun get200WithContentTypeCharset() {
227240
val request = mock<Request>()

‎components/browser/engine-gecko-nightly/src/main/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchClient.kt

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,7 @@ class GeckoViewFetchClient(
5454
}
5555

5656
return try {
57-
var fetchFlags = 0
58-
if (request.cookiePolicy == Request.CookiePolicy.OMIT) {
59-
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_ANONYMOUS
60-
}
61-
if (request.redirect == Request.Redirect.MANUAL) {
62-
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_NO_REDIRECTS
63-
}
64-
val webResponse = executor.fetch(webRequest, fetchFlags).poll(readTimeOutMillis)
57+
val webResponse = executor.fetch(webRequest, request.fetchFlags).poll(readTimeOutMillis)
6558
webResponse?.toResponse() ?: throw IOException("Fetch failed with null response")
6659
} catch (e: TimeoutException) {
6760
throw SocketTimeoutException()
@@ -70,6 +63,21 @@ class GeckoViewFetchClient(
7063
}
7164
}
7265

66+
private val Request.fetchFlags: Int
67+
get() {
68+
var fetchFlags = 0
69+
if (cookiePolicy == Request.CookiePolicy.OMIT) {
70+
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_ANONYMOUS
71+
}
72+
if (private) {
73+
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_PRIVATE
74+
}
75+
if (redirect == Request.Redirect.MANUAL) {
76+
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_NO_REDIRECTS
77+
}
78+
return fetchFlags
79+
}
80+
7381
companion object {
7482
const val MAX_READ_TIMEOUT_MINUTES = 5L
7583
}

‎components/browser/engine-gecko-nightly/src/test/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchUnitTestCases.kt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,19 @@ class GeckoViewFetchUnitTestCases : FetchTestCases() {
222222
verify(geckoWebExecutor)!!.fetch(any(), eq(GeckoWebExecutor.FETCH_FLAGS_ANONYMOUS))
223223
}
224224

225+
@Test
226+
fun performPrivateRequest() {
227+
mockResponse(200)
228+
229+
val request = mock<Request>()
230+
whenever(request.url).thenReturn("https://mozilla.org")
231+
whenever(request.method).thenReturn(Request.Method.GET)
232+
whenever(request.private).thenReturn(true)
233+
createNewClient().fetch(request)
234+
235+
verify(geckoWebExecutor)!!.fetch(any(), eq(GeckoWebExecutor.FETCH_FLAGS_PRIVATE))
236+
}
237+
225238
@Test
226239
override fun get200WithContentTypeCharset() {
227240
val request = mock<Request>()

‎components/browser/engine-gecko/src/main/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchClient.kt

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,7 @@ class GeckoViewFetchClient(
5454
}
5555

5656
return try {
57-
var fetchFlags = 0
58-
if (request.cookiePolicy == Request.CookiePolicy.OMIT) {
59-
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_ANONYMOUS
60-
}
61-
if (request.redirect == Request.Redirect.MANUAL) {
62-
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_NO_REDIRECTS
63-
}
64-
val webResponse = executor.fetch(webRequest, fetchFlags).poll(readTimeOutMillis)
57+
val webResponse = executor.fetch(webRequest, request.fetchFlags).poll(readTimeOutMillis)
6558
webResponse?.toResponse() ?: throw IOException("Fetch failed with null response")
6659
} catch (e: TimeoutException) {
6760
throw SocketTimeoutException()
@@ -70,6 +63,21 @@ class GeckoViewFetchClient(
7063
}
7164
}
7265

66+
private val Request.fetchFlags: Int
67+
get() {
68+
var fetchFlags = 0
69+
if (cookiePolicy == Request.CookiePolicy.OMIT) {
70+
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_ANONYMOUS
71+
}
72+
if (private) {
73+
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_PRIVATE
74+
}
75+
if (redirect == Request.Redirect.MANUAL) {
76+
fetchFlags += GeckoWebExecutor.FETCH_FLAGS_NO_REDIRECTS
77+
}
78+
return fetchFlags
79+
}
80+
7381
companion object {
7482
const val MAX_READ_TIMEOUT_MINUTES = 5L
7583
}

‎components/browser/engine-gecko/src/test/java/mozilla/components/browser/engine/gecko/fetch/GeckoViewFetchUnitTestCases.kt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,19 @@ class GeckoViewFetchUnitTestCases : FetchTestCases() {
222222
verify(geckoWebExecutor)!!.fetch(any(), eq(GeckoWebExecutor.FETCH_FLAGS_ANONYMOUS))
223223
}
224224

225+
@Test
226+
fun performPrivateRequest() {
227+
mockResponse(200)
228+
229+
val request = mock<Request>()
230+
whenever(request.url).thenReturn("https://mozilla.org")
231+
whenever(request.method).thenReturn(Request.Method.GET)
232+
whenever(request.private).thenReturn(true)
233+
createNewClient().fetch(request)
234+
235+
verify(geckoWebExecutor)!!.fetch(any(), eq(GeckoWebExecutor.FETCH_FLAGS_PRIVATE))
236+
}
237+
225238
@Test
226239
override fun get200WithContentTypeCharset() {
227240
val request = mock<Request>()

‎components/concept/fetch/src/main/java/mozilla/components/concept/fetch/Request.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ import java.util.concurrent.TimeUnit
3333
* sent with the request, defaults to [CookiePolicy.INCLUDE]
3434
* @property useCaches Whether caches should be used or a network request
3535
* should be forced, defaults to true (use caches).
36-
*
36+
* @property private Whether the request should be performed in a private context, defaults to false.
37+
* The feature is not support in all [Client]s, check support before using.
3738
* @see [Headers.Names]
3839
* @see [Headers.Values]
3940
*/
@@ -46,7 +47,8 @@ data class Request(
4647
val body: Body? = null,
4748
val redirect: Redirect = Redirect.FOLLOW,
4849
val cookiePolicy: CookiePolicy = CookiePolicy.INCLUDE,
49-
val useCaches: Boolean = true
50+
val useCaches: Boolean = true,
51+
val private: Boolean = false
5052
) {
5153
/**
5254
* A [Body] to be send with the [Request].

‎components/feature/downloads/src/main/java/mozilla/components/feature/downloads/AbstractFetchDownloadService.kt

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -643,14 +643,12 @@ abstract class AbstractFetchDownloadService : Service() {
643643
headers.append(RANGE, "bytes=${currentDownloadJobState.currentBytesCopied}-")
644644
}
645645

646-
val cookiePolicy = if (download.private) {
647-
Request.CookiePolicy.OMIT
648-
} else {
649-
Request.CookiePolicy.INCLUDE
650-
}
651-
652646
var isUsingHttpClient = false
653-
val request = Request(download.url.sanitizeURL(), headers = headers, cookiePolicy = cookiePolicy)
647+
val request = Request(
648+
download.url.sanitizeURL(),
649+
headers = headers,
650+
private = download.private
651+
)
654652
// When resuming a download we need to use the httpClient as
655653
// download.response doesn't support adding headers.
656654
val response = if (isResumingDownload || useHttpClient || download.response == null) {

‎components/feature/downloads/src/test/java/mozilla/components/feature/downloads/AbstractFetchDownloadServiceTest.kt

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1062,7 +1062,7 @@ class AbstractFetchDownloadServiceTest {
10621062
}
10631063

10641064
@Test
1065-
fun `WHEN a download is from a private session the client must include the correct CookiePolicy`() = runBlocking {
1065+
fun `WHEN a download is from a private session the request must be private`() = runBlocking {
10661066
val response = Response(
10671067
"https://example.com/file.txt",
10681068
200,
@@ -1076,15 +1076,14 @@ class AbstractFetchDownloadServiceTest {
10761076

10771077
service.performDownload(downloadJob)
10781078
verify(client).fetch(providedRequest.capture())
1079-
1080-
assertEquals(Request.CookiePolicy.OMIT, providedRequest.value.cookiePolicy)
1079+
assertTrue(providedRequest.value.private)
10811080

10821081
downloadJob.state = download.copy(private = false)
10831082
service.performDownload(downloadJob)
10841083

10851084
verify(client, times(2)).fetch(providedRequest.capture())
10861085

1087-
assertEquals(Request.CookiePolicy.INCLUDE, providedRequest.value.cookiePolicy)
1086+
assertFalse(providedRequest.value.private)
10881087
}
10891088

10901089
@Test

‎components/lib/fetch-httpurlconnection/src/main/java/mozilla/components/lib/fetch/httpurlconnection/HttpURLConnectionClient.kt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ class HttpURLConnectionClient : Client() {
3232

3333
@Throws(IOException::class)
3434
override fun fetch(request: Request): Response {
35+
if (request.private) {
36+
throw IllegalArgumentException("Client doesn't support private request")
37+
}
3538
if (request.isDataUri()) {
3639
return fetchDataUri(request)
3740
}

‎components/lib/fetch-okhttp/src/main/java/mozilla/components/lib/fetch/okhttp/OkHttpClient.kt

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ class OkHttpClient(
3737
)
3838

3939
override fun fetch(request: Request): Response {
40+
if (request.private) {
41+
throw IllegalArgumentException("Client doesn't support private request")
42+
}
43+
4044
if (request.isDataUri()) {
4145
return fetchDataUri(request)
4246
}

0 commit comments

Comments
 (0)