Bugs on sample (and maybe library too)

[AndroidDeveloperLB]

Steps:

1. choose "activity"
2. change orientation
3. Choose "ok result"

Then it crashes.
Tested on Nexus 5x with Android O, OPR4.170623.009

I wanted to put logs, but I don't see anything related to the library or Android OS, that is related to this issue.

When using Fragment, I don't see a crash, but it's still an issue because as I see it, it's supposed to show a snackbar, yet it doesn't do anything.

Also, even without changing orientation, I see something else: if I don't choose "OK RESULT", I always see a snackbar saying there is an error. It doesn't matter what I do, I get this error:
"Activity with request code ... fails expected result code check."

[AndroidDeveloperLB]

The issue seems to occur even on other projects using this library.
Please fix this.
If you wish, maybe this similar library could help:
https://github.com/tbruyelle/RxPermissions

It's about permissions, but the idea is the same: to get the result from another Activity.

[hanggrian]

Hi there, version 0.9 is out to address this issue. The error you have mentioned was indeed my fault, at least when starting from Activity. Starting it from fragment, however, requires a bit more work since orientation change recreates that Fragment, see here or example module for more information. Would you kindly try it in your case and let me know if the issue has been resolved?

Thanks for the detailed information and certainly for that RxPermissions link! I should've googled it before creating my own rx permissions library.

[AndroidDeveloperLB]

Is the fragment issue also fixed?
I gave the RxPermissions sample because I'm very new to Rx (and to Kotlin, but more to Rx), so I find it very hard to help you on this, other than just tell you there is a bug.

[AndroidDeveloperLB]

I tested the sample with 0.9.
The "fragment" button doesn't do anything.
The "activity" button causes a crash. Here's the log:

10-23 10:11:18.400 3745-3745/com.example.rxactivity E/AndroidRuntime: FATAL EXCEPTION: main
Process: com.example.rxactivity, PID: 3745
java.lang.IllegalStateException: Could not execute method for android:onClick
at android.view.View$DeclaredOnClickListener.onClick(View.java:5336)
at android.view.View.performClick(View.java:6256)
at android.view.View$PerformClick.run(View.java:24697)
at android.os.Handler.handleCallback(Handler.java:789)
at android.os.Handler.dispatchMessage(Handler.java:98)
at android.os.Looper.loop(Looper.java:164)
at android.app.ActivityThread.main(ActivityThread.java:6541)
at java.lang.reflect.Method.invoke(Native Method)
at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:767)
Caused by: java.lang.reflect.InvocationTargetException
at java.lang.reflect.Method.invoke(Native Method)
at android.view.View$DeclaredOnClickListener.onClick(View.java:5331)
at android.view.View.performClick(View.java:6256) 
at android.view.View$PerformClick.run(View.java:24697) 
at android.os.Handler.handleCallback(Handler.java:789) 
at android.os.Handler.dispatchMessage(Handler.java:98) 
at android.os.Looper.loop(Looper.java:164) 
at android.app.ActivityThread.main(ActivityThread.java:6541) 
at java.lang.reflect.Method.invoke(Native Method) 
at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240) 
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:767) 
Caused by: java.lang.IllegalArgumentException: Failed requirement.
at com.example.rxactivity.MainActivity.onClick(Unknown Source:26)
at java.lang.reflect.Method.invoke(Native Method) 
at android.view.View$DeclaredOnClickListener.onClick(View.java:5331) 
at android.view.View.performClick(View.java:6256) 
at android.view.View$PerformClick.run(View.java:24697) 
at android.os.Handler.handleCallback(Handler.java:789) 
at android.os.Handler.dispatchMessage(Handler.java:98) 
at android.os.Looper.loop(Looper.java:164) 
at android.app.ActivityThread.main(ActivityThread.java:6541) 
at java.lang.reflect.Method.invoke(Native Method) 
at com.android.internal.os.Zygote$MethodAndArgsCaller.run(Zygote.java:240) 
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:767) 

[hanggrian]

Yes the fragment is supposed to be fixed, but I can see it still hasn't in your case. Let me gather more information to be more of help:

