Project

General

Profile

Actions

Bug #1136

closed

Sometimes a new tab is created without a corresponding new page

Added by Lasse Gismo 10 months ago. Updated 8 months ago.

Status:
Closed
Priority:
Next Release
Start date:
12/12/2023
Due date:
% Done:

0%

Estimated time:

Description

I've PB Version 3.16 running on lineageOS and when opening an additional website, there're some open, via link, only an empty page raises and the "X" closes PB completely.


Files

Actions #1

Updated by Soren Stoutner 10 months ago

  • Priority changed from 3.x to Next Release

This is likely a crash of some sort. Can you please attach a logcat?

Actions #2

Updated by Soren Stoutner 10 months ago

By the way, you can get a Logcat from the Navigation menu inside Privacy Browser.

Actions #4

Updated by Soren Stoutner 10 months ago

  • Subject changed from Behavior on opening additional links to Opening a link in a new tab opens empty page, which crashes when closed
  • Status changed from New to Feedback

This might have the same underlying cause as Bug #1134: Sometimes the tab after a new tab in the middle of a list is set as active, but I am not completely sure what the issue is in either case. I will have to spend some time digging into it.

Does this problem happen every time you open a link in a new tab or only sometimes?

Actions #5

Updated by Lasse Gismo 10 months ago

Damn demonstration effect :-)
No, not allways, last 5 openings ran smoothly.
Will come back after next occurance(s) with new logcat.

OK?

Actions #6

Updated by Soren Stoutner 10 months ago

That logcat is sufficient as it shows the crash clearly. I just needed to know if this always happened, as that indicates what the underlying cause is. In this case, with it being intermittent, the most likely cause is a race condition between the creation of the tab and the WebView that leaves the system out of sync, but I am not yet certain where that is occurring.

Actions #7

Updated by ask low 10 months ago

Same issue as of #1134, but different description. Although still can't reproduce this consistently. It's still random.
And no. Closing the new tab doesn't crash PB for me. It just exhibits the bug. That's it.

Actions #8

Updated by Soren Stoutner 10 months ago

You won't be able to reproduce it consistently. It is what is called a race condition, which means that two things are happening simultaneously in the background (a race). If one finishes first, everything is fine. If the other finishes first, there is a crash.

Race conditions are very tricky because sometimes they only manifest on certain hardware or on certain versions of Android or under other conditions that make them hard to replicate. Among other things, that can make it hard to know when you have fixed them.

Actions #9

Updated by ask low 10 months ago

