Closed Bug 1256468 Opened 4 years ago Closed 4 years ago

[e10s] Add-on ReloadEvery doesn't refresh (unsafe CPOW usage in RemoteAddonsParent.jsm and webNavigation.sessionHistory is undefined)

Categories

(Firefox :: Extension Compatibility, defect)

48 Branch
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 48
Tracking Status
firefox46 --- fixed
firefox47 --- fixed
firefox48 --- fixed

People

(Reporter: epinal99-bugzilla2, Assigned: mconley)

References

()

Details

(Keywords: addon-compat)

Attachments

(1 file)

No description provided.
Mike, I see this message sent in the browser console with FF48 and e10s enabled when using ReloadEvery which fails to refresh the page.

Is it normal according to your patch in bug 1233803?
Blocks: e10s-addons
Flags: needinfo?(mconley)
Keywords: addon-compat
No, it's not normal. This is due to the fact that our add-on shims aren't properly QI'ing the docShell to an nsIWebNavigation before attempting to return the sessionHistory, which means that unless something else has QI'd that docShell to an nsIWebNavigation before, the sessionHistory will be undefined.

TL;DR, this is a shims bug, and I'll try to fix it ASAP.
Assignee: nobody → mconley
Flags: needinfo?(mconley)
Comment on attachment 8730959 [details]
MozReview Request: Bug 1256468 - sessionHistory shim should QI DocShell to nsIWebNavigation automatically. r?gkrizsanits

https://reviewboard.mozilla.org/r/40271/#review36909

Thanks Mike. About that unsafe CPOW message, is that gone too now?
Attachment #8730959 - Flags: review?(gkrizsanits) → review+
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #4)
> Comment on attachment 8730959 [details]
> MozReview Request: Bug 1256468 - sessionHistory shim should QI DocShell to
> nsIWebNavigation automatically. r?gkrizsanits
> 
> https://reviewboard.mozilla.org/r/40271/#review36909
> 
> Thanks Mike. About that unsafe CPOW message, is that gone too now?

I believe that'll stick around, unfortunately.
https://hg.mozilla.org/mozilla-central/rev/44b39229cd33
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment on attachment 8730959 [details]
MozReview Request: Bug 1256468 - sessionHistory shim should QI DocShell to nsIWebNavigation automatically. r?gkrizsanits

Approval Request Comment
[Feature/regressing bug #]:

e10s

[User impact if declined]:

Users with add-ons that attempt to access sessionHistory directly on WebNavigation's in the parent process (or through any of the alises, like through gBrowser or browser.sessionHistory) will sometimes fail in e10s mode.

I say "sometimes" because sometimes the docShell in the content process has been QI'd to an nsIWebNavigation, so in some cases, this will work. In other cases the QI has not occurred, so in the shim, we do the QI to be sure.

[Describe test coverage new/current, TreeHerder]:

Added an automated test.

[Risks and why]: 

Very low - it's only within the shimming code, so this shouldn't affect non-e10s at all.

[String/UUID change made/needed]:

None.
Attachment #8730959 - Flags: approval-mozilla-beta?
Attachment #8730959 - Flags: approval-mozilla-aurora?
Loic, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!
Flags: needinfo?(epinal99-bugzilla2)
Comment on attachment 8730959 [details]
MozReview Request: Bug 1256468 - sessionHistory shim should QI DocShell to nsIWebNavigation automatically. r?gkrizsanits

Addon-compat related fix, Aurora47+, Beta46+
Attachment #8730959 - Flags: approval-mozilla-beta?
Attachment #8730959 - Flags: approval-mozilla-beta+
Attachment #8730959 - Flags: approval-mozilla-aurora?
Attachment #8730959 - Flags: approval-mozilla-aurora+
(In reply to Ritu Kothari (:ritu) from comment #9)
> Loic, could you please verify this issue is fixed as expected on a latest
> Nightly build? Thanks!

With the latest Nightly (20160321030217), ReloadEvery reloads the pages and I don't see anymore this error in the browser console, even if I see always various "Unsafe CPOW usage", but I guess they are a different issue.
Flags: needinfo?(epinal99-bugzilla2)
(In reply to Loic from comment #11)
> (In reply to Ritu Kothari (:ritu) from comment #9)
> > Loic, could you please verify this issue is fixed as expected on a latest
> > Nightly build? Thanks!
> 
> With the latest Nightly (20160321030217), ReloadEvery reloads the pages and
> I don't see anymore this error in the browser console, even if I see always
> various "Unsafe CPOW usage", but I guess they are a different issue.

Thanks for a prompt verification! Please file a bug on the other issue if there isn't one filed already.
Status: RESOLVED → VERIFIED
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected
You need to log in before you can comment on or make changes to this bug.