827370 - When a web page is opened via an intent, we should not load about:blank first

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Jared Wein [:jaws] (please needinfo? me)]

STR:
Use an external app like Twitter to open a link with Firefox
When Firefox opens, notice that the address bar shows about:blank (easier to see on 2G)
Then the web page starts to load.

There are either two things that could be done here. First, we shouldn't show "about:blank" in the address bar. It is technical information that isn't valuable to the user, and gives the user the idea that Firefox is doing unnecessary work.

Second, maybe we don't need to load about:blank here (although I don't know what we would do in its absence).

Comment 1

[Aaron Train [:aaronmt]]

Bug 729911 / Bug 725660? Sounds like a regression.

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

I notice this happening to me on Nightly as well. Same STR.

Comment 3

[Paul Feher]

I think this is not a regression I can trace this back to Firefox 12

Comment 4

[Aaron Train [:aaronmt]]

There are two things here; showing 'about:blank' in the address bar was once fixed and is now re-occuring, regression. Actually showing 'about:blank' in content, is not, as it's always been there. Let's use this bug for finding the regression for showing 'about:blank' the string, in the address-bar.

Comment 5

[Matt Brubeck (:mbrubeck)]

This was originally fixed in bug 725660 (Firefox 13) so it must have regressed since then.

Comment 6

[Matt Brubeck (:mbrubeck)]

Firefox 18 is not affected, but Firefox 19 (Beta) is affected.

Comment 7

[Matt Brubeck (:mbrubeck)]

I think this is a regression from bug 800199.  I confirmed using m-c nightlies that the regression is in this range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=dac5700acf8b&tochange=cb573b9307e5

Comment 8

[Brian Nicholson (:bnicholson)]

Attachment: Remove RedirectorRunnable

As is, the RedirectorRunnable code is buggy and not working as intended. The overridden afterLoad() method isn't even being called since the method is private: http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/GeckoApp.java#1964. This is responsible buggy behavior I'm seeing when launching Twitter shortcuts, such as:
1) about:blank showing up for a long time before the page loads
2) occasionally, *only* about:blank loads and page never loads
3) onNewIntent() is fired many times (often 20+)

The bug as filed happens because when using a RedirectorRunnable, we first create an empty (about:blank) tab, then send a Tab:Load event to Gecko on the empty tab once we know the redirected URL.

Even with the fixed overridden method, I'm not seeing any performance benefits when using the RedirectorRunnable. Since it complicates the startup code, I think we should just get rid of it. I'm not sure how much the PrefetchRunnable code is helping either, but I kept it since it doesn't interfere with startup like the RedirectorRunnable.

I did, however, remove the PrefetchRunnable code from onNewIntent(). We're creating a PrefetchRunnable in two places (the other being initialize), but I don't think there's much benefit in creating one if Gecko is already running.

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 708812 [details] [diff] [review]
Remove RedirectorRunnable

It's questionable whether Prefetch has any impact, but it's straight forward. Spinning up the modem before Gecko starts (and needs to spin up the modem) seems like a common thing and a potential win.

Only using it when we initially launch seems like a good idea, but we might find that we're in the background long enough between uses that the modem winds down between Firefox uses. Maybe for those cases we'd be OOM'd in the background though.

We might want to add telemetry (in a followup bug) for checking modem state in onNewIntent.

RedirectorRunnable is a good idea, but I think the complexity it adds to startup causes more than a few issues.

Comment 10

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/bb11e1f204ca

Comment 11

[Brian Nicholson (:bnicholson)]

Filed bug 837233 for adding the RedirectorRunnable back.

Comment 12

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/bb11e1f204ca

Comment 13

[Brian Nicholson (:bnicholson)]

Comment on attachment 708812 [details] [diff] [review]
Remove RedirectorRunnable

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 800199
User impact if declined: startup UI/performance problems (see comment 8)
Testing completed (on m-c, etc.): m-c a few days
Risk to taking this patch (and alternatives if risky): low risk - just removes broken code
String or UUID changes made by this patch: none

Comment 14

[Alex Keybl [:akeybl]]

Comment on attachment 708812 [details] [diff] [review]
Remove RedirectorRunnable

FF19 regression with a very low risk fix. Please land asap today.

I'm assuming that opening links from outside Firefox would be the best way to verify. Any other direction for QA would be helpful.

Comment 15

[Brian Nicholson (:bnicholson)]

(In reply to Alex Keybl [:akeybl] from comment #14)
> Comment on attachment 708812 [details] [diff] [review]
> Remove RedirectorRunnable
> 
> FF19 regression with a very low risk fix. Please land asap today.
> 
> I'm assuming that opening links from outside Firefox would be the best way
> to verify. Any other direction for QA would be helpful.

The best way to test this is to load links from Twitter (t.co links) to open Fennec from a cold state.

Comment 16

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/43710cb56191

Comment 17

[Brian Nicholson (:bnicholson)]

https://hg.mozilla.org/releases/mozilla-beta/rev/f4abd800a550

Comment 18

[Aaron Train [:aaronmt]]

On cold-start (Fennec not running), opening a URL via an intent, this is fixed. When Fennec is already running and I open a URL via an intent, I continue to see 'about:blank' flash in the title bar. Testing on 02/05 Nightly.

Comment 19

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Aaron Train [:aaronmt] from comment #18)
> On cold-start (Fennec not running), opening a URL via an intent, this is
> fixed. When Fennec is already running and I open a URL via an intent, I
> continue to see 'about:blank' flash in the title bar. Testing on 02/05
> Nightly.

Let's open a new bug for that case.

Comment 20

[Aaron Train [:aaronmt]]

bug 838330

Comment 21

[Paul Feher]

Verified using Asus Transformer TF101 (Android 4.0.3) on:
Firefox 19.0b2 build 2 (2013-02-05), Nightly 21.0a1(2013-02-07) and Aurora 20.0a2(2013-02-07).
The behaviour is the same as Aaron describe it in comment 18.