IMPORTAND - Further development

[CollegeDev]

Hey guys (@All OPD/PD2.0 DEVS),

I'm currently trying to learn smali and how to handle with it. I do that because I will port PD2.0/OPD to Stock ROMs, and I think it is possible.

The last few days I spent a lot of time to understand how to include the PD framework to the original stock framework and yeah, I believe I found a good solution for that.

But to make it easier and possible for me to port PDroid to Stock ROMs it is neccessary trying to follow following "rules":

• If you want to insert code in the framework, put as much code as you can into your OWN methods
• Try to use not so much parameters for one method (good parameter count should be smaller or equal 3)
• Don't blow up the framework methods with your own stuff
• hold the lines at a minimum level if you insert code in the framework which is out of your own methods
• try to use not so much variables

Example (an old piece of code, please ignore style or whatever, just try to understand what I mean):

Bad way:

public void startActivity (Context who, IBinder contextThread, IBinder token, Activity target, Intent intent, int requestCode, Bundle options) {
        // BEGIN PRIVACY
        boolean isAllowed = true;
        try{
            Log.i("PrivacyContext","now we are in execStartActivity() from package: " + who.getPackageName());
            if(intent.getAction().equals(Intent.ACTION_CALL) || intent.getAction().equals(Intent.ACTION_DIAL)){
                Log.i("PrivacyContext","package: " + who.getPackageName() + " tries to take a phone call");
                if(pSetMan == null) pSetMan = new PrivacySettingsManager(who, IPrivacySettingsManager.Stub.asInterface(ServiceManager.getService("privacy")));
                PrivacySettings settings = pSetMan.getSettings(who.getPackageName(), -1);
                if(pSetMan != null && settings != null && settings.getPhoneCallSetting() != PrivacySettings.REAL){ //is not allowed
                    //test if broadcasting works!
                    final Context tmp = who;
                    isAllowed = false;
                    new Thread(new Runnable() {
                        public void run() {
                            try{
                                Thread.sleep(1000); //wait 1 Second
                            }
                            catch(Exception e){
                                //nothing here
                            }  
                            Intent privacy = new Intent("android.privacy.BLOCKED_PHONE_CALL");
                            Bundle extras = new Bundle();
                            extras.putString("packageName", tmp.getPackageName());
                            extras.putInt("phoneState", TelephonyManager.CALL_STATE_IDLE);
                            privacy.putExtras(extras);
                            tmp.sendBroadcast(privacy);
                            Log.i("PrivacyContext","sent privacy intent");
                        }
                    }).start();
                    pSetMan.notification(who.getPackageName(), 0, PrivacySettings.EMPTY, PrivacySettings.DATA_PHONE_CALL, null, settings);
                }
                else{ //is allowed
                    isAllowed = true;
                    pSetMan.notification(who.getPackageName(), 0, PrivacySettings.REAL, PrivacySettings.DATA_PHONE_CALL, null, settings);
                }

            }
        }
        catch(Exception e){
            e.printStackTrace();
             if(who != null)
                 Log.i("PrivacyContext","got exception while trying to resolve intents for package: " + who.getPackageName());
             else
                 Log.i("PrivacyContext","got exception while trying to resolve intents for unknown package");

        }
        // END PRIVACY
        ........
}

Better solution:

public void startActivity (Context who, IBinder contextThread, IBinder token, Activity target, Intent intent, int requestCode, Bundle options) {
        // BEGIN PRIVACY
        boolean isAllowed = enforcePrivacyPermissions(params..);
        ........
        // END PRIVACY
}

