1338548 - Uninitialized rv in PendingGlobalHistoryEntry::ApplyChanges(IHistory* aHistory)

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Andrew McCreight [:mccr8]]

I found this while looking at Coverity results:

  for (const URITitle& title : mTitles) {
    aHistory->SetURITitle(title.mURI, title.mTitle);
    NS_ENSURE_SUCCESS(rv, rv);
  }
  mTitles.Clear();

The rv check there looks completely bogus. SetURITitle doesn't return an rv, so maybe the check just needs to be removed? I don't know if there are sec implications, so I'm filing this hidden.

Comment 1

[Nika Layzell [:nika] (ni? for response)]

I highly doubt that this will have any sec implications. 

1. This code is never hit in firefox without enabling the super buggy prerendering pref, which is turned on on no channels currently.
2. This not early returning has almost no impact on the behavior of this function, as mTitles is almost always 1 long.

I'll write up a simple patch anyway. It's not worth it to uplift IMO.

Comment 2

[Andrew McCreight [:mccr8]]

(In reply to Michael Layzell [:mystor] from comment #1)
> I'll write up a simple patch anyway. It's not worth it to uplift IMO.

Thanks. I guess the only question is if it could be causing any of the intermittent failures blocking bug  1330332.

Comment 3

[Nika Layzell [:nika] (ni? for response)]

Attachment: Check the return value from aHistory->SetURITitle when applying pending global history entries

I think it is extremely unlikely that this is the cause of any of those intermittent failures. 

MozReview-Commit-ID: 6aPtNrqoqZj

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/e86672310690
Check the return value from aHistory->SetURITitle when applying pending global history entries, r=smaug

Comment 5

[Phil Ringnalda (:philor)]

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