Closed
Bug 1209496
Opened 9 years ago
Closed 8 years ago
DownloadContentService: Implement proxy support
Categories
(Firefox for Android Graveyard :: General, defect)
Tracking
(firefox49 fixed)
RESOLVED
FIXED
Firefox 49
Tracking | Status | |
---|---|---|
firefox49 | --- | fixed |
People
(Reporter: sebastian, Assigned: k.krish, Mentored)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
sebastian
:
review+
|
Details | Diff | Splinter Review |
Follow-up from bug 1197720.
Implement proxy support for DownloadContentService.
Reporter | ||
Comment 1•9 years ago
|
||
Richard: I filed this bug after your comment here: bug 1197720 comment 15. Do I need to manually configure a proxy to be used by HttpClient in some situations? Is there some code in sync I can look at?
Flags: needinfo?(rnewman)
Comment 2•9 years ago
|
||
We have some code to help:
java/org/mozilla/gecko/util/ProxySelector.java
but if you're using HttpClient, you're basically implementing Bug 942652. Not too hard, but it hasn't been done yet.
Depends on: 942652
Flags: needinfo?(rnewman)
Reporter | ||
Comment 3•9 years ago
|
||
Hey Krishna!
This is a first simple bug.
We are creating the HttpUrlConnection for the DownloadContentService here:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/dlc/BaseAction.java#146
And we have a helper method to add the proxies to an URLConnection here:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/util/ProxySelector.java#31
You basically just need to connect the both parts. :)
Assignee: nobody → k.krish
Status: NEW → ASSIGNED
Hey Sebastian,
I will look into the code and come up with the patch real soon :)
Hey Sebastian, Please review the patch I have submitted. Please let me know if anything is not correct.
Attachment #8747441 -
Flags: review?(s.kaspari)
Comment on attachment 8747441 [details]
Bug1209496.diff
>diff --git a/mobile/android/base/java/org/mozilla/gecko/dlc/BaseAction.java b/mobile/android/base/java/org/mozilla/gecko/dlc/BaseAction.java
>--- a/mobile/android/base/java/org/mozilla/gecko/dlc/BaseAction.java
>+++ b/mobile/android/base/java/org/mozilla/gecko/dlc/BaseAction.java
>@@ -11,25 +11,27 @@ import android.util.Log;
>
> import org.mozilla.gecko.AppConstants;
> import org.mozilla.gecko.background.nativecode.NativeCrypto;
> import org.mozilla.gecko.dlc.catalog.DownloadContent;
> import org.mozilla.gecko.dlc.catalog.DownloadContentCatalog;
> import org.mozilla.gecko.sync.Utils;
> import org.mozilla.gecko.util.HardwareUtils;
> import org.mozilla.gecko.util.IOUtils;
>+import org.mozilla.gecko.util.ProxySelector;
>
> import java.io.BufferedInputStream;
> import java.io.File;
> import java.io.FileInputStream;
> import java.io.IOException;
> import java.io.InputStream;
> import java.net.HttpURLConnection;
> import java.net.MalformedURLException;
>-import java.net.URL;
>+import java.net.URI;
>+import java.net.URISyntaxException;
>
> public abstract class BaseAction {
> private static final String LOGTAG = "GeckoDLCBaseAction";
>
> /**
> * Exception indicating a recoverable error has happened. Download of the content will be retried later.
> */
> /* package-private */ static class RecoverableDownloadContentException extends Exception {
>@@ -140,24 +142,26 @@ public abstract class BaseAction {
> throw new RecoverableDownloadContentException(RecoverableDownloadContentException.DISK_IO, e);
> } finally {
> IOUtils.safeStreamClose(inputStream);
> }
> }
>
> protected HttpURLConnection buildHttpURLConnection(String url)
> throws UnrecoverableDownloadContentException, IOException {
>- // TODO: Implement proxy support (Bug 1209496)
>+
> try {
> System.setProperty("http.keepAlive", "true");
>
>- HttpURLConnection connection = (HttpURLConnection) new URL(url).openConnection();
>+ HttpURLConnection connection = (HttpURLConnection) ProxySelector.openConnectionWithProxy(new URI(url));
> connection.setRequestProperty("User-Agent", HardwareUtils.isTablet() ?
> AppConstants.USER_AGENT_FENNEC_TABLET :
> AppConstants.USER_AGENT_FENNEC_MOBILE);
> connection.setRequestMethod("GET");
> connection.setInstanceFollowRedirects(true);
> return connection;
> } catch (MalformedURLException e) {
> throw new UnrecoverableDownloadContentException(e);
>+ } catch (URISyntaxException e) {
>+ throw new UnrecoverableDownloadContentException(e);
> }
> }
> }
Attachment #8747441 -
Attachment description: Using ProxySelector Class for creating a HttpURLConnection → Bug1209496.diff
Reporter | ||
Comment 7•8 years ago
|
||
Comment on attachment 8747441 [details]
Bug1209496.diff
Review of attachment 8747441 [details]:
-----------------------------------------------------------------
This looks good.
Can you upload a patch with the nit fixed? I'll push it to the try server afterwards.
::: mobile/android/base/java/org/mozilla/gecko/dlc/BaseAction.java
@@ +146,5 @@
> }
>
> protected HttpURLConnection buildHttpURLConnection(String url)
> throws UnrecoverableDownloadContentException, IOException {
> +
nit: Remove this empty line. :)
Attachment #8747441 -
Flags: review?(s.kaspari) → review+
Attachment #8747441 -
Attachment is obsolete: true
Attachment #8747441 -
Attachment is patch: false
Attachment #8747441 -
Flags: review+
Updated code. Removed the empty line. :)
Attachment #8749947 -
Flags: review?(s.kaspari)
Reporter | ||
Comment 9•8 years ago
|
||
Comment on attachment 8749947 [details] [diff] [review]
Bug1209496
Review of attachment 8749947 [details] [diff] [review]:
-----------------------------------------------------------------
LGTM.
For the next patches: Are you exporting your patches using Mercurial? They do not contain a header (No author, no commit message etc.).
Attachment #8749947 -
Flags: review?(s.kaspari) → review+
Reporter | ||
Comment 10•8 years ago
|
||
Reporter | ||
Comment 11•8 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/e9a379142b334234d148ea4c3b89edf259a1668b
Bug 1209496 - DownloadContentService: Add proxy support. r=sebastian
Reporter | ||
Comment 12•8 years ago
|
||
(In reply to Sebastian Kaspari (:sebastian) from comment #11)
> https://hg.mozilla.org/integration/fx-team/rev/
> e9a379142b334234d148ea4c3b89edf259a1668b
> Bug 1209496 - DownloadContentService: Add proxy support. r=sebastian
I added commit message and author and landed the patch.
Comment 13•8 years ago
|
||
bugherder |
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 49
Updated•4 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•