1319607 - uBlock Origin + a WebExtension's webRequest.onErrorOccurred listener = page load hanging

Status: VERIFIED FIXED

(WebExtensions :: Request Handling, defect)

Description

[u287251]

Attachment: webext-onerroroccurred.zip

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:53.0) Gecko/20100101 Firefox/53.0
Build ID: 20161122030216

Steps to reproduce:

This issue was first reported on my issue tracker[1]. Eventually, I narrowed the steps to reproduce as follow:

1. Launch Nightly.
2. Install uBlock Origin ("uBO")  (default settings will do).
3. Open a new tab, load with https://tweetdeck.twitter.com/
4. Observation: the page load fine, with two scripts blocked by uBO:
  - ton.twimg.com/tweetdeck-web/web/scripts/google_analytics.4a354c1e82.js
  - ssl.google-analytics.com/ga.js
5. Go to about:debugging
6. Load the very simple webextension provided as attachment [2].
7. Force a reload of the tab which URL is https://tweetdeck.twitter.com/
8 Observation: the page load hangs, with the spinner image never going away.

More observations:

The page load hang issue does NOT occur if...

- ... I remove either
    - chrome.webRequest.onBeforeSendHeaders.addListener; OR
    - chrome.webRequest.onErrorOccurred.addListener
  from the background.js file of the minimalist webextension.

- ... the "blocking" option is not used.

- ... the webextension is loaded *before* uBO is enabled, so it appears the order of the HTTP observers is key (uBO registers its own HTTP observer).

- ... the webextension version of uBO is used.

- ... if only the script ton.twimg.com/tweetdeck-web/web/scripts/google_analytics.4a354c1e82.js is NOT blocked by uBO.

Notes:

The issue also occurred when using chrome.webRequest.onBeforeRequest instead of chrome.webRequest.onBeforeSendHeaders.

***

[1] https://github.com/gorhill/uBlock/issues/2173

[2] The ZIP file consist of two small files, a valid webext extension:

manifest.json:

    {
        "name": "Bugzilla",
        "version": "1.0",
        "permissions": ["webRequest", "webRequestBlocking"],
        "background": {
            "scripts": ["background.js"]
        },
        "applications": {
            "gecko": {
                "id": "pageloadhang@webextensions"
            }
        },
        "manifest_version": 2,
        "description": "Bugzilla"
    }

background.js:

    chrome.webRequest.onBeforeSendHeaders.addListener(
        function() {},
        {urls:["<all_urls>"]},
        ["blocking"]
    );
    chrome.webRequest.onErrorOccurred.addListener(
        function() {},
        { urls:["<all_urls>"]}
    );

Comment 1

[u287251]

Attachment: bugzilla1319607-bootstrapped.zip

Comment 2

[u287251]

Attachment: bugzilla1319607-webextension.zip

Comment 3

[u287251]

I have create a minimalist bootstrapped extension, to use instead of using uBlock Origin.

The minimalist bootstrapped extension has only ONE purpose: to cancel a network request for the resource which URL is https://ton.twimg.com/tweetdeck-web/web/scripts/google_analytics.4a354c1e82.js.

I amended the minimalist webextension to change its name to make clear its related to the issue here.

Once both minimalist extensions are loaded through about:debugging, it's just a matter of clicking the "Reload" button of one or the other extension to make the issue appear/disappear:

- test case: https://tweetdeck.twitter.com/
- if the bootstrapped extension is loaded after the webextension one = page does not hang.
- if the webextension extension is loaded after the bootstrapped one = page does hang.

Comment 4

[u287251]

For the record, this also occurs with Ghostery 7.0.14[1]. Looking at the source code, I confirm this is related to the issue here, Ghostery is a webextension, and registers a webRequest.onErrorOccurred listener.

[1] https://addons.mozilla.org/en-US/firefox/addon/ghostery/

Comment 5

[Kris Maglione [:kmag]]

Your bootstrapped add-on breaks web requests on its own, since it registers a channel event sink and doesn't provide an asyncOnChannelRedirect method. After fixing that, though, I see the problem.

Comment 6

[Kris Maglione [:kmag]]

Attachment: Bug 1319607: Make request resumption more resilient.

Review commit: https://reviewboard.mozilla.org/r/95476/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/95476/

Comment 7

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8814242 [details]
Bug 1319607: Make request resumption more resilient.

https://reviewboard.mozilla.org/r/95476/#review95746

Comment 8

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5b45f7938182ea8bf814ab0bb6373bcf803755b5
Bug 1319607: Make request resumption more resilient. r=mixedpuppy

Comment 9

[Kris Maglione [:kmag]]

Comment on attachment 8814242 [details]
Bug 1319607: Make request resumption more resilient.

Approval Request Comment
[Feature/regressing bug #]: Bug 1254204
[User impact if declined]: This bug causes some requests to never fully complete, and therefore some pages to never fully load, when certain request blocking extensions are installed alongside each other.
[Describe test coverage new/current, TreeHerder]: This feature has extensive existing tests, and new tests have been added for this issue.
[Risks and why]: Low. This is a minimal change to make sure that cleanup code is called in more corner cases.
[String/UUID change made/needed]: None.

Comment 10

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/5b45f7938182

Comment 11

[Nazar Mokrynskyi]

Latest Nightly 53.0a1 (2016-11-26) fixed this issue, thanks!

Comment 12

[Julien Cristau [:jcristau] (back August 18)]

Comment on attachment 8814242 [details]
Bug 1319607: Make request resumption more resilient.

request resumption fix, take in aurora52

note that "Only resume the channel if either it was suspended by this call." could probably lose the "either", since the "or" part was removed :)

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/9dd3c365bc5d

Comment 14

[Kris Maglione [:kmag]]

(In reply to Julien Cristau [:jcristau] from comment #12)
> note that "Only resume the channel if either it was suspended by this call."
> could probably lose the "either", since the "or" part was removed :)

Ah. Quite. Good catch.

Comment 15

[Kris Maglione [:kmag]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ea74e14fae4134d3f3c1a0098f9bda5695a7ada6
Bug 1319607: Follow-up: Fix typo.

Comment 16

[Kris Maglione [:kmag]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bb32677bad4c

Comment 17

[Carsten Book [:Tomcat]]

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

Comment 18

[Vasilica Tamas, QA [:vtamas]]

I was able to reproduce this issue on Firefox 53.0a1 (2016-11-22) under Windows 10 64-bit.

Verified fixed on Firefox 53.0a1 (2016-11-29) and Firefox 52.0a2 (2016-11-29) under Windows 10 64-bit and Ubuntu 16.04 32-bit. The page is instantly loaded.