1. On fragment, by "button doesn't do anything", do you mean that next activity is started but onActivityResult is not triggered, or it won't even start next activity?

2. On Activity, it seems that it is caused by declaring onClick on XML. I will update it to more traditional setOnClickListener once no. 1 is answered.

I actually don't use RxJava that much, but paired with RxAndroid, it can do some pretty amazing asynchronous operations. Kotlin, on the other hand, has pretty much totally changed how I code. It's great that you started, give it some time and you will see that Kotlin's syntax is far more powerful than Java's.

[AndroidDeveloperLB]

1. I really mean it doesn't do anything. Anything at all. Nothing is opened. I just checked. "NextActivity" doesn't get opened. Debugging and putting a breakpoint in it doesn't go there.

2. ok.

About Kotlin, it has some good things, but it also has some disadvantages compared to Java. Java is clearer (less ambiguity than Kotlin). It is more open (overriding is enabled by default). It has more useful visibility modifiers (Kotlin lacks package modifier for classes, for example). Drives to be more efficient (Kotlin suggests to use fields using getters&setters, always, while on Java you can easily use fields when not needed). Less "junk" characters (Kotlin code has a lot of "?" and "!" because of null access). And some other things I forgot that got me annoyed with it.

I will miss Java. I worked with it for a lot of years.
But in all, Kotlin is not that bad. It does usually make code shorter, and I think that if you think enough of what you do , you can work fine with it.

Say, do you know perhaps about Rx as replacement for AsyncTask and AsyncTaskLoader? If so, can you please try to answer this:
https://stackoverflow.com/q/46884644/878126

[hanggrian]

It seems that those 2 bugs aren't related to onClick or Fragment at all, I just forgot to add default value of SharedPreferences, which again, my bad. Please try the example again as it has been updated.

I think that if you think enough of what you do , you can work fine with it.

Well that is the wisest sayings I've heard today, and there is certainly a truth in that. After all passion is what drives all of us in speaking these non-human languages.

I do use RxJava as a replacement for AsyncTask, or any asynchronous process like REST API. But to be honest, I haven't had a chance to use AsyncTaskLoader and therefore may be unqualified to answer that question.

[AndroidDeveloperLB]

About the sample, seems both fragment and activity examples work fine.
However, why does it keep saying "onError" for cancelling, finishing, and for custom result?
Shouldn't it provide a result no matter what? there can't be a real error for onActivity result, no?

About replacement to AsyncTask, I see you replied me there. Thanks.

[AndroidDeveloperLB]

Can you please update the gradle file instructions ? It still says I should use the problematic 0.9 version.
Or maybe the issue was only in the sample?

[hanggrian]

That actually is the way it is supposed to work. But I do admit it is another one of my fault for not spending much time in README.

But let me explain it here:
For every startActivityForResultAs...(), you should provide an expected result code in second parameter. If no such value is given, Activity.RESULT_OK will be used by default. The emitter will always emits onError() if result code of activity returned doesn't match the expected result when the activity is first started. I find this to be a good use of RxJava's onError(), but in future release I may add support for starting activity without this expected result code.

Version 0.9 isn't actually broken, it was just the example app. What kind of error are you getting from your build.gradle?

[AndroidDeveloperLB]

So error is just any other result, even if it's a result that can be normally be returned...
Isn't there a way to collect all possible results into the OK result ? After all, the OK is optional. You could return any result you wish, including custom one, with intent data, etc... :

Does your library support getting the Intent data result (including a bundle) ?

About issue with the repo, I thought it's an issue in both library and sample. You wrote it's just the sample, so ok.

[hanggrian]

Following pattern of my answer in your StackOverflow post, let's use Observable as example again:

startActivityForResultAsObservable(nextActivityIntent, RESULT_OK)
    .subscribe({ data -> 
        // if you reach here, then activity started has returned result code: RESULT_OK
        // data is the returned Intent (last param of onActivityResult())
        // to obtain Bundle, call data.extras
    }, { e ->
        // if you reach here, then activity returned has result code that is not RESULT_OK
        // e.g.: Activity is cancelled
        // you can still obtain Bundle by calling (e as ActivityResultException).data.extras
    })

