1308084 - nsChannelClassifier::ShouldEnableTrackingProtection has a very frequently failing NS_ENSURE_SUCCESS

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect, P2)

Description

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The most frequently failing NS_ENSURE_SUCCESS starting up Firefox and restoring my session is:

WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004002: file /home/dbaron/builds/ssd/mozilla-central/mozilla/netwerk/base/nsChannelClassifier.cpp, function ShouldEnableTrackingProtection, line 81


Line 81 is:

    nsCOMPtr<nsIHttpChannelInternal> chan = do_QueryInterface(aChannel, &rv);
    NS_ENSURE_SUCCESS(rv, rv);


Presumably this fails whenever the channel is non-HTTP (e.g., file, about:, etc.?).  I didn't check this, though.

Perhaps this should be using a test that doesn't spam the console in this case, e.g.:

  if (NS_FAILED(rv)) {
    return rv;
  }

(or maybe even an NS_OK return value?).

Comment 1

[François Marier [:francois]]

Do you have a good way to reproduce this?

I tried (using my main profile):

1. starting Firefox and opening two tabs
2. killing Firefox
3. starting Firefox again and restoring the tabs

and I didn't see that warning anywhere on the terminal.

I could probably just remove the warning since we're not really looking at it, but I'm curious to see the kinds of URLs that are failing to return a channel. I would ideally like to make sure there's nothing we care about in there.

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Every time, the URL was about:blank.

My debugging patch was https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/028ae685363f/debug-1308084

I think the warnings were printed pretty early in session restoration -- it seemed like one warning per tab being restored, and long before we try to load the actual page into any of those tabs.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It may also be relevant that I have the pref privacy.trackingprotection.enabled set to true, and browser.startup.page set to 3.

Comment 4

[François Marier [:francois]]

Attachment: Bug 1308084 - Silence noisy warning in nsChannelClassifier::ShouldEnableTrackingProtection().

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

Comment 5

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8809712 [details]
Bug 1308084 - Silence noisy warning in nsChannelClassifier::ShouldEnableTrackingProtection().

https://reviewboard.mozilla.org/r/92256/#review93188

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3b851a13d929
Silence noisy warning in nsChannelClassifier::ShouldEnableTrackingProtection(). r=gcp

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3b851a13d929

Comment 8

[Sylvestre Ledru [:Sylvestre]]

I guess it will ride the train with 53.