Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overhead due to disconnected Service #3

Open
CollegeDev opened this issue Jan 23, 2013 · 3 comments
Open

Overhead due to disconnected Service #3

CollegeDev opened this issue Jan 23, 2013 · 3 comments

Comments

@CollegeDev
Copy link

Hey,
good to know that you recognized it that the service sometimes is disconnecting itself. First, the pSetMan won't be "null" at any time, so you don't have to check if it is available or not (i did it in the past, just to be sure :-D). Second, you've added so much code in ALL privacy related classes for checking these two things. Why you don't implemented an central "reconnector" which will solve this directly inside the privacySettingsManager? I've done it in this way and you need only 2 methods.
Tested with hammering and flooding and it works like a charm. In my opion, it is a better way because you now you get a stable connection whenever you want to interact with the service.
What do you think about? Less work and more central implementation or do you prefer your solution?

@wsot
Copy link

wsot commented Jan 24, 2013

I totally agree that the handling of pSetMan/checking the service/responding to problem needs to be improved. For this first pass, I was more concerned with adding deny-on-fail functionality, knowing that I'd need to clean it up.

The basic structure I was thinking is this:
byte privacyOutcome;
PrivacySettings pSet;
if (pSetMan == null) {
//create new pSetMan
}
try {
//get the privacy settings, etc
//inside the functions checking the service connection etc, add re-connecting to the service
//get the appropriate privacy settings
} catch (PrivacyServiceException e) {
//deny access
privacyOutcome = PrivacySettings.ERROR;
}

switch (privacyOutcome) {
case REAL:
//do the real thing
break;
case ERROR:
//do the error thing;
break;
...
}

The reason the 'pSetMan == null' check is necessary is because reflection can be used to destroy the pSetMan object.
Also, we still need the PrivacyServiceException to handle if something goes bad actually getting the data from the service (vs returning null in error conditions).
The reason for using a switch is because that way multiple error conditions can be handled together if necessary. If there are not multiple error conditions then the privacyOutcome thing isn't necessary, and the producing of results can be mixed into the try-catch code above.

Is that pretty similar to what you were thinking?

@CollegeDev
Copy link
Author

Just have a look at my new patches. You can find my solution in the class: PrivacySettingsManager.java

@wsot
Copy link

wsot commented Feb 19, 2013

Sorry, I'm not sure where your new patches are. I don't see them in Github, and the first post of the XDA thread doesn't seem to have a modified version.
Sorry it has taken me ages to get back to you - I've only just had my internet reconnected (I moved house).

wsot pushed a commit that referenced this issue Feb 21, 2013
patch #2 - fixed typos
patch #3 - some fixes and translated unintentionally untranslated
  strings

Change-Id: I288bf43a027286c4c08f2a7761fc2bc19b77f04c
wsot pushed a commit that referenced this issue Feb 21, 2013
Fix long press home button for recent apps
mateor pushed a commit that referenced this issue Jul 30, 2013
Also:
 * add a second history section that logs
 * log mesg as text instead of number
 * dump Sync Status as a table

 Sample log:
Recent Sync History
  #1  : 2012-10-11 15:06:11     USER    0.4s            aagmtest1@gmail.com/com.google u0  com.android.calendar                                  com.google.android.calendar
  #2  : 2012-10-11 15:06:11     USER    0.1s            aagmtest1@gmail.com/com.google u0  subscribedfeeds                                       android.uid.system:1000
    mesg=parse-error
  #3  : 2012-10-11 15:06:11     USER    0.0s            aagmtest1@gmail.com/com.google u0  com.google.android.apps.uploader.PicasaUploadProvider android.uid.system:1000
  #4  : 2012-10-11 15:06:10     USER    0.1s            aagmtest1@gmail.com/com.google u0  com.google.android.gms.plus.action                    android.uid.system:1000

Recent Sync History Extras
  #1  : 2012-10-11 15:06:11     USER   aagmtest1@gmail.com/com.google u0  com.android.calendar                                  Bundle[{feed=aagmtest1@gmail.com, force=true, ignore_settings=true, ignore_backoff=true}]
  #2  : 2012-10-11 15:06:11     USER   aagmtest1@gmail.com/com.google u0  subscribedfeeds                                       Bundle[{ignore_backoff=true, force=true, ignore_settings=true}]
  #3  : 2012-10-11 15:06:11     USER   aagmtest1@gmail.com/com.google u0  com.google.android.apps.uploader.PicasaUploadProvider Bundle[{ignore_backoff=true, force=true, ignore_settings=true}]
  #4  : 2012-10-11 15:06:10     USER   aagmtest1@gmail.com/com.google u0  com.google.android.gms.plus.action                    Bundle[{ignore_backoff=true, force=true, ignore_settings=true}]

Sync Status
Account aagmtest1@gmail.com u0 com.google
=======================================================================
Authority                                           Syncable  Enabled  Delay  Loc  Poll  Per  Serv  User  Tot  Time  Last Sync            Periodic
-------------------------------------------------------------------------------------------------------------------------------------------------------------
com.google.android.apps.currents                    1         true            0    2     1    2     1     6    0:35  PERIODIC SUCCESS     86400
                                                                                                                     2012-10-12 14:59:40  2012-10-13 14:58:13
com.google.android.music.MusicContent               1         true            0    0     1    2     1     4    0:09  PERIODIC SUCCESS     86400
                                                                                                                     2012-10-12 14:59:18  2012-10-13 14:58:13
com.google.android.gms.plus.action                  1         true            0    0     1    1     1     3    0:00  PERIODIC SUCCESS     86400
                                                                                                                     2012-10-12 14:59:15  2012-10-13 14:58:13
com.google.android.apps.magazines                   1         true            0    1     1    2     1     5    0:14  PERIODIC SUCCESS     86400
                                                                                                                     2012-10-12 14:59:00  2012-10-13 14:58:13

Change-Id: Iffeb825e4b4f6217940a39b0dd71e06856f08f3f
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants