Implement Page Load Abandonment Rate telemetry

VERIFIED FIXED in Firefox 54

Status

()

Core
General
VERIFIED FIXED
10 months ago
4 months ago

People

(Reporter: Harald, Assigned: mconley, NeedInfo)

Tracking

(Depends on: 1 bug, Blocks: 1 bug)

unspecified
mozilla54
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox54 fixed)

Details

(URL)

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

10 months ago
Indicates if a user had a bad loading experience, either slow, broken or otherwise in a state that got the user to retry or stopping for good.

This only account for page loads that are aborted by user input:

- Backward button
- Stop
- Refresh
- Tab close
(Reporter)

Updated

10 months ago
Summary: Implement Abandonment Rate → Implement Abandonment Rate telemetry
(Reporter)

Comment 1

10 months ago
r? for data collection process
Flags: needinfo?(rweiss)
(Reporter)

Comment 2

9 months ago
:mconley would this be best implemented by platform or firefox? Seems like docshell has all the information it needs.
Flags: needinfo?(mconley)
True - though, especially in the e10s case, if the page is causing the browser to be non-responsive, we might not know that the user attempted to abort a page load until it's too late.

In the e10s case, since the parent is theoretically more freed up and able to respond (even if the content process is completely hung), we can detect if the user attempts any of the things in comment 0.

So my recommendation would be for this to be implemented in the front end.
Flags: needinfo?(mconley)
Assignee: nobody → mconley
Assignee: mconley → dale
Load balancing and taking this back from daleharvey.
Assignee: dale → mconley
(Reporter)

Comment 5

8 months ago
We had plans to test this during win 64 release; is here a chance to get it into 53? I could escalate to jhildebrand to see if frontend team can help.
Flags: needinfo?(mconley)
(Reporter)

Updated

8 months ago
Summary: Implement Abandonment Rate telemetry → Implement Page Load Abandonment Rate telemetry
(In reply to Harald Kirschner :digitarald from comment #5)
> We had plans to test this during win 64 release; is here a chance to get it
> into 53? I could escalate to jhildebrand to see if frontend team can help.

I think 53 has a good shot. 52, both daleharvey and I got bogged down with stuff that definitely had to land, which is why this slipped.

I wasn't aware that this Telemetry work was tied into the Win64 stuff. What other initiatives does it support? Are there a Trello card(s) you can point me to?

(In reply to Harald Kirschner :digitarald from comment #0)
> 
> This only account for page loads that are aborted by user input:
> 
> - Backward button
> - Stop
> - Refresh
> - Tab close

I want to know more about the window of time we care about. As you're aware, a page can load many things, and the page can become visible and interactive before those loads complete. Do we want to record when a user aborts / browses away from _any_ page that still has in-progress network requests? Or is the window of time we care about different? (Perhaps once we've first painted some of the content?)
Flags: needinfo?(mconley) → needinfo?(hkirschner)
(Reporter)

Comment 7

7 months ago
> I wasn't aware that this Telemetry work was tied into the Win64 stuff.

This one of the few measures we have that can point to a negative user experience. Win64 is a great opportunity to test these measures in an experiment and to understand how they work as a proxy for performance.

> Do we want to record when a user aborts / browses away from _any_ page that still has in-progress network requests?

We should track when a user stops loading for any page where the browser still shows a loading indicator.
Flags: needinfo?(rweiss)
Flags: needinfo?(hkirschner)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment on attachment 8830826 [details]
Bug 1307689 - Add telemetry to detect when user abandons an in-progress page load. data-review=bsmedberg,

Hoping just for a data-review on this one from one of our Data Stewards.
Attachment #8830826 - Flags: review?(benjamin)
Comment on attachment 8830826 [details]
Bug 1307689 - Add telemetry to detect when user abandons an in-progress page load. data-review=bsmedberg,

Hey digitarald, I'm hoping you can look at how I've defined the probe in Histograms.json, to make sure it's behaving and laid out as you expect (and that it'll be easy to reason about once we start examining the data).
Attachment #8830826 - Flags: feedback?(hkirschner)

Comment 15

6 months ago
mozreview-review
Comment on attachment 8830826 [details]
Bug 1307689 - Add telemetry to detect when user abandons an in-progress page load. data-review=bsmedberg,

https://reviewboard.mozilla.org/r/107536/#review109026

data-r=me with at least an improved description for the desktop/android question.