private boolean enforePrivacyPermissions(params...) {
         boolean isAllowed = true;
         try{
            Log.i("PrivacyContext","now we are in execStartActivity() from package: " + who.getPackageName());
            if(intent.getAction().equals(Intent.ACTION_CALL) || intent.getAction().equals(Intent.ACTION_DIAL)){
                Log.i("PrivacyContext","package: " + who.getPackageName() + " tries to take a phone call");
                if(pSetMan == null) pSetMan = new PrivacySettingsManager(who, IPrivacySettingsManager.Stub.asInterface(ServiceManager.getService("privacy")));
                PrivacySettings settings = pSetMan.getSettings(who.getPackageName(), -1);
                if(pSetMan != null && settings != null && settings.getPhoneCallSetting() != PrivacySettings.REAL){ //is not allowed
                    //test if broadcasting works!
                    final Context tmp = who;
                    isAllowed = false;
                    new Thread(new Runnable() {
                        public void run() {
                            try{
                                Thread.sleep(1000); //wait 1 Second
                            }
                            catch(Exception e){
                                //nothing here
                            }  
                            Intent privacy = new Intent("android.privacy.BLOCKED_PHONE_CALL");
                            Bundle extras = new Bundle();
                            extras.putString("packageName", tmp.getPackageName());
                            extras.putInt("phoneState", TelephonyManager.CALL_STATE_IDLE);
                            privacy.putExtras(extras);
                            tmp.sendBroadcast(privacy);
                            Log.i("PrivacyContext","sent privacy intent");
                        }
                    }).start();
                    pSetMan.notification(who.getPackageName(), 0, PrivacySettings.EMPTY, PrivacySettings.DATA_PHONE_CALL, null, settings);
                }
                else{ //is allowed
                    isAllowed = true;
                    pSetMan.notification(who.getPackageName(), 0, PrivacySettings.REAL, PrivacySettings.DATA_PHONE_CALL, null, settings);
                }

            }
        }
        catch(Exception e){
            e.printStackTrace();
             if(who != null)
                 Log.i("PrivacyContext","got exception while trying to resolve intents for package: " + who.getPackageName());
             else
                 Log.i("PrivacyContext","got exception while trying to resolve intents for unknown package");

        }
        return isAllowed;
}

[mateor]

Let me just second this and for the same reason. Even better is to isolate to separate classes, when possible.

[Lekensteyn]

*important

Sounds reasonable for such large code blocks, but does it also apply to smaller changes? E.g.:

@@ -471,7 +503,11 @@ class ContextImpl extends Context {

         registerService(TELEPHONY_SERVICE, new ServiceFetcher() {
                 public Object createService(ContextImpl ctx) {
-                    return new TelephonyManager(ctx.getOuterContext());
+                    // BEGIN privacy-modified
+                    //return new TelephonyManager(ctx.getOuterContext());
+                    Log.d(TAG, "PDroid:ContextImpl: returning PrivacyTelephonyManager");
+                    return new PrivacyTelephonyManager(ctx.getOuterContext());
+                    // END privacy-modified
                 }});

         registerService(THROTTLE_SERVICE, new StaticServiceFetcher() {

I don't know how much OPD/PD2 have diverged, but it shouldn't affect debugging too much to move the Log calls to the constructor right (in this case, PrivacyTelephonyManager)?

[wsot]

Yeah, porting with smali can be a big pain. I had to patch parts of it into the ROM for my Motorola Defy. Things like logging calls (as long as they don't include variables) are not really a problem because they are static calls and don't add to the registers. Adding variables to existing functions is a bit more of a pain.

Two things you may want to check out are: XAppDbg: http://forum.xda-developers.com/showthread.php?t=2123510 and This book: http://www.amazon.com/Decompiling-Android-Godfrey-Nolan/dp/1430242485 It isn't an amazing book, but it is ok.

An interesting thing in what you've put there, Lekensteyn, is that it hits on a separate issue about the 'static' service and the way the static context is used. However, I'll leave that for another discussion entirely.

[azuwis]

Come across this review http://review.cyanogenmod.org/31620 , sounds interesting.

Modification of framework is minimal, great for autopatcher/smali. Privacy implementation is in other apk, should ease development.

[yoshimo]

@CollegeDev "I'm currently trying to learn smali and how to handle with it. I do that because I will port PD2.0/OPD to Stock ROMs, and I think it is possible. " is there any progress on patching stock roms yet?

[mateor]

On 07/14/2013 10:08 AM, yoshimo wrote:

@CollegeDev https://github.com/CollegeDev "I'm currently trying to learn smali and how to handle with it. I do that because I will port PD2.0/OPD to Stock ROMs, and I think it is possible. " is there any progress on patching stock roms yet?

— Reply to this email directly or view it on GitHub https://github.com/OpenPDroid/OpenPDroidPatches/issues/13#issuecomment-20937641.

I think I know how to patch all roms stock or otherwise...but I am currently working on the OpenPdroid/Pdroid2.0 merge instead. When I get that done, moving on to implementing patching for all roms is next. We need more contributors.