Don't ship bagheeraclient, tokenserverclient, etc. unless shipping appropriate desktop features

RESOLVED FIXED in Firefox 40

Status

RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: rnewman, Assigned: rnewman)

Tracking

(Blocks: 1 bug)

unspecified
mozilla40
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

4 years ago
… and when FHR goes away, bagheeraclient can go away entirely.

The JS client isn't used on Android at all.
(Assignee)

Updated

4 years ago
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
(Assignee)

Updated

4 years ago
Summary: Don't ship bagheeraclient unless shipping the desktop FHR implementation → Don't ship bagheeraclient, tokenserverclient, etc. unless shipping appropriate desktop features
(Assignee)

Comment 2

4 years ago
Created attachment 8585243 [details] [diff] [review]
Don't ship bagheeraclient unless shipping the desktop FHR implementation. v1

Pushing to Try now. Hard to test locally -- yup, it excludes the files from my Android build!
Attachment #8585243 - Flags: feedback?(gps)
(Assignee)

Comment 3

4 years ago
Comment on attachment 8585243 [details] [diff] [review]
Don't ship bagheeraclient unless shipping the desktop FHR implementation. v1

Review of attachment 8585243 [details] [diff] [review]:
-----------------------------------------------------------------

::: services/common/tests/unit/test_load_modules.js
@@ +45,5 @@
> +      throw "Importing " + m + " should have failed!";
> +    }
> +  }
> +}
> +  

Sorry, will fix ws.

@@ +55,5 @@
> +    expectImportsToSucceed(non_android_modules);
> +    expectImportsToSucceed(non_android_test_modules, base=TEST_BASE);
> +  } else {
> +    expectImportsToFail(non_android_modules);
> +    expectTestImportsToFail(non_android_test_modules, base=TEST_BASE);

This is fixed locally.
(Assignee)

Comment 6

4 years ago
Created attachment 8585506 [details] [diff] [review]
Don't ship bagheeraclient.js or tokenserverclient.js on Android. v2

Neither of these clients are used on Android, and the Bagheera client isn't
used anywhere unless Firefox Health Report is enabled.

This patch conditionalizes their inclusion in services-common, makes the tests
aware, and also tests that they are _not_ present on Android.

Note that some unused test helpers are also omitted.
Attachment #8585506 - Flags: review?(gps)
(Assignee)

Updated

4 years ago
Attachment #8585243 - Attachment is obsolete: true
Attachment #8585243 - Flags: feedback?(gps)

Updated

4 years ago
Attachment #8585506 - Flags: review?(gps) → review+
(Assignee)

Comment 10

4 years ago
Relanded with an 'Android' check replaced with a MOZ_WIDGET check. *shakes fist at b2g*
Flags: needinfo?(rnewman)
https://hg.mozilla.org/mozilla-central/rev/042755670a6d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.