1233785 - DownloadContentService: Skip DownloadAction if no network is available

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

There's no need to do anything in DownloadAction if no network is available. Bailing out early will make implementing failure counters (bug 1215106) easier because we do not want to count network errors.

Comment 1

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: MozReview Request: Bug 1233785 - DownloadContentService: Skip DownloadAction if no network is available. r?rnewman

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

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

https://reviewboard.mozilla.org/r/28573/#review25405

::: mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java:238
(Diff revision 1)
> +    protected boolean isConnectedToNetwork(Context context) {

Should we add an exception check for Android 6?

Comment 3

[Richard Newman [:rnewman]]

Comment on attachment 8700103 [details]
MozReview Request: Bug 1233785 - DownloadContentService: Skip DownloadAction if no network is available. r?rnewman

https://reviewboard.mozilla.org/r/28573/#review25755

::: mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java:68
(Diff revision 1)
>              Log.d(LOGTAG, "Network is metered. Postponing download.");

It's worth thinking about (and putting in the bug!) what to do if the user is *always* on a metered connection -- that is, if their home network is a hotspot or they don't have home/work internet.

It's also worth thinking about whether there's a size limit before we care. We're a web browser; loading The Verge is going to hammer their data cap anyway, so perhaps if the amount to download is under, say, 100KB we should go ahead and do it?

::: mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java:242
(Diff revision 1)
> +        return networkInfo != null && networkInfo.isConnectedOrConnecting();

Why `isConnectedOrConnecting` instead of `isConnected`?

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to Richard Newman [:rnewman] from comment #3)
> It's worth thinking about (and putting in the bug!) what to do if the user
> is *always* on a metered connection -- that is, if their home network is a
> hotspot or they don't have home/work internet.

I'll create a follow-up bug for this: We should have a solution for this but it might be tricky to do it right.

> It's also worth thinking about whether there's a size limit before we care.
> We're a web browser; loading The Verge is going to hammer their data cap
> anyway, so perhaps if the amount to download is under, say, 100KB we should
> go ahead and do it?

This decision is easy to make if we are just looking at one item. But the catalog contains multiple items: How many < 100 KB items do we want to download until the sum is too much? Do we need to monitor the (local) traffic we generate with DLC? Let's answer that in the follow-up bug.

> ::: mobile/android/base/java/org/mozilla/gecko/dlc/DownloadAction.java:242
> (Diff revision 1)
> > +        return networkInfo != null && networkInfo.isConnectedOrConnecting();
> 
> Why `isConnectedOrConnecting` instead of `isConnected`?

Being overly optimistic: If we are currently establishing a connection then we should be able to download things soonish. But looking a the documentation isConnected() seems to be the better choice here:

> isConnectedOrConnecting ()
> This is good for applications that need to do anything related to the network other
> than read or write data. For the latter, call isConnected() instead, which guarantees
> that the network is fully usable.

http://developer.android.com/reference/android/net/NetworkInfo.html#isConnectedOrConnecting%28%29

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

https://hg.mozilla.org/integration/fx-team/rev/2c9aa0cc94b640b0bbd344f13c25714ed48288b3
Bug 1233785 - DownloadContentService: Skip DownloadAction if no network is available. r=rnewman

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2c9aa0cc94b6