Closed Bug 1080232 Opened 10 years ago Closed 3 years ago

Support multiplexing getCount

Categories

(Firefox for Android Graveyard :: Data Providers, defect)

Product:
Firefox for Android Graveyard
Component:
Data Providers
Platform:
All
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED INCOMPLETE

People

(Reporter: rnewman, Unassigned, Mentored, NeedInfo)

Details

(Keywords: good-first-bug, perf, Whiteboard: [lang=java])

Attachments

(2 files, 1 obsolete file)

Draft patch for the fix. Untested.
10.28 KB, patch
rnewman
: feedback+
Details | Diff | Splinter Review
Bug 1080232 - Multiplexing count.(with reviews)
58 bytes, text/x-review-board-request
Grisha
: review-
Details 
Currently we do this:

final BrowserDB db = getProfile().getDB();
BrowserDB.getCount(getContentResolver(), "history"));	
final ContentResolver cr = getContentResolver();
Telemetry.HistogramAdd("PLACES_PAGES_COUNT", db.getCount(cr, "history"));
Telemetry.HistogramAdd("PLACES_BOOKMARKS_COUNT", db.getCount(cr, "bookmarks"));
Telemetry.HistogramAdd("FENNEC_FAVICONS_COUNT", db.getCount(cr, "favicons"));
Telemetry.HistogramAdd("FENNEC_THUMBNAILS_COUNT", db.getCount(cr, "thumbnails"));

which issues four separate requests.

We should be able to run a single query that returns four rows (or four columns) and only hits the DB once.

Worth evaluating to see whether that would be a decent win to avoid stalls while gathering telemetry.
Mentor: rnewman
Whiteboard: [lang=java][good first bug]
I would like to work on this as my first bug. I have a working fennec build and have gone through the above code in BrowserApp.java. I would like clarifications on the below:

1) I don't see a combined view in BrowserContract.java that includes History, Bookmarks, Favicons and Thumbnails. The existing "Combined" view seems to have only the History and Bookmarks. Should we have to create a new view? Or is there any other way of querying multiple URIs in a single query (using UNION?) with a ContentResolver? 

2) What is the flow that triggers the "Telemetry:Gather" event in BrowserApp?
You should probably take a look at Bug 970604.

We want to add an entry point to BrowserProvider's ContentProvider interface that returns a specially sculpted query response, something like:

  URI: /counts

  Query:

    SELECT
     (SELECT count(1) from bookmarks) AS bookmarks,
     (SELECT count(1) from history) AS history,
     (SELECT count(1) from favicons) AS favicons,
     (SELECT count(1) from thumbnails) AS thumbnails;

Then the telemetry gathering code would call

  db.query(COUNTS_URI);

and retrieve the four columns of the first row.
Here's a draft patch for the fix. I haven't tested this yet. Will write the Robocop test and get back with an updated patch. May I please request that this bug be assigned to me?
Assignee: nobody → srsridhar5
Status: NEW → ASSIGNED
Attachment #8503582 - Flags: feedback?(rnewman)
Comment on attachment 8503582 [details] [diff] [review]
Draft patch for the fix. Untested.

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

This is looking good!

Comments inline.

See <https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F> -- it'd help if there was a commit message in this patch, like:

  Bug 1080232 - Support multiplexing count lookup for telemetry gathering. r=rnewman