Maybe the implementation of opening new tabs as adjacent (#1100), is still incomplete & requires proper lookup ?

A quick search on avoiding race conditions, says that the critical section of a resource sharing has to be atomical, as well as proper thread synchronisation.

Actions #10

Updated by Soren Stoutner 10 months ago

That's possible, although it could also be that the bug has existed for a while and nobody noticed or that it was introduced by some other recent change different from the opening of tabs adjacently.

Most likely it relates to all the problems in switching from ViewPager to ViewPager2 in Privacy Browser 3.14 with Feature #587: Switch FragmentPagerAdapters to FragmentStateAdapters. ViewPager2 is much more asynchronous than the previous implementation, which has ended up introducing dozens of bugs into the project (most of which I fixed in my testing before they were ever released to the public). As almost all of them are race conditions (which is what you get when you make things asynchronous), finding and squashing them all has been rather difficult. For example, Feature #1051: The first page is sometimes displayed if the app is restarted while a new intent is being loaded, Bug #1020: Unencrypted Website dialog sometimes erroneously pops up, and Bug #1069: App crashes after it has been paused and restored are all examples of race conditions introduced by ViewPager2 not always finishing creating new pages when they are expected by the rest of the app.

Actions #11

Updated by Soren Stoutner 10 months ago

@Lasse Gismo, can you please test this alt build of Privacy Browser to see if it resolves the problem for you? You can install it alongside your existing Privacy Browser. The link below will expire in about a week.

https://nextcloud.stoutner.com/s/S2eqBQBZL2q5Eaq

Actions #12

Updated by Soren Stoutner 10 months ago

The version in the above link has installation problems. Try this one:

https://nextcloud.stoutner.com/s/BHLZ4b9Ziw4JNKW

Actions #13

Updated by ask low 10 months ago

Installed but I'm not sure how to reproduce race condition. Been using it for a while & haven't happened yet.

Actions #14

Updated by Soren Stoutner 10 months ago

I have sometimes had to use a potential change for 2-4 weeks to be sure that a race condition is fixed. You can export your settings from your main Privacy Browser into this test version and use it for a while to see if you have any problems.

Actions #15

Updated by Lasse Gismo 10 months ago

A parallel installation of the test version in addition to my main PB is unproblematic?

Actions #16

Updated by Soren Stoutner 10 months ago

Yes. I designed the test version so it can be installed alongside the normal version without any problem. You will have two icons, one with the word TEST across the bottom.

Actions #17

Updated by ask low 10 months ago

Soren Stoutner wrote in #note-16:

Yes. I designed the test version so it can be installed alongside the normal version without any problem. You will have two icons, one with the word TEST across the bottom.

I would rather call it beta version instead of test version, since you're trying the changes that move into the standard version.

Not just this, but you can experiment a lot of other stuff. If you felt some feature stable & polished move that into standard version. If not, keep it experimental until it matures.

Which is why I suggested you to open a beta channel & release beta version (on redmine caz fdroid would take lot of time)

Actions #18

Updated by Soren Stoutner 10 months ago

There is actually an official test version that is part of every release, listed as the Alt version. It makes it easy to compare two releases side-by-side or do things like test import/export conversions between two particular versions. You can download them from:

https://www.stoutner.com/privacy-browser-android/changelog/

I might someday do a beta version, which is different from the Alt (test) version. But that is a different discussion.

Actions #19

Updated by Lasse Gismo 10 months ago

Not yet able to reproduce the issue with the alt build - looks promissing.

Actions #20

Updated by ask low 10 months ago

@Lasse Gismo Your issue could be different to mine. But happened on mine
https://redmine.stoutner.com/issues/1134#note-10

Actions #21

Updated by Soren Stoutner 10 months ago

That is interesting. I was expecting the change I made to fix Bug #1134: Sometimes the tab after a new tab in the middle of a list is set as active, but I wasn't sure it would fix this. Looks like it was the reverse of what I expected.

Actions #22

Updated by Lasse Gismo 9 months ago

Had the issue once again short before the timestamp (name) of the log attached.
Mostly open links out of the Fedilab app.
Seems to be, nearly, fixed from my point of view.

Actions #23

Updated by Soren Stoutner 9 months ago

Try this updated build. It has an additional change that might make the difference.

https://nextcloud.stoutner.com/s/BxtSX3NczFQiLyc

Actions #24

Updated by Lasse Gismo 9 months ago

The new build disimprove the behavior.
Installed YD, had 3 occurrences meanwhile.

Actions #25

Updated by Soren Stoutner 9 months ago

I can't imagine any way it would make things worse. Rather, it probably has to do with the variableness of the symptoms. The increase in the behavior is much more likely to be affected by whatever is happening on your device in the background. That is often the way race conditions are.

Actions #26

Updated by Soren Stoutner 9 months ago

Is there some way you could record a video of the crash? I realize this is problematic for crashes that don't occur consistently, but it might be helpful in determining what the underlying cause is.

Actions #28

Updated by Lasse Gismo 9 months ago

There's no video required - its only a new tab with empty content and to close the tab, is to close the app.

Actions #29

Updated by Lasse Gismo 9 months ago

An additional, hopefully helpful, picture that shows 1 old and the new tab.
Tabbing on the new tab, shows information from the old and closing the new via X closes the app.

Actions #30

Updated by Soren Stoutner 9 months ago

  • Subject changed from Opening a link in a new tab opens empty page, which crashes when closed to Sometimes a new tab is created without a corresponding new page

This is an interesting piece of information. It looks like a new tab is being created but a new page is not.

What is the procedure you are using to create the new tab?

Actions #31

Updated by Lasse Gismo 9 months ago

It's a simple click on a link in Fedilab.

Actions #32

Updated by Soren Stoutner 9 months ago

1. Does this ever happen when loading a link from inside the app? For example, if you long-press on a link and choose Open in New Tab. Or, if you use the + icon to open a new tab.

2. When this does happen when loading a link from Fedilab, is Privacy Browser recreated on load? You can tell if this is happening because the Privacy Browser logo will temporarily display on the screen. For context, you can check out the Android Activity Lifecycle:

https://developer.android.com/guide/components/activities/activity-lifecycle

If this is the case, you can force this to happen whenever you want for testing purposes by opening Privacy Browser, then opening 10-20 other apps (enough to force the OS to kill Privacy Browser to recover RAM), then opening a link from Fedilab.

3. Please post a copy of About > Version from inside Privacy Browser.

Actions #33

Updated by ask low 9 months ago

I believe there have to be some debugging tools for issues like these. As you said race conditions are very common. But whether handling is difficult or not I donno caz I ain't a dev.
But I've heard one can literally code an entire project with just try catch clauses is that true ?

Actions #34

Updated by Lasse Gismo 9 months ago

My answers:
1.) As I said - via link click in fedilab
2.) Didn't occur when PB is started with the click - only when there're already tab(s) open
3.) Pls. see attached

Actions #35

Updated by Soren Stoutner 9 months ago

Number 2 is very important. Let me explain further.

Android is constantly killing apps in the background. It does this for power saving issues, and to recover RAM needed by other apps, and sometimes for no good reason. Please see the attached graphic taken from Google's documentation.

Whenever an app leaves the screen, it is stopped and restarted. That is the portion of the graphic on the right. But, when an app is killed, then it has to be recreated when the user switches back to it. That is the process on the left of the graphic. This is all supposed to happen transparently to the user. Often they don't realize that their apps are constantly being killed and recreated. Developers, on the other hand, need to do a massive amount of work to recreate all the tabs, content, and other aspects of the app in the same state it was in before it was killed.

I suspect that this problem is only manifesting for you when Privacy Browser is killed in the background and then restarted by an intent sent from another app (Fedilab). Being able to confirm if that is the case or not is very important to being able to figure out the root cause and find a solution. The way you can tell if that is happening is that Privacy Browser has to reload the filterlists if it has been killed and restarted, which you can watch happen because the Privacy Browser logo will be displayed in the middle of the screen and the names of each of the filterlists will very briefly be displayed below it (similar to what happens when starting Privacy Browser from scratch, but in this case all the previous tabs will be restored after this step). This does not happen when the app is only stopped and restarted.

If this is the case, you can reliably recreate the behavior following the steps in #note-32. That makes it easy to test proposed solutions. If it is not the case, or if it is not consistent, then it makes things a bit more difficult.

Actions #36

Updated by Lasse Gismo 9 months ago

Ok - will take care of.

But - issue only happens when there're already tabs shown, after app kill (with super freezZ) and link click there's only the new one shown and that worked/s correct.

So - not sure what to do further.

Actions #37

Updated by Soren Stoutner 9 months ago

What happens if you have the OS kill the app in the background? This is different from what SuperFreezZ does (the onDestroy at the bottom of the previous graphic), because when the OS does it the saved instance state is preserved, while when SuperFreezZ does it the saved instance state is deleted. You can test this by opening Privacy Browser, then opening 20-30 other apps, then opening a link from Fedilab. You will know that you have reproduced the app kill behavior because Privacy Browser will briefly display the app logo in the center of the screen with the loading filterlists text. Then you will see all the tabs restored, and the new tab created, although my guess is that in your case the new tab is not created correctly, which is why it displays the content from the previous tab and why it ends up causing the crash when you try to close it.

Figuring out if this is what happens is very important to getting to the bottom of the problem, because the code path for opening a new tab when recreating the app after it is killed is different than the code path for opening a new tab otherwise. Figuring out which case is causing your problem leads to looking in the right place to find the fix.

If I were able to reproduce this behavior on my devices it would be easier for me to troubleshoot. But this appears to be something fairly specific to the hardware and OS you have running. So, the more information I can get from you as to what exact circumstances trigger the problem, the more likely it is that I can find a solution. This differentiating of which code path is the problem is the first important step toward narrowing this down.

Actions #38

Updated by Lasse Gismo 9 months ago

OK, will stop walking my own way and follow yours - cmome back with results.

Actions #39

Updated by Lasse Gismo 9 months ago

Yep - did it your way with the behavior as expected.
PB killed via OS, link clicked, PB restored with earlier tabs and new tab empty.
The only inrofmationthe be seen, is the certificate info of the new page (mouse over tab)
Log attached with timestamp - good luck.

Actions #40

Updated by Soren Stoutner 9 months ago

This is good because it provides us a method to test prospective fixes. It is also good because it allows me to focus my hunt for the problem. I will look into it and get back with a test build once I think I have found a solution.

Actions #41

Updated by Soren Stoutner 9 months ago

I have encountered this problem for the first time on one of my devices. Attached is a logcat.

