1245044 - Move Restrictions class to restrictions package

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Family Friendly Browsing, defect)

Description

[Sebastian Kaspari (:sebastian; :pocmo)]

The Restrictions is our entry point to check whether the normal/guest/restricted profile is allowed to use certain features:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/Restrictions.java

We want to move this class from the root to the "restrictions" package where the other code is located:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/restrictions

Note that this class is an entry point for JNI calls (Methods annotated @WrapForJNI). After moving the class mach will update the GeneratedJNIWrappers.cpp:
https://dxr.mozilla.org/mozilla-central/source/widget/android/GeneratedJNIWrappers.cpp
This may or may not require you to update some c++ code using those methods.

For testing, this class lists features that can be restricted:
https://dxr.mozilla.org/mozilla-central/source/toolkit/components/parentalcontrols/nsIParentalControlsService.idl

Comment 1

[Tushar Saini (:shatur)]

Hi Sebastian.
   I would like to work on this bug. I will read through and upload the patch. Please let me know if I can work on this bug.

Thanks

Comment 2

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to shatur from comment #1)
> Hi Sebastian.
>    I would like to work on this bug. I will read through and upload the
> patch. Please let me know if I can work on this bug.

Yeah, you can work on this bug! Just attach a patch and flag me for review as soon as it's ready. :)

Comment 3

[Vishnu (:malayaleecoder)]

Attachment: Bug1245044_v1.diff

Hello Sebastian!

I have moved the Restriction.java to restriction folder and have attached an initial patch for this bug. On running "./mach build" after committing the above changes, I did not find GeneratedJNIWrappers.cpp to be changed. Do tell me if I made any mistake and please guide me for further improvements.

Cheers!

Comment 4

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec8ef87d63db

Comment 5

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to malayaleecoder from comment #3)
> I have moved the Restriction.java to restriction folder and have attached an
> initial patch for this bug. On running "./mach build" after committing the
> above changes, I did not find GeneratedJNIWrappers.cpp to be changed. Do
> tell me if I made any mistake and please guide me for further improvements.

This is interesting. I pushed the patch to try to see if the tests still pass:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec8ef87d63db

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

The builds are failing:
> gmake[5]: *** No rule to make target `java/org/mozilla/gecko/Restrictions.java', needed by `gecko-browser.jar'. Stop.

You'll need to update this moz.build file:
https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/moz.build#502

Comment 7

[Vishnu (:malayaleecoder)]

Hello Sebastian,

I did change moz.build and the build ran successfully. But still did not see any changes in GeneratedJNIWrappers.cpp. Should I attach the patch?

Cheers!

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Yeah, go ahead, I'll push it to try again and then let's see what the tests say. :)

Comment 9

[Vishnu (:malayaleecoder)]

Attachment: Bug1245044_v2.diff

Please have a look.

Thanks!

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Are you running |mach build| locally to confirm that the build is working?

Because even with the new patch I see errors when building locally.

Comment 11

[Vishnu (:malayaleecoder)]

Yes, I am. Is that the wrong way?
I am having a hard time pushing to the try server from my system.
Please guide me on what I have to do subsequently.
Thanks.

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to malayaleecoder from comment #11)
> Yes, I am. Is that the wrong way?

No, that's correct. However with the current patch applied this should not build. Are you sure you have those changes applied when building?

Comment 13

[Vishnu (:malayaleecoder)]

@Sebastian : I am very sure that the changes have been applied. After reading your comment, I ran ./mach build and it built the entire code successfully. What to do?

Comment 14

[Sebastian Kaspari (:sebastian; :pocmo)]

Hm,

* Can you share your mozconfig file? https://pastebin.mozilla.org/

* Does |mach package| and |mach install| create and install an application on your phone?

Comment 15

[Vishnu (:malayaleecoder)]

Sebastian, please give me some more time, I think I have got a few missing files and configurations in by directory. Will fix it and ping you.

Thank You.

Comment 16

[Sebastian Kaspari (:sebastian; :pocmo)]

Sure, let me know if you need any further help or just ask the friendly folks in #mobile (https://wiki.mozilla.org/IRC)

Comment 17

[Sebastian Kaspari (:sebastian; :pocmo)]

Did you have any success?

Comment 18

[Vishnu (:malayaleecoder)]

Hello Sebastian,

I had messaged you in IRC, think you did not see that, I am currently having my examinations going on for two more days. Will fix it for sure after that. Hope it is not a problem.

Cheers.

Comment 19

[Sebastian Kaspari (:sebastian; :pocmo)]

Sure, no problem.

Comment 20

[Vishnu (:malayaleecoder)]

Attachment: Bug1245044_v3.diff

Push to try please :)

Comment 21

[Sebastian Kaspari (:sebastian; :pocmo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f73250335efc

Comment 22

[Vishnu (:malayaleecoder)]

Sebastian, Hope it's fine.
Adding checkin-needed :)

Comment 23

[Sebastian Kaspari (:sebastian; :pocmo)]

(In reply to malayaleecoder from comment #22)
> Sebastian, Hope it's fine.
> Adding checkin-needed :)

Yep, thanks! :)

Comment 24

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/779bebe283f8

Comment 25

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/779bebe283f8