931424 - Fix AutoMounter to use AutoRestore instead of AutoBool

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Dave Hylands [:dhylands]]

When I coded up bug 928558, I wasn't aware of mozilla::AutoRestore.

We should remove the AutoBool class that was introduced and replace it with AutoRestore.

Comment 1

[Elodimuor]

Hi, i'm new to contributing to the mozilla code base, I know C++ and i've built firefox on my machine. I'd like to work on this bug, can you please point me in the right direction?

(In reply to Dave Hylands [:dhylands] from comment #0)
> When I coded up bug 928558, I wasn't aware of mozilla::AutoRestore.
> 
> We should remove the AutoBool class that was introduced and replace it with
> AutoRestore.

Comment 2

[Dave Hylands [:dhylands]]

So the AutoMounter is only built for FirefoxOS on device, so you'd need to have a FirefoxOS phone and be able to build FxOS for it.

This is the page for setting up your build environment for FxOS:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox_OS/Firefox_OS_build_prerequisites

If you have a supported phone, let me know, and I can help you get a build going for it.

Comment 3

[Dave Hylands [:dhylands]]

The other option, is that you could test the functionality on something that you can build, then code the change, and I can build it and verfify that its working properly in the AutoMounter.

Comment 4

[Clayton Hughes]

Apologies for stepping on Elodimuor's toes, but I thought I could help out with this bug (I'm new to Mozilla/B2G, but I do have a phone to test on).

How do I go about testing the change? (I thought "make check" should've done the trick, but it doesn't. I'm getting the impression that a lot of the Mozilla docs don't necessarily apply to B2G. For instance, there seems to be a general preferance for hg, but all the B2G docs I've seen show examples with git...)

Comment 5

[Clayton Hughes]

Attachment: Remove AutoBool, Use AutoRestore instead

Comment 6

[Dave Hylands [:dhylands]]

The patch looks good.

I'll apply it and test it out on Monday.

For this particular change, testing it involves adding some printf_stderr's to verify when UpdateState is called and when it tries to recurse.

IIRC, triggering the recursion happens by playing some Music with with Music app, enabled USB Mass Storage, and stopping the Music.

Comment 7

[Dave Hylands [:dhylands]]

Comment on attachment 826219 [details] [diff] [review]
Remove AutoBool, Use AutoRestore instead

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

Tested on my unagi, and it seems to behaving the same as before the patch - Thanks

Comment 9

[Daniel Holbert [:dholbert]]

dhylands, can this patch be checked in, given comment 7?

(and if so, could you check it in, or flag this as checkin-needed?)

Comment 10

[Dave Hylands [:dhylands]]

Hmm. This slipped through the cracks - thanks for noticing. I'll just go ahead and land it.

Comment 11

[Dave Hylands [:dhylands]]

https://hg.mozilla.org/integration/b2g-inbound/rev/020937bcd6a0

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/020937bcd6a0