[ 01-03 15:49:52.622  7081: 7081 E/AndroidRuntime ]
FATAL EXCEPTION: main
Process: com.stoutner.privacybrowser.standard, PID: 7081
java.lang.IllegalStateException: Could not execute method for android:onClick
    at androidx.appcompat.app.AppCompatViewInflater$DeclaredOnClickListener.onClick(AppCompatViewInflater.java:473)
    at android.view.View.performClick(View.java:7659)
    at android.view.View.performClickInternal(View.java:7636)
    at android.view.View.-$$Nest$mperformClickInternal(Unknown Source:0)
    at android.view.View$PerformClick.run(View.java:30156)
    at android.os.Handler.handleCallback(Handler.java:958)
    at android.os.Handler.dispatchMessage(Handler.java:99)
    at android.os.Looper.loopOnce(Looper.java:205)
    at android.os.Looper.loop(Looper.java:294)
    at android.app.ActivityThread.main(ActivityThread.java:8177)
    at java.lang.reflect.Method.invoke(Native Method)
    at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:552)
    at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:971)
Caused by: java.lang.reflect.InvocationTargetException
    at java.lang.reflect.Method.invoke(Native Method)
    at androidx.appcompat.app.AppCompatViewInflater$DeclaredOnClickListener.onClick(AppCompatViewInflater.java:468)
    ... 12 more
Caused by: java.lang.NullPointerException: null cannot be cast to non-null type android.widget.FrameLayout
    at com.stoutner.privacybrowser.adapters.WebViewStateAdapter.deletePage(WebViewStateAdapter.kt:98)
    at com.stoutner.privacybrowser.activities.MainWebViewActivity.closeTab(MainWebViewActivity.kt:4074)
    ... 14 more

Interestingly, this comes after these lines, which are Bug #1106: New intents sometimes aren't loaded on restart on Android 14 (WebView crash):

[ 01-03 15:49:18.423 22476:22535 W/cr_ChildProcessConn ]
onServiceDisconnected (crash or killed by oom): pid=4805 bindings:W  S

[ 01-03 15:49:18.455 22476:22535 W/cr_ChildProcessConn ]
onServiceDisconnected (crash or killed by oom): pid=9271 bindings:W  S

[ 01-03 15:49:18.519 22476:22535 W/cr_ChildProcessConn ]
onServiceDisconnected (crash or killed by oom): pid=22844 bindings:W  S

[ 01-03 15:49:18.534 22476:22535 W/cr_ChildProcessConn ]
onServiceDisconnected (crash or killed by oom): pid=22650 bindings:W  S

[ 01-03 15:49:18.551 22476:22535 W/cr_ChildProcessConn ]
onServiceDisconnected (crash or killed by oom): pid=22647 bindings:W  S

[ 01-03 15:49:18.568 22476:22535 W/cr_ChildProcessConn ]
onServiceDisconnected (crash or killed by oom): pid=22631 bindings:W  S

[ 01-03 15:49:18.629 22476:22535 W/cr_ChildProcessConn ]
onServiceDisconnected (crash or killed by oom): pid=22679 bindings:W  S

[ 01-03 15:49:18.657 22476:22476 E/chromium ]
[ERROR:aw_browser_terminator.cc(154)] Renderer process (4805) crash detected (code -1).

[ 01-03 15:49:18.668 22476:22476 E/chromium ]
[ERROR:aw_browser_terminator.cc(154)] Renderer process (9271) crash detected (code -1).

[ 01-03 15:49:18.687 22476:22476 E/chromium ]
[ERROR:aw_browser_terminator.cc(154)] Renderer process (22844) crash detected (code -1).

[ 01-03 15:49:18.710 22476:22476 E/chromium ]
[ERROR:aw_browser_terminator.cc(154)] Renderer process (22650) crash detected (code -1).

[ 01-03 15:49:18.716 22476:22476 E/chromium ]
[ERROR:aw_browser_terminator.cc(154)] Renderer process (5311) crash detected (code -1).

[ 01-03 15:49:18.723 22476:22476 E/chromium ]
[ERROR:aw_browser_terminator.cc(154)] Renderer process (22647) crash detected (code -1).

[ 01-03 15:49:18.739 22476:22476 E/chromium ]
[ERROR:aw_browser_terminator.cc(154)] Renderer process (4795) crash detected (code -1).

[ 01-03 15:49:18.747 22476:22476 E/chromium ]
[ERROR:aw_browser_terminator.cc(154)] Renderer process (4869) crash detected (code -1).

[ 01-03 15:49:18.753 22476:22476 E/chromium ]
[ERROR:aw_browser_terminator.cc(154)] Renderer process (22631) crash detected (code -1).

