Fenix: [Bug] 1 second delay in page load from browser resizing during URL navigations

Created on 13 Feb 2020  ·  23Comments  ·  Source: mozilla-mobile/fenix

This issue was raised on GeckoView under bug 1613488. It was determined to be something that needed to be handled in either Fenix or Android Components.

Please see the discussion there.

P1 performance P1 🐞 bug

Most helpful comment

I tested this in a custom build from master and can confirm that it's fixed! Nice work!

Profile: https://perfht.ml/3exF5j9
The screenshots in the screenshot track keep their full size throughout the profile, and there's no time spent in resize DOM events or expensive resize reflows any more. The only CNN code that runs once I navigate away from it is the unload event handler, which is expected.

And I can see that loadURI is called in the Gecko parent process before the Java UI thread runs BrowserFragment.onCreateView(). Excellent!

All 23 comments

I'm writing up more detail here to make things easier:

Steps to reproduce

Have cnn.com loaded in Fenix. Then tap the URL bar, enter google.com, and press enter.

Expected behavior

GeckoView should keep its full size the entire time. It's not visible while the URL bar is focused - there's an autocomplete overlay that hides it. The google.com load should start as quickly as possible.

Actual behavior

While the URL bar is focused, GeckoView is resized to make space for the keyboard. Once the new URL is submitted, GeckoView is resized back to the full size.
Both resizes cause the new page load to be delayed by expensive resizing work. Profile: https://perfht.ml/3bBrh5R
You can see that the screenshot size changes as you move your mouse through the screenshot track in the profile. And you can see that the first network load in the content process is only started after 1.2 seconds of expensive work have completed:

  • There are cyan and blue bars in the marker track of the web content process - these are restyles and reflows due to the resize.
  • There's also a bunch of JavaScript running; this is triggered by a DOM "resize" event that is fired in content.

Device information

  • Android device: Moto G5
  • Fenix version: Nightly 200212 06:01 (Build #20430606) 2.0.0, d83345a8

@csadilek is this something that you want to look at? Is there a place we want to put these issues?

@mstange To be clear, the platform perf team feels that there is nothing for the platform team to change and the front-end team should make this change, right?

@csadilek @boek This might be a tricky change for the FE perf team (given the cascading effects on the user experience) – can we get some help from the Fenix team?

(note: I see no indication of _how_ expensive this is – maybe it's in the profile but I haven't checked – so maybe we want to figure that out before prioritizing)

(note: I see no indication of _how_ expensive this is – maybe it's in the profile but I haven't checked – so maybe we want to figure that out before prioritizing)

@mstange How expensive is this in practice? i.e. which device, which build variant, and how long does it take?

@mstange How expensive is this in practice? i.e. which device, which build variant, and how long does it take?

Sorry, I'm moving too quickly: I see a device and build specified in comment 0. I don't know how to read the profiles: do you mind telling us what the approximate perf improvement would be (in ms?) from addressing this change?

According to the profile, when the user is currently on cnn.com, the perf improvement would be over a second. That's pretty huge.

The information was hidden in this part of my comment:

And you can see that the first network load in the content process is only started after 1.2 seconds of expensive work have completed

It really depends on the page though. That is, it depends on the page that's currently loaded in the tab: that's the page that's getting resized.

Triage: we thought we would check if this blocks the Java main thread to see what impact it has on the user but as mstange mentions above, this delays page load by about a second and still sounds worth fixing.

In triage, we were concerned that we would be unable to find a front-end fix for this but it's already been investigated on the GV side.

I'd like to follow up with @csadilek on best next steps here.

Leaving in triage column so we don't forget about this bug.

Talking to Christian, the main question seems to be: why is the GV view still in the hierarchy when the search fragment is visible? (and GV is not visible)?

Apparently the GeckoView instance is still in the hierarchy when the resize occurs. Fenix might be able to avoid this with some careful trickery.

Triage: we expect the Fenix team to take a look. @liuche Is that okay?

@mcomella, @mstange, are we able to test this again now that all of these animations are in? I'm wondering if this has changed at all given that I'm doing a bit of wonky-workarounds for the engine view animation (hiding it, then setting it to visible).

This may have made this worse or better 😅 but I think it's likely it affected this.

Also happy to test this on the Fenix side if you're able to point me towards a tool for doing so 😄

Yes, this is still happening. Profile: https://perfht.ml/3atAfkq
I'm only testing the case when using the URL bar in an existing tab to navigate that tab. Do the new animations affect that case? I thought they were mostly about going into / out of a tab.

The profile again is of navigating from cnn.com to google.com. The first network request to google.com is delayed by 2 seconds.

Triage: this is a really high impact issue for improving page load times and the Fenix team's expertise would be useful here.

@ekager do you have time to take a look at this one? @csadilek asked me to prioritize it, you can ask him for help on it.

Some Fenix eng took a look, but will wait to pick this up next sprint after taking the Gecko/Firefox profiling training because of the learning curve in profiling.

@liuche I am assigning it to myself. I want to test this again and look into it this week.

Also need to verify if this has improved after our fix of https://bugzilla.mozilla.org/show_bug.cgi?id=1628449 (see https://github.com/mozilla-mobile/fenix/issues/8782#issuecomment-611148467.)

OK, some updates:

  • I do see a significant improvement here on latest master, even compared to Play Store Nightly which is from a week ago. This is likely due the page load fixes we landed last week.

  • However, I still see the resizing happening after confirming the URL in the Awesomebar. I also see this effect in both sample and reference browser, which have a different implementation. They use A-Cs AwesomebarFeature directly. That's interesting because there we actually hide the engine/gecko view when the awesomebar is displayed, but that doesn't seem to stop it from resizing either: https://github.com/mozilla-mobile/android-components/issues/6664

To be continued...

More progress:

  • The A-C bug is fixed now which resolves the problem in sample and reference browser. As mentioned above, we did already hide the engine view when displaying the awesomebar, but that didn't actually propagate to the nested GeckoView: https://github.com/mozilla-mobile/android-components/pull/6695

  • The root problem is in GV though and just worked around for now. Kudos to @jonalmeida for having the right intuition about the root cause: https://bugzilla.mozilla.org/show_bug.cgi?id=1630775

  • In Fenix, as we don't use A-C's AwesomeBarFeature, we still need to make sure to set the visibility of the engine view when navigating to/from the SearchFragment. @ekager and I looked at this briefly and will continue tomorrow once we have the A-C fix published.

@mstange Thanks for filing!

We've landed patches in A-C and Fenix now. I no longer see the resizes caused by the keyboard and the corresponding reflows / layout work. On the two devices I have here (unfortunately both are fairly fast) I see a much shorter gap between initializing the browser fragment (going back from the search fragment and awesome bar to the browser) and the new network request. I think we can improve the time it takes to initialize the BrowserFragment in a follow-up, but that's a different problem than you described here.

When you get a chance, can you verify that this is improved / fixed now? Next Nightly build to the Play store goes out Monday morning. Otherwise, it would have to be a custom build from master.

Thanks again.

I tested this in a custom build from master and can confirm that it's fixed! Nice work!

Profile: https://perfht.ml/3exF5j9
The screenshots in the screenshot track keep their full size throughout the profile, and there's no time spent in resize DOM events or expensive resize reflows any more. The only CNN code that runs once I navigate away from it is the unload event handler, which is expected.

And I can see that loadURI is called in the Gecko parent process before the Java UI thread runs BrowserFragment.onCreateView(). Excellent!

Thanks @mstange.

Was this page helpful?
0 / 5 - 0 ratings