// here you expect the result code to be 1234, it will throw onError() if you setResult() other than 1234 in your started activity
startActivityForResultAsObservable(nextActivityIntent, 1234)

I'm actually very open about future development of my libraries. Just like this one, I thought this was the simplest form of starting an activity without having if else code in onActivityResult() and without request code, but it turns out to be confusing to some.

[AndroidDeveloperLB]

I don't say it doesn't work. Just that it's not an error. I don't like to "ride" on callbacks that don't have the correct term.
Original code of onActivityResult can't have an error, so I'd expect everything in the same place.

I have issue with the app we work on, but this might be our code that has issues.

However, even though the sample seems to work fine, I think that the dialogs you create are shown by using the previously destroyed activity.
I can see it by adding a log in the onCreate of the activity, and in the "onNext" within the "onClick", writing what instance of the MainActivity is being used. I can see that for both, it's the same instance, even though the previous MainActivity instance was destroyed.

See here:

So you actually use the previously destroyed instance of MainActivity (memory leak) to handle the onActivityResult of the new one...

If you used a TextView instead of dialog, you'd see that nothing will occur on the new instance of MainActivty, because the text was set on the old one instead.

See why I think with Rx you have to be more careful than with Android...
:)

[AndroidDeveloperLB]

Tested with TextView. Got the same issue.
The text doesn't get updated, because it's set on an already destroyed activity.

[hanggrian]

Okay I can see your point, thank you for some amazing observation. I suppose it is fixable with activity/fragment instance check I'm currently working on. If not, then I'm afraid this library is doomed.

[AndroidDeveloperLB]

Sorry I don't have enough experience with Rx to help you on this.
I know only on normal Android framework how it's done: onActivityResult should handle the current instance.

[hanggrian]

I managed to fix this issue by passing newly created activity instance in onActivityResult(). There's only one major issue here: since onNext is customized, it is no longer viable to use RxJava's interfaces. And since this lib no longer depends on RxJava, it seems misleading to name it 'rx-activity'. I might need your help to rename this lib...

But for now, would you kindly confirm that example module is working? Only focus on activity-callback code as rx-activity should be deprecated now.

[AndroidDeveloperLB]

This time, the dialog doesn't get shown on onActivityResult() at all, whether I change orientation or not.

See here:

All I did is press on "activity", and then on "ok result". I didn't change the orientation.
Notice that "onResultOK" isn't written to the log.

[hanggrian]

That's odd, I could have sworn it is working fine on AVD and my Pixel XL. I have just pushed another commit, please try it again.

[AndroidDeveloperLB]

1. Each time I try to pull the changes it says I have merge issues. I think it's because you have some un-needed folders on the repo, which are specific to your PC configurations (maybe ".idea", not sure).
   I have to delete everything and clone the entire repo again and again, after each change you perform.

2. I still see the "startActivityForResult2" handling being called on the "onClick", which means it will still be called on the activity that died:

I think that the handling of the result should be set in the onCreate. Not in the "onClick". This is because "onCreate" is called before "onActivityResult", so it can be initialized there. "onClick" however is called only by the user.

3. You have Pixel XL ? I'm interested in this device. How is it? Does it have screen burn?

Anyway, I have to leave this library now. I have other tasks. I might try this at home though.

[hanggrian]

1. I'm not sure if it does have anything to do with me being on Mac and you being on PC, but it seems likely. I kinda don't know how to resolve this one.

2. Actually no, if you look at the last param of startActivityForResult2, it is T.(Int, Int, Intent?) -> Unit where T is the Activity passed from onActivityResult(). So this inside startActivityForResult2 will refer to instance passed from onActivityResult(), not died instance of Activity. But now I'm really baffled because we got different result:

3. It's the 1st generation though. Speaker could be a little louder, but overall it has been the best phone I've ever owned.

I understand, thank you for all your help!

[hanggrian]

I got it:

