Closed
Bug 1208587
Opened 9 years ago
Closed 9 years ago
SUTAgent is using Apache HttpClient library that has been removed in Android API Level 23
Categories
(Firefox for Android Graveyard :: Testing, defect)
Tracking
(firefox44 fixed)
RESOLVED
FIXED
Firefox 44
Tracking | Status | |
---|---|---|
firefox44 | --- | fixed |
People
(Reporter: sebastian, Assigned: sebastian)
References
Details
Attachments
(3 files)
3.39 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
4.55 KB,
patch
|
mcomella
:
review+
|
Details | Diff | Splinter Review |
3.05 KB,
patch
|
jmaher
:
review+
|
Details | Diff | Splinter Review |
SUTAgent uses org.apache.http.*. This package has been removed in Android API level 23 (M / 6.0). Build errors: https://pastebin.mozilla.org/8847539 Fennec switched from org.apache to ch.boye in bug 1169421.
Assignee | ||
Comment 1•9 years ago
|
||
(In reply to Sebastian Kaspari (:sebastian) from comment #0) > Fennec switched from org.apache to ch.boye in bug 1169421. Another option would be to switch to URLConnection to avoid adding a third-party dependency. I guess that depends on whether SUTAgent is just doing simple requests or uses some fancy HTTP features. From the build log it looks like SUTAgent calls Notification.setLatestEventInfo() that is deprecated now. In Fennec we switched to using NotificationCompat.Builder (Bug 1195692).
Assignee | ||
Comment 2•9 years ago
|
||
gbrown, jmaher: What do you think would be the best way to go forward here?
Flags: needinfo?(jmaher)
Flags: needinfo?(gbrown)
Comment 3•9 years ago
|
||
I suspect we might be able to get rid of the funciton GetInternetData: https://dxr.mozilla.org/mozilla-central/search?q=GetInternetData&case=true To be honest I am not sure if there is anyway to use the GetInternetData in other non obvious ways. Most likely it isn't used at all and given the fact that nothing calls it there is high confidence we don't need it anytmore. If you remove that entire function and the related imports, will the build work?
Flags: needinfo?(jmaher)
Assignee | ||
Comment 4•9 years ago
|
||
Thanks! I'm going to try that (and refactor the Notification.setLatestEventInfo usage).
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
Assignee | ||
Comment 5•9 years ago
|
||
Attachment #8668467 -
Flags: review?(jmaher)
Assignee | ||
Comment 6•9 years ago
|
||
Attachment #8668468 -
Flags: review?(michael.l.comella)
Assignee | ||
Comment 7•9 years ago
|
||
Attachment #8668469 -
Flags: review?(jmaher)
Assignee | ||
Updated•9 years ago
|
Attachment #8668467 -
Attachment description: Remove unused method GetInternetData() → SUTAgent: Remove unused method GetInternetData()
Comment 8•9 years ago
|
||
Comment on attachment 8668467 [details] [diff] [review] SUTAgent: Remove unused method GetInternetData() Review of attachment 8668467 [details] [diff] [review]: ----------------------------------------------------------------- great!
Attachment #8668467 -
Flags: review?(jmaher) → review+
Comment 9•9 years ago
|
||
Comment on attachment 8668469 [details] [diff] [review] SUTAgent: Inline isIPv4Address() Review of attachment 8668469 [details] [diff] [review]: ----------------------------------------------------------------- other than my confusion on the regex, this is good ::: build/mobile/sutagent/android/SUTAgentAndroid.java @@ +55,5 @@ > public class SUTAgentAndroid extends Activity > { > final Handler mHandler = new Handler(); > > + private static final Pattern IPV4_PATTERN = Pattern.compile("^(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}$"); I don't understand this pattern? how does loopback work or other addresses work? is there a reference for this?
Attachment #8668469 -
Flags: review?(jmaher) → review-
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Joel Maher (:jmaher) from comment #9) > Comment on attachment 8668469 [details] [diff] [review] > SUTAgent: Inline isIPv4Address() > > Review of attachment 8668469 [details] [diff] [review]: > ----------------------------------------------------------------- > > other than my confusion on the regex, this is good > > ::: build/mobile/sutagent/android/SUTAgentAndroid.java > @@ +55,5 @@ > > public class SUTAgentAndroid extends Activity > > { > > final Handler mHandler = new Handler(); > > > > + private static final Pattern IPV4_PATTERN = Pattern.compile("^(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)(\\.(25[0-5]|2[0-4]\\d|[0-1]?\\d?\\d)){3}$"); > > I don't understand this pattern? how does loopback work or other addresses > work? is there a reference for this? I basically just inlined the code from InetAddressUtils: http://androidxref.com/source/xref/external/apache-http/src/org/apache/http/conn/util/InetAddressUtils.java I just saw that the latest HttpClient release has a refactored, more readabable, version: > private static final String IPV4_BASIC_PATTERN_STRING = > "(([1-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\.){1}" + // initial first field, 1-255 > "(([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])\\.){2}" + // following 2 fields, 0-255 followed by . > "([0-9]|[1-9][0-9]|1[0-9]{2}|2[0-4][0-9]|25[0-5])"; // final field, 0-255 http://grepcode.com/file/repo1.maven.org/maven2/org.apache.httpcomponents/httpclient/4.5/org/apache/http/conn/util/InetAddressUtils.java#45
Comment 11•9 years ago
|
||
Comment on attachment 8668469 [details] [diff] [review] SUTAgent: Inline isIPv4Address() Review of attachment 8668469 [details] [diff] [review]: ----------------------------------------------------------------- given that this is the same regex as used in the code we were importing, lets r+. Please reference where you did get it from in a comment.
Attachment #8668469 -
Flags: review- → review+
Assignee | ||
Comment 12•9 years ago
|
||
Together with other API 23 build patches: https://treeherder.mozilla.org/#/jobs?repo=try&revision=93b51c8b5499
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8668468 [details] [diff] [review] SUTAgent: Replace deprecated Notification APIs with Notification.Builder Mike has so much in his review queue, maybe you have less? :)
Attachment #8668468 -
Flags: review?(mhaigh)
Assignee | ||
Comment 14•9 years ago
|
||
leave-open: I'm going to land the two reviewed patches.
Keywords: leave-open
Comment 15•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/488526a93c27 https://hg.mozilla.org/integration/fx-team/rev/43e9af141078
https://hg.mozilla.org/mozilla-central/rev/488526a93c27 https://hg.mozilla.org/mozilla-central/rev/43e9af141078
Comment on attachment 8668468 [details] [diff] [review] SUTAgent: Replace deprecated Notification APIs with Notification.Builder Review of attachment 8668468 [details] [diff] [review]: ----------------------------------------------------------------- Seems reasonable to me.
Attachment #8668468 -
Flags: review?(michael.l.comella)
Attachment #8668468 -
Flags: review?(mhaigh)
Attachment #8668468 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: leave-open
Comment 19•9 years ago
|
||
Sorry to be so late to the party! Patches look good to me.
Flags: needinfo?(gbrown)
https://hg.mozilla.org/mozilla-central/rev/df5a6ba88fed
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox44:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Updated•3 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
•