::: toolkit/components/telemetry/Histograms.json:10651
(Diff revision 3)
>      "description": "The time a given main thread runnable took to run (in milliseconds). The key comes from the runnables nsINamed::name value."
> +  },
> +  "BUSY_TAB_ABANDONED": {
> +    "alert_emails": ["hkirschner@mozilla.com"],
> +    "expires_in_version": "60",
> +    "kind": "enumerated",

This seems like it would be better as a categorical histogram now (better tooling and labeling).

::: toolkit/components/telemetry/Histograms.json:10652
(Diff revision 3)
> +  },
> +  "BUSY_TAB_ABANDONED": {
> +    "alert_emails": ["hkirschner@mozilla.com"],
> +    "expires_in_version": "60",
> +    "kind": "enumerated",
> +    "bug_numbers": [1307689],

Did you check whether Harald wants this opt-out?

::: toolkit/components/telemetry/Histograms.json:10654
(Diff revision 3)
> +    "alert_emails": ["hkirschner@mozilla.com"],
> +    "expires_in_version": "60",
> +    "kind": "enumerated",
> +    "bug_numbers": [1307689],
> +    "n_values": 15,
> +    "description": "Records a value each time a tab that is showing the loading throbber is interrupted. Causes are: 0=Stop, 1=Back, 2=Forward, 3=History navigation, 4=Refresh, 5=Tab closed, 6=New URL"

Please document whether this is desktop and android or just desktop.
Attachment #8830826 - Flags: review?(benjamin) → review+
Comment on attachment 8830826 [details]
Bug 1307689 - Add telemetry to detect when user abandons an in-progress page load. data-review=bsmedberg,

https://reviewboard.mozilla.org/r/107536/#review109026

> Did you check whether Harald wants this opt-out?

Good question - I'll needinfo him.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Hey digitarald, is this supposed to be an opt-out probe?
Flags: needinfo?(hkirschner)
And, to save round-trip time, in the event that the answer to comment 21's question is "Yes", does your data-review+ still stand, bsmedberg?
Flags: needinfo?(benjamin)
Yes.
Flags: needinfo?(benjamin)
(Reporter)

Comment 24

6 months ago
(In reply to Mike Conley (:mconley) from comment #21)
> Hey digitarald, is this supposed to be an opt-out probe?

Opt-in for now as most experiments that depend on this can be run on beta or the longitudinal set on release. We can re-consider this at a later point when we have new use cases.
Flags: needinfo?(hkirschner)
Attachment #8830826 - Flags: review?(dtownsend)
Attachment #8830939 - Flags: review?(dtownsend)

Comment 25

6 months ago
mozreview-review
Comment on attachment 8830939 [details]
Bug 1307689 - Regression tests.

https://reviewboard.mozilla.org/r/107606/#review109462

::: browser/base/content/test/tabs/browser_abandonment_telemetry.js:94
(Diff revision 4)
> +    },
> +  },
> +
> +  // Test going forward to a previous page
> +  {
> +    name: "Going forward to a previous page",

Nit: Going forward to the next page
Attachment #8830939 - Flags: review?(dtownsend) → review+

Comment 26

6 months ago
mozreview-review
Comment on attachment 8830826 [details]
Bug 1307689 - Add telemetry to detect when user abandons an in-progress page load. data-review=bsmedberg,

https://reviewboard.mozilla.org/r/107536/#review109458

Does this track cases where a user clicks on a link in a page that is still loading? I'm worried that will give us a lot of bad data.

::: browser/base/content/browser.js:1782
(Diff revision 5)
>    }
>    evt.stopPropagation();
>    evt.preventDefault();
>  }
>  
> +function maybeRecordAbandonmentTelemetry(type) {

This assumes that its the current tab that is changing in some way. That isn't necessarily true. You should pass the tab through to this function.

::: browser/base/content/browser.js:3267
(Diff revision 5)
>  
> +  // Do this after the above case where we might flip remoteness.
> +  // Unfortunately, we'll count the remoteness flip case as a
> +  // "newURL" load, since we're using loadURIWithFlags, but hopefully
> +  // that's rare enough to not matter.
> +  maybeRecordAbandonmentTelemetry("reload");

You could still do this and just clear the tab's busy attribute before calling loadURI. It's a bit of a hack though.

::: browser/base/content/tabbrowser.xml:2499
(Diff revision 5)
>                var animate = aParams.animate;
>                var byMouse = aParams.byMouse;
>                var skipPermitUnload = aParams.skipPermitUnload;
>              }
>  
> +            if (window.maybeRecordAbandonmentTelemetry) {

When would this not be true?
Attachment #8830826 - Flags: review?(dtownsend) → review-
Comment on attachment 8830826 [details]
Bug 1307689 - Add telemetry to detect when user abandons an in-progress page load. data-review=bsmedberg,

https://reviewboard.mozilla.org/r/107536/#review109458

Thankfully, no, this case is avoided.

> This assumes that its the current tab that is changing in some way. That isn't necessarily true. You should pass the tab through to this function.

Good idea.

> You could still do this and just clear the tab's busy attribute before calling loadURI. It's a bit of a hack though.

True. I think I'd rather not complicate things more than necessary here. The picture we're going to get isn't perfect, but I think it'll be reasonably high-resolution enough.

> When would this not be true?

Good point. Being overcautious - I'll remove the check.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 30

6 months ago
mozreview-review
Comment on attachment 8830826 [details]
Bug 1307689 - Add telemetry to detect when user abandons an in-progress page load. data-review=bsmedberg,

https://reviewboard.mozilla.org/r/107536/#review109714

::: browser/base/content/browser.js:866
(Diff revision 6)
>  }
>  
>  // A shared function used by both remote and non-remote browser XBL bindings to
>  // load a URI or redirect it to the correct process.
>  function _loadURIWithFlags(browser, uri, params) {
> +  maybeRecordAbandonmentTelemetry(gBrowser.selectedTab, "newURI");

You should be passing the tab for the browser here
Attachment #8830826 - Flags: review?(dtownsend) → review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Bah, autolanded this, but forgot to include - I got data-review+ from bsmedberg in comment 15.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 36

6 months ago
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5deef62563d2
Add telemetry to detect when user abandons an in-progress page load. data-review=bsmedberg, r=bsmedberg,mossop
https://hg.mozilla.org/integration/autoland/rev/452e602b9ee9
Regression tests. r=mossop
Backed out in https://hg.mozilla.org/integration/autoland/rev/f0a49d937170 for https://treeherder.mozilla.org/logviewer.html#?job_id=73466647&repo=autoland in a bunch of browser-chrome chunks.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 40

6 months ago
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/98654213e247
Add telemetry to detect when user abandons an in-progress page load. data-review=bsmedberg, r=bsmedberg,mossop
https://hg.mozilla.org/integration/autoland/rev/ae4a32972c94
Regression tests. r=mossop

Comment 41

6 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/98654213e247
https://hg.mozilla.org/mozilla-central/rev/ae4a32972c94
Status: NEW → RESOLVED
Last Resolved: 6 months ago
status-firefox54: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla54
I added some automated tests for this, but it'd be great if I could get some help testing corner cases.

Hey tracy, do you have some cycles to help me with this?
Flags: needinfo?(twalker)
BUSY_TAB_ABANDONED looks good from my testing.

bucket 0 = "stop" 
bucket 1 = "back"
bucket 2 = "forward"
bucket 3 = "historyNavigation"
bucket 4 = "reload" 
bucket 5 = "tabClosed"
bucket 6 = "newURI"

each of the above was correctly reported from the obvious related actions.  

historyNavigation being the only bucket a little unclear what is happening.  per chat with mconley in irc, this bucket represents using the dropdown history associated with the forward/back button when you choose and item more than one page away from the one you're abandoning. 

When using things like history from History menu or loading a bookmark to load over the slow loading page, that is is reported the newURI bucket.  mconley also confirmed that behavior is okay.

for "reload" you have to use the keyboard shortcut (command + R on macOS) as the stop button is displayed instead of the reload button when the page is loading.

note: the histogram shows a bucket 7.  but there is no action associated for bucket 7.  some telemetry oddness?
Status: RESOLVED → VERIFIED
Flags: needinfo?(twalker)
Thanks tracy.

digitarald - data from this probe has started to trickle in. Any surprises or modifications required? Or am I good to try to get this uplifted?
Flags: needinfo?(hkirschner)
(Reporter)

Comment 45

6 months ago
Looking at the data and trying to understand how it is actionable, we also need to know how many pages got successfully loaded. We would need to come up with a definition of what successful page of means, of course. This also means that the abandonment probe should be limited to the page loads (the busy indicator could also be triggered later in the session for endless scrolling/ads).

I will file a follow up bug to propose changes to this probe.
I assume FX_PAGE_LOAD_MS just looks for domloaded event, so we need a better reference. I would throw in
Flags: needinfo?(hkirschner)
Did the bug that you mentioned in comment 45 ever get filed?
Flags: needinfo?(hkirschner)
Depends on: 1348501
You need to log in before you can comment on or make changes to this bug.