Closed Bug 1017242 Opened 5 years ago Closed 5 years ago

Eliminate Android 2.2 (API 8) code

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 33

People

(Reporter: rnewman, Assigned: rnewman)

References

Details

(Whiteboard: [file follow-up from comment 21])

Attachments

(13 files)

2.23 KB, patch
mfinkle
: review+
Details | Diff | Splinter Review
4.85 KB, patch
mcomella
: review+
Details | Diff | Splinter Review
5.06 KB, patch
kats
: review+
Details | Diff | Splinter Review
9.47 KB, patch
kats
: review+
Details | Diff | Splinter Review
3.84 KB, patch
kats
: review+
Details | Diff | Splinter Review
1.38 KB, patch
mcomella
: review+
Details | Diff | Splinter Review
3.09 KB, patch
mcomella
: review+
Details | Diff | Splinter Review
3.09 KB, patch
mfinkle
: review+
Details | Diff | Splinter Review
1.16 KB, patch
mfinkle
: review+
Details | Diff | Splinter Review
3.87 KB, patch
myk
: review+
Details | Diff | Splinter Review
1.53 KB, patch
mfinkle
: review+
Details | Diff | Splinter Review
4.68 KB, patch
mfinkle
: review+
Details | Diff | Splinter Review
4.91 KB, patch
Margaret
: review+
Details | Diff | Splinter Review
We ended 2.2 support in Firefox 31. Let's use this bug to track removal of any API 8 code from the codebase in 32 and up.
Depends on: 1017244
mobile/android/base/widget/TwoWayView.java
1061:        if (Build.VERSION.SDK_INT < 9) {
1072:        if (Build.VERSION.SDK_INT < 9) {
3145:        if (Build.VERSION.SDK_INT >= 5) {

mobile/android/base/webapp/Allocator.java
79:        if (android.os.Build.VERSION.SDK_INT > 8) {

mobile/android/base/util/GamepadUtils.java
27:        if (Build.VERSION.SDK_INT >= 9) {
50:        if (Build.VERSION.SDK_INT < 9) {

mobile/android/base/tests/testBrowserProvider.java
540:            if (Build.VERSION.SDK_INT >= 8 &&
644:            if (Build.VERSION.SDK_INT >= 8 &&

mobile/android/base/tests/testGetUserMedia.java
21:        if (Build.VERSION.SDK_INT >= 9) {

mobile/android/base/SysInfo.java.in
50:        if (android.os.Build.VERSION.SDK_INT < 9) {

mobile/android/base/mozglue/GeckoLoader.java.in
88:            if (Build.VERSION.SDK_INT >= 8) {

mobile/android/base/GeckoSharedPrefs.java
153:        if (Build.VERSION.SDK_INT < 9) {

mobile/android/base/gfx/LayerView.java
146:        if (Build.VERSION.SDK_INT >= 9) {
670:        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.GINGERBREAD) {
684:        if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.GINGERBREAD) {

mobile/android/base/health/BrowserHealthRecorder.java
342:        if (android.os.Build.VERSION.SDK_INT < android.os.Build.VERSION_CODES.GINGERBREAD) {

mobile/android/base/sync/setup/activities/LocaleAware.java
31:    if (Build.VERSION.SDK_INT < Build.VERSION_CODES.GINGERBREAD) {

mobile/android/base/GeckoAppShell.java
719:        if (Build.VERSION.SDK_INT < 9) {
732:        if (Build.VERSION.SDK_INT < 9) {
2200:        if (Build.VERSION.SDK_INT >= 9) {
2207:            if (Build.VERSION.SDK_INT >= 9)
2479:        if (Build.VERSION.SDK_INT >=Build.VERSION_CODES.FROYO)
2514:        if (Build.VERSION.SDK_INT >=Build.VERSION_CODES.FROYO)

mobile/android/base/db/BrowserDatabaseHelper.java
95:        if (Build.VERSION.SDK_INT >= 8) {
133:        if (Build.VERSION.SDK_INT >= 8) {

mobile/android/base/CrashReporter.java
247:        if (Build.VERSION.SDK_INT < Build.VERSION_CODES.GINGERBREAD) {
395:            if (Build.VERSION.SDK_INT >= 8) {

mobile/android/base/BrowserApp.java
293:        if (Build.VERSION.SDK_INT >= 9 &&

mobile/android/base/background/healthreport/HealthReportDatabaseStorage.java
237:    public static boolean CAN_USE_ABSOLUTE_DB_PATH = (Build.VERSION.SDK_INT >= Build.VERSION_CODES.FROYO);

mobile/android/base/AndroidGamepadManager.java
304:        if (Build.VERSION.SDK_INT < 9) {

plus a bunch in GeckoApp.
Blocks: 1017244
Depends on: 979921
No longer depends on: 1017244
(In reply to Richard Newman [:rnewman] from comment #2)
> mobile/android/base/widget/TwoWayView.java
> 1061:        if (Build.VERSION.SDK_INT < 9) {
> 1072:        if (Build.VERSION.SDK_INT < 9) {
> 3145:        if (Build.VERSION.SDK_INT >= 5) {

FYI: TwoWayView should be considered a third-party dependency. So, please don't touch it while fixing this bug as the upstream project still needs to support Froyo.
Assignee: nobody → rnewman
Status: NEW → ASSIGNED
Attachment #8458293 - Flags: review?(myk)
Attachment #8458296 - Flags: review?(margaret.leibovic)
Keywords: meta
Summary: [meta] Eliminate Android 2.2 (API 8) code → Eliminate Android 2.2 (API 8) code
Attachment #8458296 - Flags: review?(margaret.leibovic) → review+
Attachment #8458293 - Flags: review?(myk) → review+
Attachment #8458257 - Flags: review?(margaret.leibovic)
Attachment #8458258 - Flags: review?(margaret.leibovic)
Attachment #8458260 - Flags: review?(bugmail.mozilla)
Attachment #8458261 - Flags: review?(bugmail.mozilla)
Attachment #8458262 - Flags: review?(michael.l.comella)
Attachment #8458263 - Flags: review?(michael.l.comella)
Attachment #8458264 - Flags: review?(michael.l.comella)
Attachment #8458265 - Flags: review?(michael.l.comella)
Attachment #8458294 - Flags: review?(liuche)
Attachment #8458295 - Flags: review?(liuche)
Attachment #8458260 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8458261 [details] [diff] [review]
Part 5: remove API 8 code from gamepad code

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

::: mobile/android/base/util/GamepadUtils.java
@@ +26,3 @@
>      private static boolean isGamepadKey(KeyEvent event) {
> +        if (Build.VERSION.SDK_INT < 12) {
> +            return false;

Why return false for < 12?
That constant isn't defined until API 12. in other words: Eclipse spotted a crash bug.
Attachment #8458295 - Flags: review?(liuche) → review+
Attachment #8458265 - Flags: review?(michael.l.comella) → review+
Attachment #8458262 - Flags: review?(michael.l.comella) → review+
Attachment #8458264 - Flags: review?(michael.l.comella) → review+
Attachment #8458263 - Flags: review?(michael.l.comella) → review+
Comment on attachment 8458261 [details] [diff] [review]
Part 5: remove API 8 code from gamepad code

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

I doubt it would crash; doesn't the constant get inlined at compile time? So compiling using an SDK > v12 should work on a runtime < v12, it would just presumably always return false in that case. Nevertheless this is better.
Attachment #8458261 - Flags: review?(bugmail.mozilla) → review+
Comment on attachment 8458257 [details] [diff] [review]
Part 1: remove API 8 code from mozglue

Do we need the fallbacks? Does getExternalStoragePublicDirectory return null?
Attachment #8458257 - Flags: review?(margaret.leibovic) → review+
Attachment #8458294 - Flags: review?(liuche) → review+
(In reply to Mark Finkle (:mfinkle) from comment #21)

> Do we need the fallbacks? Does getExternalStoragePublicDirectory return null?

I can only presume that they're there for a reason. That reason *might* have been because someone was too lazy to add an `else` clause. I'll look deeper, but perhaps not in this bug.
Whiteboard: [file follow-up from comment 21]
Comment on attachment 8458258 [details] [diff] [review]
Part 2: remove API 8 code from GeckoApp and BrowserApp

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

::: mobile/android/base/GeckoApp.java
@@ +2376,5 @@
>              try {
>                  obj.put("lac", gcl.getLac());
>                  obj.put("cid", gcl.getCid());
>  
> +                int psc = gcl.getPsc();

I don't see this line but rnewman said on irc that it was gone in his latest rebase on fx-team so this should be removed from the patch.
Attachment #8458258 - Flags: review?(margaret.leibovic) → review+
Depends on: 1053016
See Also: → 1217675
You need to log in before you can comment on or make changes to this bug.