Your log seems to point to this@MainActivity, which already died. While mine point to this@startActivityForResult2, which is a newly created instance. I really do hope we continue this since it has been somewhat productive, but nevertheless thank you for all your help.

[AndroidDeveloperLB]

1. Just choose when commiting a change, to ignore the ".idea" folder that you have here:
   https://github.com/HendraAnggrian/rx-activity/tree/master/.idea

2. Actually yes. Every anonymous class has a reference to the class that contained it. It's not a static class you are creating. It's an inner class. There is access to the hosting class (MainActivity) and all of its fields and functions. If you let an anonymous class stay for longer than the Activity that hosts it, and then perform actions on it, they may not affect the new Activity that was created afterwards.
   To demonstrate it, you can use a TextView instead of log.

Here, add this TextView to your layout, just above the FrameLayout that has the id "container" :

        <TextView
            android:id="@+id/textView" android:layout_width="wrap_content"
            android:layout_height="wrap_content"/>

Then, change the "onClick" code to have these lines:

override fun onClick(v: View) {
    val textView = findViewById<TextView>(R.id.textView) // this was added
    startActivityForResult2(Intent(this, NextActivity::class.java).putExtra("from", "ACTIVITY")) { requestCode, resultCode, _ ->
        debug("$this Callback")
        supportAlert("Callback", "requestCode = $requestCode\nresultCode = $resultCode", OkButton)
        textView.text="got result..."  // and this was added
        if (resultCode == Activity.RESULT_OK) {
            supportActionBar!!.title = "damn you all to hell"
        }
    }
}

If you don't change orientation when the NextActivity is opened, everything seems ok, because the MainActivity is still the same.
However, if when you go to NextActivity, and then change orientation, you won't see that the TextView has changed, because you do it on the old MainActivity instance.

3. Does it have screen burn though? I'm worried because it has OLED screen.

[hanggrian]

But that TextView actually got changed, am I missing something here?

[hanggrian]

Oh I thought it was only second generation of Pixel that has those issues. Mine doesn't, but then again it's only about 3 months old.

[AndroidDeveloperLB]

For me it doesn't work.
I've uploaded a video to show the issue.
First I show the case that it works fine. Then I reset it by re-creating the MainActivity. Then, I perform the steps to show the issue.
See here:
https://a.uguu.se/DNNCEpROGDDV_device-2017-10-23-195356.mp4

And these are the changes in code:

[hanggrian]

if you findViewById outside startActivityForResult2, then it points to TextView of a destroyed Activity, no? Try putting findViewById inside startActivityForResult2 or simply in onCreate.

Here's the newest MainActivity.kt

[AndroidDeveloperLB]

Something is really weird here.
I did what you wrote, and now it works fine !
But how could it be?
The code of startActivityForResult2 runs on the old Activity. How does writing inside of it affects the new instance of the MainActivity? How when I do there findViewById , it does it on the new instance?
It doesn't make sense...
It's not anonymous class? Then what is it? And how can it reach the new MainActivity on its own? Doesn't it have a reference to the old one?
Is this a Kotlin magic? Or Rx?

I tried to debug the app, but something is wrong on my IDE/SDK because it fails to stop on the breakpoints I put. I tried on older version of the IDE, and on emulator and an older version of gradle . Still can't debug.
I think you just blocked debugging on the sample for some reason. In the gradle file, you've set bad properties for the debug buildType. I had to add this myself:
debuggable true
minifyEnabled false

During debug (after it worked), I noticed there is no reference to the real one. I can't access "this@MainActivity", unless I actually use it in code, which then causes memory leak.
Even weirder, if I call "this", it's actually of the MainActivity type, and it has the new instance.

I then tried to show the bytecode of the MainActivity.kt. The code of startActivityForResult2 isn't there. There isn't even the "applog" text I've put. In fact, even the word "startActivityForResult2" isn't there...

Please explain what is going on. This is very different from what I know on Java, and doesn't make sense to me at all.

[hanggrian]

It's actually not that complicated, and no RxJava is involved this time.

The last parameter of startActivityForResult2 (callback) is a function types that brings 4 values: Activity/Fragment (T), request code, result code, and nullable Intent. You may see it clearer in Java:

