missing nsIRequest in nsAboutCacheEntry::Channel QI

RESOLVED FIXED in Firefox 55

Status

()

RESOLVED FIXED
a year ago
a year ago

People

(Reporter: schien, Assigned: schien)

Tracking

unspecified
mozilla56
Points:
---
Bug Flags:
qe-verify -

Firefox Tracking Flags

(firefox55 fixed, firefox56 fixed)

Details

(Whiteboard: [necko-active])

Attachments

(1 attachment)

hit assertion while running netwerk/test/browser/browser_about_cache.js in debug build
>ASSERTION: QueryInterface needed: 'query_result.get() == mRawPtr'
Comment hidden (mozreview-request)
Whiteboard: [necko-active]

Comment 2

a year ago
mozreview-review
Comment on attachment 8884225 [details]
Bug 1379095 - add nsIRequest in nsAboutCacheEntry::Channel QI.

https://reviewboard.mozilla.org/r/155174/#review160242

Uuups!!  Thanks!
Attachment #8884225 - Flags: review?(honzab.moz) → review+

Comment 3

a year ago
Pushed by schien@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8e0c107d042f
add nsIRequest in nsAboutCacheEntry::Channel QI. r=mayhemer

Comment 4

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/8e0c107d042f
Status: NEW → RESOLVED
Last Resolved: a year ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
@mayhemer do you think this patch worth uplifting to Firefox 55 (which is beta now)?
Flags: needinfo?(honzab.moz)
(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #5)
> @mayhemer do you think this patch worth uplifting to Firefox 55 (which is
> beta now)?

Yep.
Flags: needinfo?(honzab.moz)
Comment on attachment 8884225 [details]
Bug 1379095 - add nsIRequest in nsAboutCacheEntry::Channel QI.

Approval Request Comment
[Feature/Bug causing the regression]: bug 1266196
[User impact if declined]: no, fix test case error
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: one line fix for QueryInterface
[String changes made/needed]: no
Attachment #8884225 - Flags: approval-mozilla-beta?
(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #7)
> Comment on attachment 8884225 [details]
> Bug 1379095 - add nsIRequest in nsAboutCacheEntry::Channel QI.
> 
> Approval Request Comment
> [Feature/Bug causing the regression]: bug 1266196
> [User impact if declined]: no, fix test case error

So this is an old bug with no user impact?  What makes it worth uplifting?
Flags: needinfo?(schien)
(In reply to Julien Cristau [:jcristau] from comment #8)
> (In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment
> #7)
> > Comment on attachment 8884225 [details]
> > Bug 1379095 - add nsIRequest in nsAboutCacheEntry::Channel QI.
> > 
> > Approval Request Comment
> > [Feature/Bug causing the regression]: bug 1266196
> > [User impact if declined]: no, fix test case error
> 
> So this is an old bug with no user impact?  What makes it worth uplifting?

To reduce the noise in test automation.
Flags: needinfo?(schien)
Comment on attachment 8884225 [details]
Bug 1379095 - add nsIRequest in nsAboutCacheEntry::Channel QI.

fix nsAboutCacheEntry::Channel queryinterface, beta55+
Attachment #8884225 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment 11

a year ago
bugherderuplift
https://hg.mozilla.org/releases/mozilla-beta/rev/a36197d667da
status-firefox55: --- → fixed
(In reply to Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz) from comment #7)
> [Is this code covered by automated tests?]: yes
> [Has the fix been verified in Nightly?]: yes
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Shih-Chiang Chien's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.