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

Blocks all application's AsyncTask executions on Android 4.x #4

Open
gpiancastelli opened this issue Sep 6, 2013 · 6 comments
Open

Comments

@gpiancastelli
Copy link

When TwitterOAuthView has loaded Twitter's application authorization form, and the user navigates away from the fragment or activity containing the view, the underlying TwitterOAuthTask (which extends AsyncTask) remains blocked, as a result of the call to Object.wait() in its waitForAuthorization() method. However, this means that, on Android 4.x where only one AsyncTask can be executed at a time, any other AsyncTask belonging to the application will never start until TwitterOAuthTask receives a proper notification via Object.notify(), which will never be called.

As a side effect, on Android 4.x it's also impossible to load TwitterOAuthView two times in a row if the first time the user has navigated away from the fragment or activity containing the view, because the AsyncTask that loads Twitter's authorization page cannot be executed until the previous blocked AsyncTask receive an Object.notify() call.

@mglagola
Copy link

mglagola commented Sep 6, 2013

A quick fix seems to override onBackPressed() in your activity and send a message to the view's async task notifyAuthorization() method.

For example:
In your Activity w/ the twitter view

    @Override
    public void onBackPressed () {
        twitterView.notifyBackButtonPress();
        super.onBackPressed();
    }

In the TwitterOAuthView

    //add this line below private static final boolean DEBUG = false
    private TwitterOAuthTask asyncTask;

    //Change the start(..) method to this:
    public void start(String consumerKey, String consumerSecret, String callbackUrl, boolean dummyCallbackUrl,
            Listener listener)
    {
        if (consumerKey == null || consumerSecret == null || callbackUrl == null || listener == null)
        {
            throw new IllegalArgumentException();
        }

        Boolean dummy = Boolean.valueOf(dummyCallbackUrl);

        asyncTask = new TwitterOAuthTask();
        asyncTask.execute(consumerKey, consumerSecret, callbackUrl, dummy, listener);
    }


    //add these methods somewhere in TwitterOAuthView class
    public TwitterOAuthTask getAsyncTask() {
        return asyncTask;
    }

    public void notifyBackButtonPress() {
        getAsyncTask().notifyAuthorization();
    }

This is working for me on 4.X

@gpiancastelli
Copy link
Author

Thanks for the tip.

However, from a quick glance at the code, it seems to me that, as the result of the AsyncTask's notification, the listener's onFailure method will be called, suggesting a failure to obtain Twitter's authorization, while indeed there's not even been an attempt at doing so.

Furthermore, this issue is the cause of #3 and the overriding of onBackPressed would not work in that case.

@TakahikoKawasaki
Copy link
Owner

It sounds that there should be a mechanism to cancel the AsyncTask from outside.

@TakahikoKawasaki
Copy link
Owner

I added cancel() method which enables to cancel a running AsyncTask. In addition, TwitterOAuthView now overrides onDetachedFromWindow() and the implementation calls cancel() method from within it. This means that cancel() is automatically executed. You can enable/disable this automatic call by setCancelOnDetachedFromWindow(boolean).

I hope these changes solve the issues your are facing.

@gpiancastelli
Copy link
Author

I did solve the issue myself (but did not find the time to contribute back some code) by breaking your suspended AsyncTask into two tasks, the first retrieving Twitter's request token with the authorization URL, the second getting Twitter's access token after the user has authorized the application from the displayed web page. That seemed to give me some greater degree of peace of mind than fiddling with wait() and notify() while failing to see the need to. I would probably spend some more time making my code more reliable now, and proceed from there for any future application. However, regarding your solution, I still do believe that a BACK or a cancellation should not result in an onFailure() call, because you'd miss a way to distinguish a real failure (which the user should be notified about by some dialog or message) from a user's change of mind (which needs no on-screen notification at all).

@Mgamerz
Copy link

Mgamerz commented May 22, 2014

Is there any confirmation this has been fixed?

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

4 participants