But with Kotlin, that Activity/Fragment is the receiver of that function type. When you call findViewById() within startActivityForResult2(), the java version would look like activity.findViewById(), where Activity is retrieved from first parameter of onActivityResult2():

Since startActivityForResult2() and onActivityResult2() are extension functions, we do not supply it as a param but instead as receiver.

Oh and all of them are have inline keyword, so I expect the bytecode to replace startActivityForResult2() with actual lines of code inside startActivityForResult2(). But I really haven't seen it myself.

It isn't different at all than what you know in Java, but I suppose shorter syntax makes it look confusing at first glance.

[AndroidDeveloperLB]

This brings so many questions:

1. This is an anonymous function. Isn't it the same as anonymous class on Java, just with a function?
2. Doesn't it have a reference to the higher order scope? Otherwise, how could it reach it when I try to reach it...
3. They write on the link you've provided, that unlike Java, I can change the values of the higher order scope. Does it really modify them, or just a new instance of them (a clone) ?
4. You say the first parameter of the Function is Activity/Fragment, but I see in the Java code you've provided it's only Activity.
5. How did you reach the Java code for this? I can't find startActivityForResult2 in the MainActivity Java code...
6. Why would "this" actually call the first parameter? Doesn't look intuitive at all... The "this" was supposed to refer to the function... Or is it impossible ?
7. How does the function live beyond the Activity ?

To me it seems a lot of code here is hidden.

[hanggrian]

1. It is anonymous, function types will always be translated to interfaces in Java.
2. It does have a reference to higher order scope. It's just that since that function type has receiver, this will point to that receiver. If you want to refer to higher order, this@MainActivity is equivalent to MainActivity.this.
3. I believe they modify. Although I'm not really sure how they do it behind the curtain, they might make a clone without us knowing.
4. It is actually generic T that I limit only to accept Activity, Fragment, or support Fragment.
5. I will write the java code equivalent to existing kotlin example to clear things up.
6. To refer to the function, you may call this@startActivityForResult2, that would point to Activity/Fragment that called that function. But if the function type doesn't have a receiver, this would automatically refer to this@startActivityForResult2.
7. I'm not sure what this one means.

Yes, I have accepted it as the price to pay to have shorter syntax.

[hanggrian]

Please take a look at newly added MainActivity2.java and NextActivity2.java.

[AndroidDeveloperLB]

1. So it's the same?
2. If it has a reference to the MainActivity that's getting destroyed, this is a memory leak. Very short one, but still...
   I couldn't see that it has a reference to the old one during debug though. Are you sure it has a reference? If so, how did you find it?
3. My guess is that it's like on C#. It gets cloned, so that the value doesn't really change in the original place. However, if it's a special object, it might get modified within. For example, if it's a list, and you modify one of its items, I think it will change.
   Need to test it to verify, of course.
4. But I see something else in the Java function you showed.
5. I meant how you showed the Java code behind Kotlin, to see the code of startActivityForResult2 inside MainActivity.
   Looking at the code you've shown now, this has a reference to the old MainActivity that's destroyed.
6. How odd.
7. Never mind.

[hanggrian]

1. Yes, sir.
2. I'm not sure, it's just my assumption. You clearly have tested it more than I do, so I imagine your findings are far more reliable than mine.
3. If they do clone some of the variables, won't their hash code differ from the original ones?
4. It is generic, I just didn't specify the explicit type argument: ActivityCallbacks.<MainActivity2>startActivityForResult(...)
5. Yes that is why I assume no. 2 has a reference of dead MainActivity. But if we don't use anything from that dead activity, is it still a memory leak?
6. I typically see it this way: always note whether or not any function you are invoking has receiver.

[AndroidDeveloperLB]

I completely forgot what this is all about...
2017...

[hanggrian]

Hello old friend, back then we were leveraging RxJava and Kotlin for Android permission and activity results because we hate request codes LOL. But nowadays Android has a native ActivityLauncher, so this library is reduced to only permission.

I wish good luck to you and your career.

[AndroidDeveloperLB]

OK thanks