Closed Bug 936727 Opened 6 years ago Closed 6 years ago

UpgradeReceiver logs account name

Categories

(Firefox for Android :: Android Sync, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 28

People

(Reporter: rnewman, Assigned: swaroop.rao)

Details

(Whiteboard: [mentor=rnewman][lang=java][good first bug])

Attachments

(1 file)

11-08 16:52:08.599 I/GeckoLogger(32719): fennec_rnewman :: UpgradeReceiver :: Migrating preferences on upgrade for Android account named foo@bar.com.
Whiteboard: [mentor=rnewman][lang=java][good first bug]
I'm a complete newbie to the Mozilla project, but I am very comfortable programming in Java. Can I take up this defect to fix?
(In reply to swaroop.rao from comment #1)
> I'm a complete newbie to the Mozilla project, but I am very comfortable
> programming in Java. Can I take up this defect to fix?

Absolutely!  This is a great first bug to learn how to set up a Fennec build environment.  Have you got Fennec building yet?  If not, start at https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec.  You can join #mobile on irc.mozilla.org for support.
Assignee: nobody → swaroop.rao
Status: NEW → ASSIGNED
Cool! I've started following the instructions to set up the dev env on my Macbook. I started following instructions at https://developer.mozilla.org/en-US/docs/Developer_Guide/Build_Instructions/Mac_OS_X_Prerequisites and then moved on to the link you mentioned. I see that the instruction to clone the source code repository using hg is mentioned in both pages. Should I really do it twice? I already cloned the repository as part of the steps in the first page, so I'm going to skip the step in https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec.

I will check back a bit later after I've set up my code completely.


(In reply to Nick Alexander :nalexander from comment #2)
> (In reply to swaroop.rao from comment #1)
> > I'm a complete newbie to the Mozilla project, but I am very comfortable
> > programming in Java. Can I take up this defect to fix?
> 
> Absolutely!  This is a great first bug to learn how to set up a Fennec build
> environment.  Have you got Fennec building yet?  If not, start at
> https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec.  You can
> join #mobile on irc.mozilla.org for support.
OK, I'm done with the entire build. It completed successfully. I pushed the APK file to my Nexus 4 (v4.3 Jelly Bean, unrooted) and tried it out. It works. So, what next?
(In reply to swaroop.rao from comment #4)
> OK, I'm done with the entire build. It completed successfully. I pushed the
> APK file to my Nexus 4 (v4.3 Jelly Bean, unrooted) and tried it out. It
> works. So, what next?

The next step is to reproduce the bug, so that when you fix it you can verify that it's fixed.

First, set up Sync on your desktop, and pair it with your Nexus 4 build.

Then install a new version of the APK, while watching logs:

  adb logcat -v time | fgrep Gecko

you should see a log item just like in Comment 0.


Then you'll want to make a change to the class. You're touching code that primarily lives in Git:

  https://github.com/mozilla-services/android-sync/blob/develop/src/main/java/org/mozilla/gecko/sync/receivers/UpgradeReceiver.java#L56

but you should be able to make the same change in your full Mozilla source checkout (Mercurial) to make a new build and verify the change.

You can either send a pull request with your change to the GitHub repository, or attach to this bug a Mercurial patch for the change you make to the Mercurial repository.


You'll probably want to use Utils.obfuscateEmail to replace the account name.


Let us know if you have questions!
OK, I'm not quite sure how to "set up Sync on your desktop, and pair it with your Nexus 4 build." I don't use a desktop. I use a Macbook for my development activities. All I have installed on it is an application called Android File Transfer that automatically opens up when I connect my Nexus 4 via the USB cable.
(In reply to swaroop.rao from comment #6)
> OK, I'm not quite sure how to "set up Sync on your desktop, and pair it with
> your Nexus 4 build." I don't use a desktop. I use a Macbook for my
> development activities. All I have installed on it is an application called
> Android File Transfer that automatically opens up when I connect my Nexus 4
> via the USB cable.

Hi Swaroop, great to see you making progress on this ticket.  rnewman means that you need to create a Sync account to see the behaviour we're talking about.  "Desktop" here means non-Android phone, since you can't create Sync accounts on your phone.  So fire up Firefox on your Macbook, create a Sync account, and pair your Android phone with that account.  You can start with the instructions at https://support.mozilla.org/en-US/kb/how-do-i-set-up-firefox-sync
OK, done. I made the change in UpgradeReceiver.java and tested it before and after, following rnewman's instructions. After the change, the log message printed looks as follows:

11-12 23:12:57.852 I/GeckoLogger(22897): fennec_swarooprao :: UpgradeReceiver :: Migrating preferences on upgrade for Android account named XXXXXXX.XXX@XXXXX.XXX.

So, what's the next step?
Forgot to add another line. Running 'hg status' shows the following:

BigMac:mozilla-central swarooprao$ hg status
M mobile/android/base/sync/receivers/UpgradeReceiver.java
Please review and provide feedback. I have followed the steps to generate the patch file but the generated patch file contains more lines than the ones I touched, so hopefully everything's OK.
Attachment #831225 - Flags: review?(rnewman)
Comment on attachment 831225 [details] [diff] [review]
Bug 936727 - Garble account name that is logged by UpgradeReceiver

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

Looks great. Just needs r=rnewman in the patch subject line, and this is good to check in (and uplift).

Congratulations on your first Firefox for Android patch!
Attachment #831225 - Flags: review?(rnewman) → review+
Keywords: checkin-needed
Whiteboard: [mentor=rnewman][lang=java][good first bug] → [mentor=rnewman][lang=java][good first bug][android-sync uplift needed]
(In reply to Richard Newman [:rnewman] from comment #12)
> Comment on attachment 831225 [details] [diff] [review]
> Bug 936727 - Garble account name that is logged by UpgradeReceiver
> 
> Review of attachment 831225 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks great. Just needs r=rnewman in the patch subject line, and this is
> good to check in (and uplift).
> 
> Congratulations on your first Firefox for Android patch!

Thanks, Richard! I'm excited about this and hope to contribute a lot more in the future.

Couple more questions, if you don't mind.
1. What do you mean by "Just needs r=rnewman in the patch subject line". Should I create an updated patch file?
2. What is 'uplift'?
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #13)
> https://hg.mozilla.org/integration/b2g-inbound/rev/ee57a684f9f2

Ryan, what does this mean? I noticed that the 'checkin needed' keyword was in strikethrough font. Does that mean that the patch has already been checked in and I don't need to do anything more?
(In reply to swaroop.rao from comment #15)

> Ryan, what does this mean? I noticed that the 'checkin needed' keyword was
> in strikethrough font. Does that mean that the patch has already been
> checked in and I don't need to do anything more?

checkin-needed means "hey, someone like RyanVM, could you land this for me?". Ryan's comment was him doing so.

As a new contributor, you don't have the right permissions to push patches, and it wasn't a good time for me to do so; this is the mechanism to make sure it gets checked in sometime, by someone.


(In reply to swaroop.rao from comment #14)

> 1. What do you mean by "Just needs r=rnewman in the patch subject line".

The patch subject -- "Bug xxxxxx - Fix blah blah" -- should end ". r=rnewman". You can see this in the commit that Ryan landed: he added the reviewer (r=) annotation when he prepared the patch for landing.

Doing it in the patch you upload for review saves a little time.

> Should I create an updated patch file?

No need; Ryan took care of it.

> 2. What is 'uplift'?

As I mentioned in Comment 5, the code you touched is developed in Git; the Mercurial section inside the Firefox codebase is imported from that upstream repository. We need to make sure this same work is applied there, so that it doesn't get erased next time we land a change from the Sync codebase into Firefox.
Great! So, when will this bug be marked as 'RESOLVED'?
(In reply to swaroop.rao from comment #17)
> Great! So, when will this bug be marked as 'RESOLVED'?

When the landed commit makes its way to mozilla-central.  There's a process by which the tree you landed on (b2g-inbound) gets merged into mainline (mozilla-central); when that happens, the sheriff who managed the merge will automatically update this ticket.  You'll see it in the next few days.  (Usually, this happens multiple times a day, but just right now the trees are closed due to testing failures.  You can see for yourself at https://treestatus.mozilla.org/)
https://hg.mozilla.org/mozilla-central/rev/ee57a684f9f2
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 28
https://github.com/mozilla-services/android-sync/commit/5130dca33ec7638f3afb5f0b163d4af288348b56
Whiteboard: [mentor=rnewman][lang=java][good first bug][android-sync uplift needed] → [mentor=rnewman][lang=java][good first bug]
Relanded after this disappeared.
Product: Android Background Services → Firefox for Android
You need to log in before you can comment on or make changes to this bug.