::: mobile/android/base/BrowserApp.java
@@ +1524,5 @@
>              }
>  
>          } else if ("Telemetry:Gather".equals(event)) {
> +            Cursor cur = BrowserDB.getAllCounts(getContentResolver());
> +            

Nit: please make sure your patch doesn't add trailing whitespace. (Throughout.)

@@ +1526,5 @@
>          } else if ("Telemetry:Gather".equals(event)) {
> +            Cursor cur = BrowserDB.getAllCounts(getContentResolver());
> +            
> +            try{
> +                if(cur!=null && cur.moveToFirst()){

Nit: space between "if" and "(", and between ")" and "{".

Substantive comment: `cur` should never be null. If you're going to check whether it is, do it *outside* the `try` block.

@@ +1534,5 @@
> +                            cur.getInt(cur.getColumnIndex("bookmarks")));
> +                    Telemetry.HistogramAdd("FENNEC_FAVICONS_COUNT", 
> +                            cur.getInt(cur.getColumnIndex("favicons")));
> +                    Telemetry.HistogramAdd("FENNEC_THUMBNAILS_COUNT", 
> +                            cur.getInt(cur.getColumnIndex("thumbnails")));

Put each of these on a single line.

@@ +1538,5 @@
> +                            cur.getInt(cur.getColumnIndex("thumbnails")));
> +                }
> +            }
> +            finally{
> +                if(cur!=null){

Don't check for null here.

::: mobile/android/base/db/BrowserDB.java
@@ +238,5 @@
>      }
> +    
> +    /**
> +     *  Return the count of Bookmarks, History, Thumbnails and Favicons in a single row. 
> +     *  Hits the db only once. 

Nit: "DB".

@@ +241,5 @@
> +     *  Return the count of Bookmarks, History, Thumbnails and Favicons in a single row. 
> +     *  Hits the db only once. 
> +     */
> +    public static Cursor getAllCounts(ContentResolver cr){
> +        return sDb.getAllCounts(cr);

This'll get bitrotted by Bug 1077590. Please coordinate with wesj to make sure this is handled correctly.

::: mobile/android/base/db/BrowserProvider.java
@@ +228,5 @@
>          map.put(SearchManager.SUGGEST_COLUMN_INTENT_DATA,
>                  Combined.URL + " AS " + SearchManager.SUGGEST_COLUMN_INTENT_DATA);
>          SEARCH_SUGGEST_PROJECTION_MAP = Collections.unmodifiableMap(map);
> +        
> +        //All counts

No need for this comment.

@@ +653,5 @@
>              String[] selectionArgs, String sortOrder) {
>          SQLiteDatabase db = getReadableDatabase(uri);
>          final int match = URI_MATCHER.match(uri);
> +        
> +        if(match == COUNTS){

Same comment about spacing.

@@ +656,5 @@
> +        
> +        if(match == COUNTS){
> +            // We don't need the query builder below for fetching the counts of all the tables.
> +            final String query = "SELECT"+
> +              "(SELECT count(1) from " + TABLE_BOOKMARKS + ") AS bookmarks,"+

Spaces around operators and between query clauses, and indenting, like this:

 final String query = "SELECT " +
                      "(SELECT count(1) from " + TABLE_BOOKMARKS + ") AS bookmarks, " +
Attachment #8503582 - Flags: feedback?(rnewman) → feedback+
Assignee: srsridhar5 → nobody
Status: ASSIGNED → NEW
It seems Sridhar did a good job with the patch. Is it OK if I port his code to new project structure taking into account review comments?
Yes, but please do take the time to make sure this bug is still relevant! It's 18 months old.
Looks like this is still relevant: https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/BrowserApp.java?rev=151059cf9645#1752

If the above link goes bad, look for the "Telemetry:Gather" string in BrowserApp.
(In reply to Michael Comella (:mcomella) [not actively working on fennec: expect slow responses] from comment #7)
> Looks like this is still relevant:
> https://mxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/
> mozilla/gecko/BrowserApp.java?rev=151059cf9645#1752
> 
> If the above link goes bad, look for the "Telemetry:Gather" string in
> BrowserApp.

Hi Michael,
Is it okay, if I take this forward. Thanks.
Sure Shubham, go for it.

By the way, you can use the "Need more information from" field to give a user a persistent notification so they'll be more likely to see your comments or questions.
Hi Michael,
Looks like instead of four only two queries are made to database currently:

            Telemetry.addToHistogram("PLACES_PAGES_COUNT", db.getCount(cr, "history"));
            Telemetry.addToHistogram("FENNEC_BOOKMARKS_COUNT", db.getCount(cr, "bookmarks"));

In this case, do we still need the optimisation.
Flags: needinfo?(michael.l.comella)
(In reply to Shubham from comment #10)
> Looks like instead of four only two queries are made to database currently:
> 
>             Telemetry.addToHistogram("PLACES_PAGES_COUNT", db.getCount(cr,
> "history"));
>             Telemetry.addToHistogram("FENNEC_BOOKMARKS_COUNT",
> db.getCount(cr, "bookmarks"));
> 
> In this case, do we still need the optimisation.

Good catch!

Considering hitting the disk is one of the slowest operations we can do (and could delay other concurrent disk accesses), I would say it's worthwhile.
Flags: needinfo?(michael.l.comella)
Hi Michael,
I have pushed a patch, can you please review it. Thanks.
Comment on attachment 8797340 [details]
Bug 1080232 - Multiplexing count.(with reviews)

Grisha, do you mind taking a look at this one?
Attachment #8797340 - Flags: review?(michael.l.comella) → review?(gkruglov)
Attachment #8797342 - Flags: review?(michael.l.comella) → review?(gkruglov)
My apologies for a delay in reviewing this. I'm planning to take a look at this tomorrow.
Comment on attachment 8797340 [details]
Bug 1080232 - Multiplexing count.(with reviews)

https://reviewboard.mozilla.org/r/82936/#review83858

The core of this looks good! A few more changes, and we're there :-)

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:1641
(Diff revision 1)
>      }
>  
>      @Override
>      public void handleMessage(final String event, final NativeJSObject message,
>                                final EventCallback callback) {
> +

Try not to have unnecessary whitespace changes in your patches.

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:1751
(Diff revision 1)
>              }
>  
>          } else if ("Telemetry:Gather".equals(event)) {
>              final BrowserDB db = getProfile().getDB();
>              final ContentResolver cr = getContentResolver();
> -            Telemetry.addToHistogram("PLACES_PAGES_COUNT", db.getCount(cr, "history"));
> +            Cursor cur = db.getAllCounts(cr);

Make `cur` final, we don't expect/want that object reference to change.

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:1752
(Diff revision 1)
>  
>          } else if ("Telemetry:Gather".equals(event)) {
>              final BrowserDB db = getProfile().getDB();
>              final ContentResolver cr = getContentResolver();
> -            Telemetry.addToHistogram("PLACES_PAGES_COUNT", db.getCount(cr, "history"));
> -            Telemetry.addToHistogram("FENNEC_BOOKMARKS_COUNT", db.getCount(cr, "bookmarks"));
> +            Cursor cur = db.getAllCounts(cr);
> +            try {

As rnewman pointed out in a previous review, this cursor should not be null. If we're going to check that it is, it seems cleaner to separate our concerns here by introducing a simple static method to do cursor-related work safely.

Here's a general pattern I'd follow:
```
private static void addCountsToTelemetry(final Telemetry telemetry, final BrowserDB db) {
    final Cursor cur = db.getAllCounts(cr);
    if (cur == null) {
        debug("Counts cursor is unexpectedly null");
        return;
    }
    
    try {
        if (!cur.moveToFirst()) {
            debug("Counts unavailable");
            return;
        }
        
        [...other stuff...]
    
    } finally {
        cur.close();
    }
}

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:1754
(Diff revision 1)
>              final BrowserDB db = getProfile().getDB();
>              final ContentResolver cr = getContentResolver();
> -            Telemetry.addToHistogram("PLACES_PAGES_COUNT", db.getCount(cr, "history"));
> -            Telemetry.addToHistogram("FENNEC_BOOKMARKS_COUNT", db.getCount(cr, "bookmarks"));
> +            Cursor cur = db.getAllCounts(cr);
> +            try {
> +                if (cur != null && cur.moveToFirst()) {
> +                    Telemetry.addToHistogram("PLACES_PAGES_COUNT", cur.getInt(cur.getColumnIndex

House style is to use getColumnIndexOrThrow instead to get at the index. One of the reasons is that if something's very broken with the cursor, it'll be a bit easier to trace through crash logs this way.

Another is that this way there's no chance we'll try to use the default "-1" index accidentally; it's better to fail as soon as we know something very unexpected happened.

::: mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java:1090
(Diff revision 1)
> -
>          SQLiteDatabase db = getReadableDatabase(uri);
> +        if(match == COUNTS){
> +            String history_table ;
> +            String bookmark_table;
> +            if (hasFaviconsInProjection(projection))

Always use curly braces in your if statements, even if they're simple ones such as this one.

Unfortunately, we're not writing Python.

::: mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java:1095
(Diff revision 1)
> +            if (hasFaviconsInProjection(projection))
> +                history_table = VIEW_HISTORY_WITH_FAVICONS;
> +            else
> +                history_table = TABLE_HISTORY;
> +
> +            if (hasFaviconsInProjection(projection)) {

You should avoid duplicate call to `hasFaviconsInProject`.

You can do this either by combining the two if statements, or by caching its result value into a (final) boolean, and checking against that value instead.

::: mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java:1105
(Diff revision 1)
> +            } else {
> +                bookmark_table = TABLE_BOOKMARKS;
> +            }
> +
> +            final String query = "SELECT "+
> +                    "(SELECT count(_ID) from " + history_table + " WHERE visits > 0) AS " +

nit: FROM should be capitalized.

::: mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java:1107
(Diff revision 1)
> +            }
> +
> +            final String query = "SELECT "+
> +                    "(SELECT count(_ID) from " + history_table + " WHERE visits > 0) AS " +
> +                    "history, "+
> +                    "(SELECT count(_ID) from " + bookmark_table + " WHERE type = " + Bookmarks

Don't hard-code column and table names when you can.

`_ID` is actually `_id`, and you can get that via `History._ID`

Similarly, "history.visits" is `History.VISITS`, and you can get the table name via `History.TABLE_NAME`.

Same applies for bookmarks query.

This way we can quickly find all of the column/table uses either via an IDE or in http://dxr.mozilla.org/.

::: mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java:1119
(Diff revision 1)
>          String limit = uri.getQueryParameter(BrowserContract.PARAM_LIMIT);
>          String groupBy = null;
>  
>          switch (match) {
> +
> +

Same comment about whitespace changes.

::: mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java:1269
(Diff revision 1)
>          }
>  
>          trace("Running built query.");
>          Cursor cursor = qb.query(db, projection, selection, selectionArgs, groupBy,
>                  null, sortOrder, limit);
> +

Same comment about whitespace changes.

::: mobile/android/base/java/org/mozilla/gecko/db/StubBrowserDB.java:163
(Diff revision 1)
>      private final StubURLMetadata urlMetadata = new StubURLMetadata();
>      private final StubUrlAnnotations urlAnnotations = new StubUrlAnnotations();
>      private SuggestedSites suggestedSites = null;
>  
> +
> +

Same comment about whitespace changes.
Comment on attachment 8797342 [details]
Bug 1080232 review changes -r=:Grisha Kruglov

https://reviewboard.mozilla.org/r/82942/#review83866

Even better is to roll this commit into the main commit. No need to separate the two in this case, really - it makes the review a little confusing, it'll be cleaner when someone's digging through this code via change logs or `blame`, and if something goes wrong, it's easier to back out one commit instead of two commits.

You can do this easily via `hg histedit`. It helps to have latest `hg` installed, `histedit` will be less quirky.
Attachment #8797342 - Flags: review?(gkruglov) → review+
Comment on attachment 8797340 [details]
Bug 1080232 - Multiplexing count.(with reviews)

https://reviewboard.mozilla.org/r/82936/#review83870

Re-flag me for a review (in the commit message) once you push up the changes.
Attachment #8797340 - Flags: review?(gkruglov)
(In reply to :Grisha Kruglov from comment #19)
> Comment on attachment 8797340 [details]
> Bug 1080232 - Multiplexing count.
> 
> https://reviewboard.mozilla.org/r/82936/#review83870
> 
> Re-flag me for a review (in the commit message) once you push up the changes.

Hi Grisha,
Thanks for the review. I will get back with the changes.
Hi Grisha,
Made the suggested changes. Please have a look. Thanks.
Flags: needinfo?(gkruglov)
(In reply to Shubham from comment #22)
> Hi Grisha,
> Made the suggested changes. Please have a look. Thanks.

Thanks, Shubham! Couple of things though, which will make my life as a reviewer easier:

1) combine everything you have here into one commit. Currently there are two commits, and the second one is purely review feedback. In this case reviewboard makes it a little annoying to review the work - but it works very well if you roll feedback changes into the original commit itself. You can do this using `hg histedit`, as I mentioned in Comment 18 - just mark second commit as "roll", and then push the resulting single commit for a review (ensuring commit message is what you want, etc). You should see changes on this page once you do that.

2) in reviewboard itself, you will see "open issues" - it'll be a yellow count on top of the page. You can mark issues as "fixed" once you address them (either by changing your code, or responding to feedback with a comment), helping reviewer figure out what's going on.
Flags: needinfo?(gkruglov)
Assignee: nobody → shubham2892
Status: NEW → ASSIGNED
Comment on attachment 8797342 [details]
Bug 1080232 review changes -r=:Grisha Kruglov

https://reviewboard.mozilla.org/r/82942/#review85254

Looking good, thanks! One more nit to address, then combine both commits into one and repush.

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:1637
(Diff revision 2)
>          }
>  
>          mBrowserToolbar.refresh();
>      }
>  
> +    private static void addHistoryAndBookmarkCountsToTelemetry(final BrowserDB db, ContentResolver cr) {

Mark `cr` as `final` as well.

::: mobile/android/base/java/org/mozilla/gecko/BrowserApp.java:1648
(Diff revision 2)
> +        try {
> +            if (!cur.moveToFirst()) {
> +                Log.d(LOGTAG, "Count unavailable");
> +                return;
> +            }
> +            Telemetry.addToHistogram("PLACES_PAGES_COUNT", cur.getInt(cur.getColumnIndexOrThrow

Nit: if you want to break up a long line into multiple lines to stay under the 80char/line mark, here's a better way to do that, similar to what you'll see in the codebase:

```
Telemetry.addToHistogram(
    "PLACES_PAGES_COUNT",
    cur.getInt(cur.getColumnIndexOrThrow("history"))
)
```

::: mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java:1104
(Diff revision 2)
> -
>              } else {
>                  bookmark_table = TABLE_BOOKMARKS;
>              }
> -
> -            final String query = "SELECT "+
> +            final String query = "SELECT " +
> +                    "(SELECT COUNT(" + History._ID + ") FROM " + history_table + " WHERE " + History

Nit, but important for reability. Readability matters very much because once this (or any!) change lands, it will be read many times over by many people, so making code easier to read is a huge productivity multiplier across time and space!

Break up lines around operations such as "+", not around object/class access.
So, cleaner-looking/more readable version of this query might look something like this:

```
final String query = "SELECT " +
                    "(SELECT COUNT(" + History._ID + ") FROM " + history_table +
                    " WHERE " + History.VISITS + " > 0) AS history, " +
                    "(SELECT COUNT(" + Bookmarks._ID + ") FROM " + bookmark_table +
                    " WHERE " + Bookmarks.TYPE + " = " + Bookmarks.TYPE_BOOKMARK + ") AS " + "bookmarks;";
```

::: mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java:1107
(Diff revision 2)
>              }
> -
> -            final String query = "SELECT "+
> -                    "(SELECT count(_ID) from " + history_table + " WHERE visits > 0) AS " +
> -                    "history, "+
> -                    "(SELECT count(_ID) from " + bookmark_table + " WHERE type = " + Bookmarks
> +            final String query = "SELECT " +
> +                    "(SELECT COUNT(" + History._ID + ") FROM " + history_table + " WHERE " + History
> +                    .VISITS + " > 0) AS history, " +
> +                    "(SELECT COUNT(" + Bookmarks._ID + ") FROM " + bookmark_table + " WHERE " +
> +                    Bookmarks.TYPE + " = " + Bookmarks.TYPE_BOOKMARK + ") AS " + "bookmarks;";

Combine "[...] AS " + "bookmarks;"; into "AS bookmarks;"
Attachment #8797342 - Attachment is obsolete: true
Hi Grisha,
As mentioned by you, I have combined the commits and pushed it as one commit. I have also marked all the issues on review board as resolved. Please have a look. Thanks.
Flags: needinfo?(gkruglov)
Attachment #8797340 - Flags: review?(michael.l.comella)
Comment on attachment 8797340 [details]
Bug 1080232 - Multiplexing count.(with reviews)

https://reviewboard.mozilla.org/r/82936/#review85956

Thanks, Shubham! This is looking good, except that I missed a clashing constant in a previous review. That needs to change, otherwise this code won't actually work properly.

::: mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java:101
(Diff revision 2)
>      // History matches
>      static final int HISTORY = 200;
>      static final int HISTORY_ID = 201;
>      static final int HISTORY_OLD = 202;
>  
> +    static final int COUNTS = 1000;

This is clashing with TOPSITES constant.

The latest constant I see is at 1400, which you do not have here. You probably want to pull from central and rebase your patch on top of the latest code. Ping me on IRC in #mobile if you need help with this! (irc name: grisha). See: https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java#139
Flags: needinfo?(gkruglov)
Hi Grisha,
Made the above change, and rebased the patch on top of the latest code. Please have a look. Thanks.
Flags: needinfo?(gkruglov)
Attachment #8797340 - Flags: review?(michael.l.comella) → review?(gkruglov)
Hi Grisha,
Just following up on this.
Hi Grisha,
Any feedback on this.
Comment on attachment 8797340 [details]
Bug 1080232 - Multiplexing count.(with reviews)

https://reviewboard.mozilla.org/r/82936/#review91652

I think something went wrong with your last push. You're missing your own changes to BrowserApp to actually use the new getAllCounts method, and added an unnecessary file.

::: mobile/android/base/java/org/mozilla/gecko/db/StubBrowserDB.java:30
(Diff revision 3)
> +import android.content.Context;
> +import android.database.ContentObserver;
> +import android.database.Cursor;
> +import android.graphics.drawable.BitmapDrawable;
> +
> +class StubSearches implements Searches {

Why is this being added?
Attachment #8797340 - Flags: review?(gkruglov) → review-
Flags: needinfo?(gkruglov)
Hi, I'm a student at Seneca college learning open source, and I was hoping to work on this bug for my course.
If no one else is currently working on it, I'd like to give it a try. :)
Grisha, is this bug still available?
Mentor: michael.l.comella
Flags: needinfo?(gkruglov)
Yeah - although I reckon the improvement will be rather slim. Telemetry:Gather now triggers two count calls, so the improvement here would be combining them into a single SQL query. But, probably still worth doing, primarily as a learning vehicle for the contributor.

Shouldn't be hard to piece together a patch from the two work-in-progress ones attached to this bug.

Time showed that this probably isn't a great "first" bug though - see previous two attempts. It's not difficult, but there are a few moving parts involved on top of the regular "getting used to bugzilla review flow", etc learning curve.
Flags: needinfo?(gkruglov)
Assignee: shubham2892 → nobody
Status: ASSIGNED → NEW
Hi , I have 1+year of Software Engineering experience .This is my first experience with open source community .I would like to work on this bug as my "getting used to bugzilla review flow" - mentioned by Grisha .


I don't have any source code or environment to start with for this bug .Please guide me on how should I begin with , get the code base and know the requirement for this bug  .

I have read all discussion for this thread and I got a little idea about combining SQL query for performance .
Nevin, would you be able to mentor this bug?
Mentor: cnevinchen
Flags: needinfo?(cnevinchen)
Feel free to email me if I can help!

For code and setup, see this document
https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Build_Instructions/Simple_Firefox_for_Android_build

And we use mozreview to submit / review / try patches
http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html

btw, please select "Need more information from... " when you reply on bugzilla so he/she can get notified :)
Flags: needinfo?(cnevinchen) → needinfo?(divyanshu17x)
Keywords: good-first-bug
Whiteboard: [lang=java][good first bug] → [lang=java]

Would somebody be willing to mentor me for this issue? I have a working build up and running on Linux.

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INCOMPLETE
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: