1390203 - Only grab highlights images on wifi

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, enhancement, P1)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

We originally wanted to get these images from Gecko if we had them so we don't download multiple large images (bug 1322501). However, we weren't sure if it was really important so we filed a bug to identify how large these images were (bug 1388415).

However, we don't want to invest time in that stream of bugs so we're going to take a shortcut and only grab these images on wifi.

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

These bugs add the code to download the image.

Comment 2

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Bug 1390203: Only get highlight images on wifi.

Review commit: https://reviewboard.mozilla.org/r/171264/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171264/

Comment 3

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8899928 [details]
Bug 1390203: Only get highlight images on wifi.

https://reviewboard.mozilla.org/r/171264/#review176410

LGTM.

::: mobile/android/base/java/org/mozilla/gecko/activitystream/homepanel/stream/StreamOverridablePageIconLayout.java:65
(Diff revision 1)
> +        final NetworkUtils.ConnectionType connectionType = NetworkUtils.getConnectionType(
> +                (ConnectivityManager) getContext().getSystemService(Context.CONNECTIVITY_SERVICE));
> +        if (connectionType == NetworkUtils.ConnectionType.WIFI &&
> +                !TextUtils.isEmpty(overrideImageURL)) {

nit: Is it time for a NetworkUtils.isWiFi method? :)

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

> nit: Is it time for a NetworkUtils.isWiFi method? :)

lol, yeah, maybe.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Bug 1390203: Add NetworkUtils.isWifi.

It doesn't appear we simply check for a wifi connection type anywhere so the
existing code shouldn't need to be updated.

Review commit: https://reviewboard.mozilla.org/r/171392/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/171392/

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8899928 [details]
Bug 1390203: Only get highlight images on wifi.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/171264/diff/1-2/

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8900052 [details]
Bug 1390203: Add NetworkUtils.isWifi.

https://reviewboard.mozilla.org/r/171392/#review176856

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8899928 [details]
Bug 1390203: Only get highlight images on wifi.

https://reviewboard.mozilla.org/r/171264/#review176858

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/772283005b0d
Add NetworkUtils.isWifi. r=sebastian
https://hg.mozilla.org/integration/autoland/rev/b1779a4ff84a
Only get highlight images on wifi. r=sebastian

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I had to back out bug 1390356, and these patches made changes the same files so had to be backed out in https://hg.mozilla.org/integration/autoland/rev/54d1572562d1

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c3726830fb8d
Add NetworkUtils.isWifi. r=sebastian
https://hg.mozilla.org/integration/autoland/rev/30602859cba7
Only get highlight images on wifi. r=sebastian

Comment 12

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/c3726830fb8d
https://hg.mozilla.org/mozilla-central/rev/30602859cba7