[ 01-03 15:49:18.759 22476:22476 E/chromium ]
[ERROR:aw_browser_terminator.cc(154)] Renderer process (22679) crash detected (code -1).

[ 01-03 15:49:18.764 22476:22476 E/chromium ]
[ERROR:aw_browser_terminator.cc(110)] Render process (4805) kill (OOM or update) wasn't handed by all associated webviews, killing application.

For me, I only see Bug #1106: New intents sometimes aren't loaded on restart on Android 14 (WebView crash) on Android 14, which is why it is missing from the logcats submitted by @Lasse Gismo. But it has a similar effect in that it causes the app to be recreated and the tabs to be restored. In this once instance that then caused this bug, which is that the number of tabs ends up being one more than the number of pages.

Actions #42

Updated by Soren Stoutner 9 months ago

@Lasse Gismo, can you try this test build and see if it resolves the problem for you? As usual, the link will expire in a few weeks.

https://nextcloud.stoutner.com/s/jiaP8ayi7EXmbFL

Actions #43

Updated by Lasse Gismo 9 months ago

Looks good until now.
Will come back in a few days with results.

Actions #44

Updated by ask low 9 months ago

https://nextcloud.stoutner.com/s/jiaP8ayi7EXmbFL

This test build has a severe issue of tabs hanging in the middle of nowhere

Actions #45

Updated by Soren Stoutner 9 months ago

I don't understand what you are describing. Can you please be a little more explicit?

Actions #46

Updated by ask low 9 months ago

Tabs hang in the sense the site interface don't respond to scroll, touch, etc until I close the tabs. And it's random too.
I'm back to the previous alt build now & it doesn't happen.

Actions #47

Updated by Soren Stoutner 9 months ago

I can't see any way that any of the changes in this test version could cause that type of behavior. Rather, I would imagine it has something to do with Android System WebView itself and either an incompatibility with the website you are visiting or with one of the changes you are making as root where you are redirecting directories to /dev/null.

For example, your logcat has a lot of these:

[ 01-11 06:55:47.438 32491:32491 I/auditd   ]
type=1400 audit(0.0:108921): avc:  denied  { read } for  comm="ThreadPoolForeg" name=48545450204361636865 dev="dm-4" ino=36658 scontext=u:r:untrusted_app:s0:c156,c256,c512,c768 tcontext=u:object_r:app_data_file:s0 tclass=lnk_file permissive=0 app=com.stoutner.privacybrowser.alt

And a lot of these:

[ 01-11 09:20:01.054 10056:10085 W/chromium ]
[WARNING:cache_util.cc(144)] Unable to delete cache folder.
Actions #48

Updated by ask low 9 months ago

That's strange. I've symlinked /data/data/com.stoutner.privacybrowser.alt/\
cache/WebView/Default/HTTP\ Cache
to /dev/null like how standard version used to be. The recent test build #1136#note-42 has hang issues, but strangely don't have this issue on #1134#note-12 build.

Actions #49

Updated by Soren Stoutner 9 months ago

Whatever is causing the problem, I can't imagine any way it relates to the changes in the code between those two versions.

Actions #50

Updated by Soren Stoutner 9 months ago

@ask low, if you have made any changes to WebView DevTools recently, you might try undoing them temporarily to see if they are responsible for the behavior you are seeing.

Actions #51

Updated by ask low 9 months ago

I actually had that issue & reverted back to my last alt build, even before I messed with DevTools.
You can see my comment timings of tabs hang log & your comment of DevTools recommendation. There's an hour difference.

Infact, I'm on the alt build from #1134#note-12 with the 4 flags enabled in DevTools. Browsed for almost 4 more hours without any hangs.

Actions #52

Updated by Soren Stoutner 9 months ago

I have committed the test build in #note-42 as it is likely to make progress even if it isn't the full fix. https://gitweb.stoutner.com/?p=PrivacyBrowserAndroid.git;a=commitdiff;h=70da2a558d6e35c38adb596479c36f7e451dc9a5;ds=sidebyside

However, I will leave this open until we hear back from @Lasse Gismo if this completely resolves his problem.

Actions #53

Updated by Lasse Gismo 8 months ago

After 2 weeks of working with, I'll declare the described bug as fixed with that release - congrats to Soren and many thanks.

Actions #54

Updated by Soren Stoutner 8 months ago

  • Status changed from Feedback to Closed

Thanks for the followup information.

Actions

Also available in: Atom PDF