Closed Bug 1351585 Opened 7 years ago Closed 7 years ago

Code review of open-source Leanplum SDK

Categories

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

Product:
Firefox for Android Graveyard
Component:
General
Platform:
All
Android
Type:
enhancement

Tracking

(firefox55 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 55
Tracking Flags:
Tracking Status
firefox55 --- fixed

People

(Reporter: sebastian, Assigned: cnevinchen)

References

Details

(Whiteboard: [LP_M1])

Attachments

(9 files, 3 obsolete files)

Bug 1351585 - Part 1. Add Leanplum SDK source code to thirdparty module
59 bytes, text/x-review-board-request
maliu
: review+
Details
Bug 1351585 - Part 2. Add build flag and change our existing build system to inject leanplum implementation
59 bytes, text/x-review-board-request
nalexander
: review+
Details
Bug 1351585 - Part 3. Add proguard for leanplum.
59 bytes, text/x-review-board-request
maliu
: review+
Details
Bug 1351585 - Part 1. Add Leanplum SDK source code to thirdparty module
59 bytes, text/x-review-board-request
Details
Bug 1351585 - Part 2. Change our existing build system to reference leanplum
59 bytes, text/x-review-board-request
Details
Bug 1351585 - Part 3. Add proguard for leanplum.
59 bytes, text/x-review-board-request
Details
Bug 1351585 - Part 4. Use Switchboard to limit Leanplum startup/init
59 bytes, text/x-review-board-request
Details
fix ddce5b17 o draft Bug 1351585 - Part 1. Add Leanplum SDK source code to thirdparty module
59 bytes, text/x-review-board-request
Details
fix 1b3f2cf0 o draft Bug 1351585 - Part 2. Change our existing build system to reference leanplum
59 bytes, text/x-review-board-request
Details 
As soon as we land the SDK and use it in the app it's part of the code we maintain. We should do a review of this code (quality and privacy).
Comment on attachment 8867424 [details]
Bug 1351585 - Part 1. Add Leanplum SDK source code to thirdparty module


Due to the tight schedule, I want to modify Leanplum's SDK code base to reduce the effort of adding dependencies to our code base. Also make less impact on our APK size/method count.
Besides, we’ll shift to Gradle in near future, I don’t want to spend too many effort on Mozbuild,

They are couple for dependencies/code I've stripped from their code base here: 

1. Legacy http client: they have server-side compatibility issue so they can't change it. I'm going to use our ch.boye.httpclientandroidlib version to replace that ( change import).

2. BuildConfig: Their code use two build config in their code, one accept a system variable and use it as their user-agent name. The other just printing debug info. I've remove those code and it works fine.

3. Google Play Location services (Geofence): Since we don't need this feature now, I don't want to add it. 

My idea is to make their SDK easy for us to adpot via moz-build. Once we shift to Gradle build, I'll use the approach in Bug 1351573. 

Since this bug is for Leanplum code review, The patch I provided is the proposed modification of their source code and mozbuild config to put it into our repository.
Hi Nick
According to https://bugzilla.mozilla.org/show_bug.cgi?id=1143888
I still need the puppet code and buildbot-configs code to work.
Can you please teach me how to add leanplum app_id and access_key to build?
Thank you!
Flags: needinfo?(nalexander)
I also found I don't know how to set the flag to make MOZ_INSTALL_MMA to true...
The reason why I use MMA instead of Leanplum is because we might change the implementation someday,
Flags: needinfo?(max)
(In reply to Nevin Chen [:nechen] from comment #4)
> 1. Legacy http client: they have server-side compatibility issue so they
> can't change it. I'm going to use our ch.boye.httpclientandroidlib version
> to replace that ( change import).

Every now and then we circle around to the idea of removing ch.boye.httpclientandroidlib from our codebase entirely. So far that work hasn't been taken up in earnest, but let's not make it more difficult by creating new dependencies here.

I haven't looked at their code base, but any chance we could get away with just using HttpURLConnection and friends?
Flags: needinfo?(cnevinchen)
Comment on attachment 8867535 [details]
Bug 1351585 - Part 5. Add build flag to inject mma implementation.

https://reviewboard.mozilla.org/r/139088/#review142718

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:888
(Diff revision 1)
>              protected Void doInBackground(Void... params) {
>                  super.doInBackground(params);
>                  if (SwitchBoard.isInExperiment(context, Experiments.LEANPLUM) &&
>                          GeckoPreferences.getBooleanPref(context, GeckoPreferences.PREFS_HEALTHREPORT_UPLOAD_ENABLED, true)) {
>                      // Do LeanPlum start/init here
> +                    MmaDelegate.init(context.getApplicationContext());

Why are we not put this into org.mozilla.gecko.switchboard.AsyncConfigLoader#doInBackground ?

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:870
(Diff revision 2)
>  
>      /**
>       * Initializes the default Switchboard URLs the first time.
>       * @param intent
>       */
> -    private static void initSwitchboard(final Context context, final SafeIntent intent, final boolean isInAutomation) {
> +    private void initSwitchboard(final Context context, final SafeIntent intent, final boolean isInAutomation) {

Do we still need to remove static if we move MmaDelegate.init into AsyncConfigLoader?

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:888
(Diff revision 2)
>              protected Void doInBackground(Void... params) {
>                  super.doInBackground(params);
>                  if (SwitchBoard.isInExperiment(context, Experiments.LEANPLUM) &&
>                          GeckoPreferences.getBooleanPref(context, GeckoPreferences.PREFS_HEALTHREPORT_UPLOAD_ENABLED, true)) {
>                      // Do LeanPlum start/init here
> +                    MmaDelegate.init(BrowserApp.this.getApplication());

Why are we not put the init into AsyncConfigLoader#doInBackground after SwitchBoard.loadConfig?
(In reply to :Grisha Kruglov from comment #16)
> Every now and then we circle around to the idea of removing
> ch.boye.httpclientandroidlib from our codebase entirely. So far that work
> hasn't been taken up in earnest, but let's not make it more difficult by
> creating new dependencies here.
Totally agree with you

> 
> I haven't looked at their code base, but any chance we could get away with
> just using HttpURLConnection and friends?

They use socket as keep alive. And there is some helper method can let them get Http Header and Status code to do some checking from that socket.
HttpURLConnection didn't provide that feature. And doing it manually means we gonna have our own logic to do that ( InputStream -> String -> parsing...).

The short term solution is to use our ch.boye library. longer term is to work with them to change their implementation.
Flags: needinfo?(cnevinchen)
Comment on attachment 8867532 [details]
Bug 1351585 - Part 2. Add build flag and change our existing build system to inject leanplum implementation

https://reviewboard.mozilla.org/r/139082/#review142814

::: mobile/android/base/AndroidManifest.xml.in:485
(Diff revision 2)
> +                <action android:name="com.leanplum.LeanplumPushListenerService" />
> +            </intent-filter>
> +        </receiver>
> +
> +        <!-- Leanplum Local Push Notification Service-->
> +        <service android:name="com.leanplum.LeanplumLocalPushListenerService" />

Should we explicitly declare this component exported and enabled?

::: mobile/android/base/AndroidManifest.xml.in:504
(Diff revision 2)
> +                <action android:name="com.google.android.gms.iid.InstanceID" />
> +            </intent-filter>
> +        </service>
> +
> +        <!-- Leanplum GCM/FCM Registration Service -->
> +        <service android:name="com.leanplum.LeanplumPushRegistrationService" />

Should we explicitly declare this component exported and enabled?

::: mobile/android/base/AndroidManifest.xml.in:508
(Diff revision 2)
> +        <!-- Leanplum GCM/FCM Registration Service -->
> +        <service android:name="com.leanplum.LeanplumPushRegistrationService" />
> +
> +
> +        <!-- Geofencing Service -->
> +        <service android:name="com.leanplum.ReceiveTransitionsIntentService" />

Should we explicitly declare this component exported and enabled?

::: mobile/android/base/Makefile.in:143
(Diff revision 2)
>  
>  ifdef MOZ_INSTALL_TRACKING
>  GECKOVIEW_JARS += gecko-thirdparty-adjust_sdk.jar
>  endif
>  
> +

nit: not necessary white space

::: mobile/android/base/Makefile.in:164
(Diff revision 2)
>  
>  ifdef MOZ_ANDROID_MLS_STUMBLER
>  FENNEC_JARS += ../stumbler/stumbler.jar
>  endif
>  
> +

nit: not necessary white space

::: mobile/android/base/moz.build:182
(Diff revision 2)
>  else:
>      constants_jar.sources += ['java/org/mozilla/gecko/' + x for x in [
>          'adjust/StubAdjustHelper.java',
>      ]]
>  
> +

nit: not necessary white space
Comment on attachment 8867533 [details]
Bug 1351585 - Part 3. Add proguard for leanplum.

https://reviewboard.mozilla.org/r/139084/#review142818
Attachment #8867533 - Flags: review?(max) → review+
Comment on attachment 8867534 [details]
Bug 1351585 - Part 4. Use Switchboard to limit Leanplum startup/init

https://reviewboard.mozilla.org/r/139086/#review142820

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:884
(Diff revision 2)
>          final String serverUrl = TextUtils.isEmpty(serverExtra) ? SWITCHBOARD_SERVER : serverExtra;
> -        new AsyncConfigLoader(context, serverUrl).execute();
> +        new AsyncConfigLoader(context, serverUrl) {
> +            @Override
> +            protected Void doInBackground(Void... params) {
> +                super.doInBackground(params);
> +                if (SwitchBoard.isInExperiment(context, Experiments.LEANPLUM) &&

Why are we not put the init into AsyncConfigLoader#doInBackground after SwitchBoard.loadConfig?
Comment on attachment 8867535 [details]
Bug 1351585 - Part 5. Add build flag to inject mma implementation.

https://reviewboard.mozilla.org/r/139088/#review142826

::: mobile/android/base/java/org/mozilla/gecko/mma/MmaLeanplumImp.java:14
(Diff revision 2)
> +import android.content.Context;
> +
> +
> +public class MmaLeanplumImp implements MmaInterface {
> +    @Override public void init(Application application) {
> +

Is this the actual implementation other than the Stub?
The idea of this bug was to just do a review of the existing leanplum code to make sure that it doesn't contain anything that would block us from releasing it. But I guess we can do the integration here too. :)
Comment on attachment 8867424 [details]
Bug 1351585 - Part 1. Add Leanplum SDK source code to thirdparty module

https://reviewboard.mozilla.org/r/138964/#review142888

::: mobile/android/thirdparty/build.gradle:8
(Diff revision 4)
>  apply plugin: 'com.android.library'
>  
>  android {
>      compileSdkVersion 23
>      buildToolsVersion mozconfig.substs.ANDROID_BUILD_TOOLS_VERSION
> -
> +    useLibrary 'org.apache.http.legacy'

I wonder how we get this to compile in non-gradle builds (as far as I know this line doesn't actually add the library but just makes it known at compile time) - nalexander might know.

::: mobile/android/thirdparty/build.gradle:43
(Diff revision 4)
> +    compile "com.android.support:appcompat-v7:${mozconfig.substs.ANDROID_SUPPORT_LIBRARY_VERSION}"
> +    compile "com.google.android.gms:play-services-gcm:${mozconfig.substs.ANDROID_GOOGLE_PLAY_SERVICES_VERSION}"
> +    compile "com.google.android.gms:play-services-location:${mozconfig.substs.ANDROID_GOOGLE_PLAY_SERVICES_VERSION}"
> +    compile "com.google.android.gms:play-services-ads:${mozconfig.substs.ANDROID_GOOGLE_PLAY_SERVICES_VERSION}"
> +    compile "com.android.support:support-annotations:${mozconfig.substs.ANDROID_GOOGLE_PLAY_SERVICES_VERSION}"

This should be behind build flags to be only included if really needed.
Comment on attachment 8867532 [details]
Bug 1351585 - Part 2. Add build flag and change our existing build system to inject leanplum implementation

https://reviewboard.mozilla.org/r/139082/#review142884

::: mobile/android/base/AndroidManifest.xml.in:488
(Diff revision 2)
> +        <service android:name="com.leanplum.LeanplumPushListenerService" android:exported="false"
> +            android:enabled="false">
> +            <intent-filter>
> +                <action android:name="com.google.android.c2dm.intent.RECEIVE" />
> +            </intent-filter>
> +        </service>

Are you sure this is correct? We already define a receiver for this if MOZ_GCM is enabled. I had the impression that we would need a custom receiver that will route the received messages either to leanplum or our web push implementation. However I can't find this in the leanplum docs.
Comment on attachment 8867535 [details]
Bug 1351585 - Part 5. Add build flag to inject mma implementation.

https://reviewboard.mozilla.org/r/139088/#review142892

::: mobile/android/base/moz.build:1344
(Diff revision 2)
>  <span class="indent">>>>></span>    CONFIG['ANDROID_PLAY_SERVICES_GCM_AAR_LIB'],
>  <span class="indent">>>>></span>    CONFIG['ANDROID_SUPPORT_ANNOTATIONS_JAR_LIB'],
>  <span class="indent">>>>></span>    CONFIG['ANDROID_SUPPORT_V4_AAR_LIB'],
>  <span class="indent">>>>></span>    CONFIG['ANDROID_SUPPORT_V4_AAR_INTERNAL_LIB'],
>  <span class="indent">>>>></span>    CONFIG['ANDROID_APPCOMPAT_V7_AAR_LIB'],
>  <span class="indent">>>>></span>    CONFIG['ANDROID_PLAY_SERVICES_ADS_AAR_LIB'],

this one doesn't seem to use at least play-services-location like the gradle build?
nechen: I've pushed back a version of your sequence with your pieces folded and re-ordered, and some changes on top.  Both moz.build and Gradle build for me, but I haven't really thought through everything, and I haven't tried to launch it or anything.  It should unblock you for the rest of the day, though.  Please make your changes and trigger try builds (android-api-15 and android-api-15-gradle, please).

(There's a commit for Bug 1365089 hidden underneath that you'll need to pull in to make this actually work.)
Flags: needinfo?(nalexander)
Attachment #8867424 - Flags: review?(nalexander)
Attachment #8867532 - Flags: review?(nalexander)
Attachment #8867533 - Flags: review?(nalexander)
Attachment #8867535 - Flags: review?(nalexander)
Attachment #8867995 - Flags: review?(nalexander)
Attachment #8867996 - Flags: review?(nalexander)
Attachment #8867997 - Flags: review?(nalexander)
Attachment #8867999 - Flags: review?(nalexander)
Attachment #8868000 - Flags: review?(nalexander)
Comment on attachment 8868000 [details]
fix 1b3f2cf0 o draft Bug 1351585 - Part 2. Change our existing build system to reference leanplum

https://reviewboard.mozilla.org/r/139554/#review143114

::: mobile/android/base/moz.build:133
(Diff revision 1)
>  x.script = 'generate_build_config.py:generate_java'
>  x.inputs += ['AdjustConstants.java.in']
>  y = GENERATED_FILES['generated/preprocessed/org/mozilla/gecko/AppConstants.java']
>  y.script = 'generate_build_config.py:generate_java'
>  y.inputs += ['AppConstants.java.in']
> -
> +y = GENERATED_FILES['generated/preprocessed/org/mozilla/gecko/MmaConstants.java']

y object is replaced here. How do the w,x,y,z things work? Is it doesn't matter whether w,x,y,z are been replaced? If not, how about replace them all with a same var to avoid misunderstanding?
Comment on attachment 8868000 [details]
fix 1b3f2cf0 o draft Bug 1351585 - Part 2. Change our existing build system to reference leanplum

https://reviewboard.mozilla.org/r/139554/#review143114

> y object is replaced here. How do the w,x,y,z things work? Is it doesn't matter whether w,x,y,z are been replaced? If not, how about replace them all with a same var to avoid misunderstanding?

Correct: `y` is just a reference to the `GeneratedFile` instance in that collection.  I thought about calling them `x1, ..., x5`, or making them all `x`.  If you care, we can do that in this patch.  Numbering or labeling them isn't great for alphabetical ordering; having them all the same looks a little odd.
Attachment #8867534 - Attachment is obsolete: true
Attachment #8867534 - Flags: review?(s.kaspari)
Attachment #8867534 - Flags: review?(max)
Attachment #8867535 - Attachment is obsolete: true
Attachment #8867535 - Flags: review?(s.kaspari)
Attachment #8867535 - Flags: review?(max)
Comment on attachment 8867532 [details]
Bug 1351585 - Part 2. Add build flag and change our existing build system to inject leanplum implementation

https://reviewboard.mozilla.org/r/139082/#review143654

A couple of addition comments, but otherwise this looks good to me.  Please do any additional Leanplum integration work in a separate patch or a separate ticket.

::: mobile/android/base/MmaConstants.java.in:26
(Diff revision 3)
> +    null;
> +//#endif
> +
> +public static final String MOZ_LEANPLUM_APP_ID =
> +//#ifdef MOZ_LEANPLUM_APP_ID
> +    "@MOZ_LEANPLUM_APP_ID@";

This will be `MOZ_LEANPLUM_SDK_CLIENTID` after the patches in https://reviewboard.mozilla.org/r/139456/diff/2#index_header land, so can you rebase onto that?

::: mobile/android/base/generate_build_config.py:48
(Diff revision 3)
>                  'MOZ_ANDROID_DOWNLOADS_INTEGRATION',
>                  'MOZ_ANDROID_DOWNLOAD_CONTENT_SERVICE',
>                  'MOZ_ANDROID_EXCLUDE_FONTS',
>                  'MOZ_ANDROID_GCM',
>                  'MOZ_ANDROID_MLS_STUMBLER',
> +                'MOZ_ANDROID_MMA',

You'll need to add the `MOZ_LEANPLUM_*` stuff here too.
Attachment #8867532 - Flags: review?(nalexander) → review+
Comment on attachment 8867532 [details]
Bug 1351585 - Part 2. Add build flag and change our existing build system to inject leanplum implementation

https://reviewboard.mozilla.org/r/139082/#review143660

::: mobile/android/base/java/org/mozilla/gecko/mma/MmaLeanplumImp.java:26
(Diff revision 3)
> +    public void init(Application application) {
> +        Leanplum.setApplicationContext(application);
> +        Parser.parseVariables(application);
> +        LeanplumActivityHelper.enableLifecycleCallbacks(application);
> +
> +        Leanplum.setAppIdForDevelopmentMode(MmaConstants.MOZ_LEANPLUM_APP_ID, MmaConstants.MOZ_LEANPLUM_SDK_KEY);

This should be guarded by `AppConstants.MOZILLA_OFFICIAL`.  You want to set production for official builds.

See https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/AppConstants.java.in?q=path%3AAppConstants%2Ajava%2A&redirect_type=single#263 and usages in the tree.
Attachment #8868000 - Attachment is obsolete: true
Attachment #8868000 - Flags: review?(s.kaspari)
Attachment #8868000 - Flags: review?(max)
Attachment #8867999 - Attachment is obsolete: true
Attachment #8867999 - Flags: review?(s.kaspari)
Attachment #8867999 - Flags: review?(max)
Attachment #8867998 - Attachment is obsolete: true
Attachment #8867998 - Flags: review?(s.kaspari)
Attachment #8867998 - Flags: review?(max)
Attachment #8867997 - Attachment is obsolete: true
Attachment #8867997 - Flags: review?(s.kaspari)
Attachment #8867997 - Flags: review?(max)
Attachment #8867996 - Attachment is obsolete: true
Attachment #8867996 - Flags: review?(s.kaspari)
Attachment #8867996 - Flags: review?(max)
Attachment #8867995 - Attachment is obsolete: true
Attachment #8867995 - Flags: review?(s.kaspari)
Attachment #8867995 - Flags: review?(max)
Comment on attachment 8868979 [details]
Bug 1351585 - Part 4. Add pref to turn on/off leanplum.

https://reviewboard.mozilla.org/r/140656/#review144248

::: mobile/android/app/mobile.js:422
(Diff revision 1)
> +#if MOZ_UPDATE_CHANNEL == nightly
> +pref("mma.enabled", true);
> +#elif MOZ_UPDATE_CHANNEL == beta
> +pref("mma.enabled", true);
> +#else
> +pref("mma.enabled", true);

All true in these scenario?
Comment on attachment 8867424 [details]
Bug 1351585 - Part 1. Add Leanplum SDK source code to thirdparty module

https://reviewboard.mozilla.org/r/138964/#review144250
Attachment #8867424 - Flags: review?(max) → review+
Comment on attachment 8868979 [details]
Bug 1351585 - Part 4. Add pref to turn on/off leanplum.

https://reviewboard.mozilla.org/r/140656/#review144270

I don't think using a Gecko pref for this makes sense.  First, can you _disable_ Leanplum while Gecko is running?  What does that mean?  Does the browser stop receiving events?  Will we turn off any options that we have had Leanplum toggle remotely for us?

As written, the Prefs stuff is invoked from http://searchfox.org/mozilla-central/rev/24c443a440104dabd9447608bd41b8766e8fc2f5/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java#637.  That's well before Gecko is running.  Are you confident your prefs listener makes sense at that point?  I'm not!

You should have Jim Chen (:darchons) review this patch.
Attachment #8868979 - Flags: review-
Attachment #8868979 - Attachment is obsolete: true
Attachment #8868979 - Flags: review?(s.kaspari)
Attachment #8868979 - Flags: review?(max)
Assignee: nobody → cnevinchen
Pushed by nechen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/01942414ab89
Part 1. Add Leanplum SDK source code to thirdparty module r=maliu
https://hg.mozilla.org/integration/autoland/rev/a1e493554517
Part 2. Add build flag and change our existing build system to inject leanplum implementation r=nalexander
https://hg.mozilla.org/integration/autoland/rev/2f4c52ef4a4b
Part 3. Add proguard for leanplum. r=maliu
Keywords: checkin-needed
This was not ready to land: it depends on Bug 1365089, which needs additional work.

nechen: the only reason this doesn't break the build is that everything is opt-in, and without Bug 1365089, nothing is setting any of the build flags.  So I guess this can stay in and wait until we enable everything, but that's not what I expected.  Is that what you were trying to achieve?
Flags: needinfo?(cnevinchen)
Yes! Exactly! Thanks for your full support on this. Deeply appreciated!
Flags: needinfo?(cnevinchen)
Comment on attachment 8867424 [details]
Bug 1351585 - Part 1. Add Leanplum SDK source code to thirdparty module

Confused. This already landed but there are still review flags? Clearing..
Attachment #8867424 - Flags: review?(s.kaspari)
Attachment #8867532 - Flags: review?(s.kaspari)
Attachment #8867533 - Flags: review?(s.kaspari)
Priority: -- → P1
Attachment #8867532 - Flags: review?(max)
Comment on attachment 8867533 [details]
Bug 1351585 - Part 3. Add proguard for leanplum.

This has landed.
Attachment #8867533 - Flags: review?(nalexander)
Comment on attachment 8867424 [details]
Bug 1351585 - Part 1. Add Leanplum SDK source code to thirdparty module

This has landed.
Attachment #8867424 - Flags: review?(nalexander)
Attachment #8867995 - Attachment is obsolete: false
Attachment #8867996 - Attachment is obsolete: false
Attachment #8867997 - Attachment is obsolete: false
Attachment #8867998 - Attachment is obsolete: false
Attachment #8867999 - Attachment is obsolete: false
Attachment #8868000 - Attachment is obsolete: false
Attachment #8867995 - Flags: review?(s.kaspari)
Attachment #8867995 - Flags: review?(nalexander)
Attachment #8867996 - Flags: review?(s.kaspari)
Attachment #8867996 - Flags: review?(nalexander)
Attachment #8867997 - Flags: review?(s.kaspari)
Attachment #8867997 - Flags: review?(nalexander)
Attachment #8867998 - Flags: review?(s.kaspari)
Attachment #8867999 - Flags: review?(s.kaspari)
Attachment #8867999 - Flags: review?(nalexander)
Attachment #8868000 - Flags: review?(s.kaspari)
Attachment #8868000 - Flags: review?(nalexander)
maliu: you flagged a bunch of patches for re-review.  Accident?
Attachment #8867995 - Flags: review?(s.kaspari)
Attachment #8867995 - Flags: review?(nalexander)
Attachment #8867996 - Flags: review?(s.kaspari)
Attachment #8867996 - Flags: review?(nalexander)
Attachment #8867997 - Flags: review?(s.kaspari)
Attachment #8867997 - Flags: review?(nalexander)
Attachment #8867998 - Flags: review?(s.kaspari)
Attachment #8867999 - Flags: review?(s.kaspari)
Attachment #8867999 - Flags: review?(nalexander)
Attachment #8868000 - Flags: review?(s.kaspari)
Attachment #8868000 - Flags: review?(nalexander)
Whiteboard: [LP_M2]
Whiteboard: [LP_M2] → [LP_M1]
Flags: needinfo?(max)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: