Closed Bug 1233803 Opened 4 years ago Closed 4 years ago

Get rid of of sessionHistory CPOW sent up via WebNavigation:SetHistory message

Categories

(Firefox :: Session Restore, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 46
Tracking Status
e10s m8+ ---
firefox45 --- fixed
firefox46 --- fixed

People

(Reporter: mconley, Assigned: mconley)

References

(Blocks 1 open bug)

Details

Attachments

(7 files)

58 bytes, text/x-review-board-request
Felipe
: review+
Details
58 bytes, text/x-review-board-request
gkrizsanits
: review+
Details
58 bytes, text/x-review-board-request
Felipe
: review+
Details
1.92 KB, patch
Details | Diff | Splinter Review
58 bytes, text/x-review-board-request
gkrizsanits
: review+
Details
22.44 KB, patch
RyanVM
: feedback+
Details | Diff | Splinter Review
6.70 KB, application/zip
Details
We send a CPOW for sync access to the session history object. We shouldn't do that.
Assignee: nobody → mconley
Comment on attachment 8703799 [details]
MozReview Request: Bug 1233803 - Outlaw usage of sessionHistory CPOW in browser code. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29461/diff/1-2/
Attachment #8703799 - Flags: review?(felipc)
Comment on attachment 8703800 [details]
MozReview Request: Bug 1233803 - Add RemoteWebNavigation sessionHistory shim for addons. r?krizsa

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29463/diff/1-2/
Attachment #8703800 - Flags: review?(gkrizsanits)
Mike needs an interposition for webNavigation, problem is that in the e10s case we have RemoteWebNavigation implemented in js (http://mxr.mozilla.org/mozilla-central/source/toolkit/modules/RemoteWebNavigation.jsm#30)

We skip interposition for vanilla JS objects as an optimization: http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/wrappers/AddonWrapper.cpp#33

Should we 1) remove the optimization 2) add another case if the object has a QueryInterface method or do something else?
Flags: needinfo?(wmccloskey)
 It's worth to mention that the object we are trying to do interposition on, is special in a sense that the WrapperFactory already detects that it has to apply "double-wrapping" on it. So I'm refreshing my memories about how to do that and try to use the same machinery. That way we could keep the optimization and extend the condition to include JS implemented XPCOM objects too. That would be the cleanest and sanest approach I could come up with.
Bobby, I kind of assumed that for double wrapped objects the WN check here: http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/wrappers/AddonWrapper.cpp#34 would return true. But it does not seem to be the case. Does the UncheckedUnwrap call unwrap double wrapped objects completely? I would like to detect if the target is a JS implemented XPCOM object because I want to include these objects into the interposition. Do you know what is the best way to do that?
Flags: needinfo?(bobbyholley)
Note that I'm blocked on this M8 until I can figure out how to shim this thing, or unless I can get a compelling argument not to shim it (it'd need to be pretty compelling, since a quick mxr shows a number of add-ons accessing webNavigation.sessionHistory)
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #8)
> Bobby, I kind of assumed that for double wrapped objects the WN check here:
> http://mxr.mozilla.org/mozilla-central/source/js/xpconnect/wrappers/
> AddonWrapper.cpp#34 would return true.

I would expect IS_WN_CLASS to return true for a double-wrapped object, so !IS_WN_CLASS would be false, causing us not to take the early |return true| guarded on that conditional.

> But it does not seem to be the case.

Clarify?

> Does the UncheckedUnwrap call unwrap double wrapped objects completely?

It does not. UncheckedUnwrap unwraps JS wrappers. "Double-Wrapped Objects" should really be called "Double-Reflected Objects", and aren't related to the UncheckedUnwrap stuff.

> I
> would like to detect if the target is a JS implemented XPCOM object because
> I want to include these objects into the interposition. Do you know what is
> the best way to do that?

QI to nsIXPConnectWrappedJS. See UnwrapNativeCPOW for an example.
Flags: needinfo?(bobbyholley)
Does that give you the information necessary for me to shim the nsIWebNavigation implementation in RemoteWebNavigation?
Flags: needinfo?(gkrizsanits)
(In reply to Bobby Holley (busy) from comment #10)
> > But it does not seem to be the case.
> 
> Clarify?

I was afraid you are going to say this... then my memories weren't half as bad as I hoped. I will debug this then and see why we don't get here what we both expect. One option is that this jsm object is not getting double wrapped for some reason, but I have no idea why. Another reason might be that I made a mistake somewhere in the debugging, which I don't think is the case, but an option none the less.

(In reply to Mike Conley (:mconley) - Needinfo me! from comment #11)
> Does that give you the information necessary for me to shim the
> nsIWebNavigation implementation in RemoteWebNavigation?

I will look into this tomorrow, and will find a what's going on. Hopefully I can fix this on platform side once I figured out the reason for this behavior and then your shim will just work as it is. Removing the needinfo flag from Bill for now.
Flags: needinfo?(wmccloskey)
Flags: needinfo?(gkrizsanits)
I'm still stuck here. First I thought maybe RemoteWebNavigation does not get "double reflected" for some reason. So I pulled the whole thing through XPCOM factory and tried the same thing. It didn't help. I also tried with other JS implemented XPCOM objects (instantiated with Components.classes) to be on the safe side. But I experience the same behavior, namely that the clasp in AddonWrapper InterposePtoperty (it's the clasp of the unwrapped target object) seems to be a plain JS object:

(lldb) print *clasp
(const js::Class) $8 = {
  name = 0x0000000109b6f59d "Object"
  flags = 67108864
  addProperty = 0x0000000000000000
  delProperty = 0x0000000000000000
  getProperty = 0x0000000000000000
  setProperty = 0x0000000000000000
  enumerate = 0x0000000000000000
  resolve = 0x0000000000000000
  mayResolve = 0x0000000000000000
  finalize = 0x0000000000000000
  call = 0x0000000000000000
  hasInstance = 0x0000000000000000
  construct = 0x0000000000000000
  trace = 0x0000000000000000
  spec = {
    createConstructor_ = 0x0000000107ff3750 (XUL`CreateObjectConstructor(JSContext*, JSProtoKey) at Object.cpp:1033)
    createPrototype_ = 0x0000000107ff3860 (XUL`CreateObjectPrototype(JSContext*, JSProtoKey) at Object.cpp:1045)
    constructorFunctions_ = 0x000000010bb93390
    constructorProperties_ = 0x0000000000000000
    prototypeFunctions_ = 0x000000010bb936b0
    prototypeProperties_ = 0x000000010bb938e0
    finishInit_ = 0x0000000107ff3ad0 (XUL`FinishObjectClassInit(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSObject*>) at Object.cpp:1078)
    flags = 0
  }
  ext = {
    isWrappedNative = false
    weakmapKeyDelegateOp = 0x0000000000000000
    objectMovedOp = 0x0000000000000000
  }
  ops = {
    lookupProperty = 0x0000000000000000
    defineProperty = 0x0000000000000000
    hasProperty = 0x0000000000000000
    getProperty = 0x0000000000000000
    setProperty = 0x0000000000000000
    getOwnPropertyDescriptor = 0x0000000000000000
    deleteProperty = 0x0000000000000000
    watch = 0x0000000000000000
    unwatch = 0x0000000000000000
    getElements = 0x0000000000000000
    enumerate = 0x0000000000000000
    funToString = 0x0000000000000000
  }
}

I would expect it to be a WN class. And I have no clue why it isn't, maybe AddonWrappers do something tricky? (next thing I will look into). But it seems like AddonWrapper is not getting the double reflected version of the object. And this is concerning...

(In reply to Bobby Holley (busy) from comment #10)
> QI to nsIXPConnectWrappedJS. See UnwrapNativeCPOW for an example.

I was just wondering if I do the right thing, and to detect if a JSObject handle, is a double reflected object I should just check it's clasp. But it seems like your expectations and mines are the same and something else happening.

Setting needInfo flag again since there is something very fishy going on here.
Flags: needinfo?(bobbyholley)
Mike, sorry that this takes so long, I was not expecting complications like this... I will be on PTO tomorrow so I'm afraid this will take a bit longer :(
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #14)
> Mike, sorry that this takes so long, I was not expecting complications like
> this... I will be on PTO tomorrow so I'm afraid this will take a bit longer
> :(

That's alright - I can try to work with billm or bholley today to try to figure this out today. If not, we'll figure this out when you're back.
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #15)
> That's alright - I can try to work with billm or bholley today to try to
> figure this out today. If not, we'll figure this out when you're back.

Ok, I'll try to summarize the issue.

- We don't interpose on vanilla JS objects
- RemoteWebNavigation seems like a vanilla JS object (no double reflector)
- to apply double reflector on it I think we should use Cc to instantiate it
- by default WNs are not too flexible so usually wrappedJSObject is used on them and interposition on those does not seem to work (I attached a patch that modifies an interposition test to illustrate the issue if someone has the time to debug it)

question: how can we make interpose work on these kind of components?
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #16)
> - We don't interpose on vanilla JS objects
> - RemoteWebNavigation seems like a vanilla JS object (no double reflector)
> - to apply double reflector on it I think we should use Cc to instantiate it

This all sounds correct to me.

> - by default WNs are not too flexible so usually wrappedJSObject is used on
> them and interposition on those does not seem to work (I attached a patch
> that modifies an interposition test to illustrate the issue if someone has
> the time to debug it)

When you say "is used on them", do you mean just that "the component permits access to the JS implementation behind the XPCOM interface by setting wrappedJSObject = this", or that "the component does the above, _and_ the consumer pulls out the JS implementation by accessing wrappedJSObject before passing the thing to InterposeProperty"?

If it's the latter, the consumer needs to not do that in order for this to work right, because once we're dealing with the underlying JSObject, the machinery has no way of knowing that it was previously being used to represent an XPCOM interface. If it's the former, that would imply that some code around AddonWrapper is automatically extracting the JS-implementation if available, which sounds like a bug.

(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #13)
> I also tried with
> other JS implemented XPCOM objects (instantiated with Components.classes) to
> be on the safe side. But I experience the same behavior, namely that the
> clasp in AddonWrapper InterposePtoperty (it's the clasp of the unwrapped
> target object) seems to be a plain JS object:

If I understand your experiments right, sounds like the former. If so, we need to figure out who is extracting the JSImplementation before interposition and stop them from doing that.
Flags: needinfo?(bobbyholley)
One of the things that bholley has pointed out is that we're instantiating a RemoteWebNavigation by new'ing the implementation directly from the RemoteWebNavigation JSM, as opposed to going through XPCOM and createInstance-ing it. By doing this, we seem to side-step our interposition layer.

He suspects that if we register RemoteWebNavigation as a traditional XPCOM component and use createInstance instead, we might get the interposition stuff for free. I'm going to whip something up and see if that will work. Wish me luck!
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #18)
> One of the things that bholley has pointed out is that we're instantiating a
> RemoteWebNavigation by new'ing the implementation directly from the
> RemoteWebNavigation JSM, as opposed to going through XPCOM and
> createInstance-ing it. By doing this, we seem to side-step our interposition
> layer.

Specifically, you're not creating a double-wrapped XPCOM component, and so the thing appears as a vanilla JS object to the interposition code.
Comment on attachment 8705153 [details]
MozReview Request: Bug 1233803 - Register RemoteWebNavigation as a standard js-implemented XPCOM component. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29885/diff/1-2/
Attachment #8705153 - Attachment description: MozReview Request: Experimental shim stuff → MozReview Request: Bug 1233803 - Register RemoteWebNavigation as a standard js-implemented XPCOM component. r?felipe
Attachment #8705153 - Flags: review?(felipc)
Comment on attachment 8703799 [details]
MozReview Request: Bug 1233803 - Outlaw usage of sessionHistory CPOW in browser code. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29461/diff/2-3/
Attachment #8703799 - Flags: review?(felipc)
Comment on attachment 8703800 [details]
MozReview Request: Bug 1233803 - Add RemoteWebNavigation sessionHistory shim for addons. r?krizsa

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29463/diff/2-3/
Attachment #8703800 - Flags: review?(gkrizsanits)
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #18)
> One of the things that bholley has pointed out is that we're instantiating a
> RemoteWebNavigation by new'ing the implementation directly from the
> RemoteWebNavigation JSM, as opposed to going through XPCOM and
> createInstance-ing it. By doing this, we seem to side-step our interposition
> layer.
> 
> He suspects that if we register RemoteWebNavigation as a traditional XPCOM
> component and use createInstance instead, we might get the interposition
> stuff for free. I'm going to whip something up and see if that will work.
> Wish me luck!

I've noticed that and pointed it out earlier too, in fact I had a patch that
does this but did not work out somehow, I wonder why. Anyway, the important
thing that we seem to do this patter all over the place, so I wanted to ask
what was the purpose for RemoteWebNavigation to all the dance of QI + wrappedJSObject
initially?
Attachment #8703800 - Flags: review?(gkrizsanits) → review+
Comment on attachment 8703800 [details]
MozReview Request: Bug 1233803 - Add RemoteWebNavigation sessionHistory shim for addons. r?krizsa

https://reviewboard.mozilla.org/r/29463/#review27489

r=me if any of my suggestions work for the target.wrappedJSObject line. If I misinterpreted that line or you want to do it differently, please let me know.

::: toolkit/components/addoncompat/RemoteAddonsParent.jsm:964
(Diff revision 3)
> +  let impl = target.wrappedJSObject;
> +  if (!impl) {
> +    return target.sessionHistory;
> +  }

Is this the case when we're in non e10s mode? The funny (I mean horrible) thing about wrappedJSObject that is used for these double reflected objects but also for xray wrappers (for completly different things). So this code might just waives the xray wrapper of a native WebNavigation object.

Can we do instead either: a) use instanceof (might not be easy because of scopes) b) check classID c) check if e10s is on or off.
(In reply to Gabor Krizsanits [:krizsa :gabor] from comment #23)
> (In reply to Mike Conley (:mconley) - Needinfo me! from comment #18)
> > One of the things that bholley has pointed out is that we're instantiating a
> > RemoteWebNavigation by new'ing the implementation directly from the
> > RemoteWebNavigation JSM, as opposed to going through XPCOM and
> > createInstance-ing it. By doing this, we seem to side-step our interposition
> > layer.
> > 
> > He suspects that if we register RemoteWebNavigation as a traditional XPCOM
> > component and use createInstance instead, we might get the interposition
> > stuff for free. I'm going to whip something up and see if that will work.
> > Wish me luck!
> 
> I've noticed that and pointed it out earlier too,

Sorry for not catching that. :(

> in fact I had a patch that
> does this but did not work out somehow, I wonder why. Anyway, the important
> thing that we seem to do this patter all over the place, so I wanted to ask
> what was the purpose for RemoteWebNavigation to all the dance of QI +
> wrappedJSObject
> initially?

Looking back through the blame, it looks like the thing that eventually became RemoteWebNavigation.jsm was originally an Object in the remote-browser XBL binding that got split out into a JSM for cleanliness. I don't think there was an explicit reason to just implement the interface in JS without registering the component.
Comment on attachment 8705153 [details]
MozReview Request: Bug 1233803 - Register RemoteWebNavigation as a standard js-implemented XPCOM component. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29885/diff/2-3/
Comment on attachment 8703799 [details]
MozReview Request: Bug 1233803 - Outlaw usage of sessionHistory CPOW in browser code. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29461/diff/3-4/
Attachment #8703799 - Flags: review?(felipc)
Attachment #8703800 - Attachment description: MozReview Request: Bug 1233803 - Add remote browser sessionHistory shim for addons. r?krizsa → MozReview Request: Bug 1233803 - Add RemoteWebNavigation sessionHistory shim for addons. r?krizsa
Comment on attachment 8703800 [details]
MozReview Request: Bug 1233803 - Add RemoteWebNavigation sessionHistory shim for addons. r?krizsa

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29463/diff/3-4/
https://reviewboard.mozilla.org/r/29463/#review27489

> Is this the case when we're in non e10s mode? The funny (I mean horrible) thing about wrappedJSObject that is used for these double reflected objects but also for xray wrappers (for completly different things). So this code might just waives the xray wrapper of a native WebNavigation object.
> 
> Can we do instead either: a) use instanceof (might not be easy because of scopes) b) check classID c) check if e10s is on or off.

I'll instanceof the target to nsIDocShell - if it resolves to true, I guess I know that we're looking at an in-process browser. Sound good?
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #30)
> I'll instanceof the target to nsIDocShell - if it resolves to true, I guess
> I know that we're looking at an in-process browser. Sound good?

Yes, that sounds perfect.
Comment on attachment 8705153 [details]
MozReview Request: Bug 1233803 - Register RemoteWebNavigation as a standard js-implemented XPCOM component. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29885/diff/3-4/
Comment on attachment 8703799 [details]
MozReview Request: Bug 1233803 - Outlaw usage of sessionHistory CPOW in browser code. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29461/diff/4-5/
Comment on attachment 8703800 [details]
MozReview Request: Bug 1233803 - Add RemoteWebNavigation sessionHistory shim for addons. r?krizsa

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29463/diff/4-5/
Comment on attachment 8707536 [details]
MozReview Request: Bug 1233803 - Add sessionHistory shim for gBrowser and remote browsers. r?krizsa

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30747/diff/1-2/
Comment on attachment 8705153 [details]
MozReview Request: Bug 1233803 - Register RemoteWebNavigation as a standard js-implemented XPCOM component. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29885/diff/4-5/
Comment on attachment 8703799 [details]
MozReview Request: Bug 1233803 - Outlaw usage of sessionHistory CPOW in browser code. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29461/diff/5-6/
Comment on attachment 8703800 [details]
MozReview Request: Bug 1233803 - Add RemoteWebNavigation sessionHistory shim for addons. r?krizsa

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29463/diff/5-6/
Comment on attachment 8707536 [details]
MozReview Request: Bug 1233803 - Add sessionHistory shim for gBrowser and remote browsers. r?krizsa

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30747/diff/2-3/
Comment on attachment 8707536 [details]
MozReview Request: Bug 1233803 - Add sessionHistory shim for gBrowser and remote browsers. r?krizsa

https://reviewboard.mozilla.org/r/30747/#review27655
Attachment #8707536 - Flags: review+
Comment on attachment 8705153 [details]
MozReview Request: Bug 1233803 - Register RemoteWebNavigation as a standard js-implemented XPCOM component. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29885/diff/5-6/
Comment on attachment 8703799 [details]
MozReview Request: Bug 1233803 - Outlaw usage of sessionHistory CPOW in browser code. r?felipe

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29461/diff/6-7/
Comment on attachment 8703800 [details]
MozReview Request: Bug 1233803 - Add RemoteWebNavigation sessionHistory shim for addons. r?krizsa

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/29463/diff/6-7/
Comment on attachment 8707536 [details]
MozReview Request: Bug 1233803 - Add sessionHistory shim for gBrowser and remote browsers. r?krizsa

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/30747/diff/3-4/
Hey gabor, I had to update that last shims patch to fix a permaorange, because I didn't account for the non-e10s case. Can I assume your r+ still stands?

Interdiff: https://reviewboard.mozilla.org/r/30747/diff/3-4/
Flags: needinfo?(gkrizsanits)
(In reply to Mike Conley (:mconley) - Needinfo me! from comment #45)
> Hey gabor, I had to update that last shims patch to fix a permaorange,
> because I didn't account for the non-e10s case. Can I assume your r+ still
> stands?
> 
> Interdiff: https://reviewboard.mozilla.org/r/30747/diff/3-4/

LGTM
Flags: needinfo?(gkrizsanits)
Comment on attachment 8703799 [details]
MozReview Request: Bug 1233803 - Outlaw usage of sessionHistory CPOW in browser code. r?felipe

https://reviewboard.mozilla.org/r/29461/#review28017
Attachment #8703799 - Flags: review?(felipc) → review+
Comment on attachment 8705153 [details]
MozReview Request: Bug 1233803 - Register RemoteWebNavigation as a standard js-implemented XPCOM component. r?felipe

https://reviewboard.mozilla.org/r/29885/#review28019

::: toolkit/content/widgets/browser.xml:1181
(Diff revision 6)
>                "_remoteWebNavigation",
> +              "_remoteWebNavigationImpl",

do you need to remove _remoteWebNavigation from here?
Attachment #8705153 - Flags: review?(felipc) → review+
https://reviewboard.mozilla.org/r/29885/#review28019

> do you need to remove _remoteWebNavigation from here?

Nope - we still assign that in the constructor for the remote-browser binding. Consumers of the nsIWebNavigation interface will use that, but the \_remoteWebNavigationImpl is useful for when we need to fiddle with the RemoteWebNavigation instance behind the scenes.
https://hg.mozilla.org/integration/fx-team/rev/3ba697fefce2e891796b3f3f7bb7142bc952a7e7
Bug 1233803 - Register RemoteWebNavigation as a standard js-implemented XPCOM component. r=felipe

https://hg.mozilla.org/integration/fx-team/rev/df24394939b1fd83240ee24f070fb3e99bffd285
Bug 1233803 - Outlaw usage of sessionHistory CPOW in browser code. r=felipe

https://hg.mozilla.org/integration/fx-team/rev/7172bb64336d335f7ff3a2a5ef9d1f4699d45a59
Bug 1233803 - Add RemoteWebNavigation sessionHistory shim for addons. r=krizsa

https://hg.mozilla.org/integration/fx-team/rev/5d48bcc8521af3239f1c197a3cb90897afcafb12
Bug 1233803 - Add sessionHistory shim for gBrowser and remote browsers. r=krizsa
Mike lets get this uplifted to 45
Flags: needinfo?(mconley)
Whiteboard: [e10s-45-uplift]
Comment on attachment 8703799 [details]
MozReview Request: Bug 1233803 - Outlaw usage of sessionHistory CPOW in browser code. r?felipe

Approval Request Comment
[Feature/regressing bug #]:

None.

[User impact if declined]:

Slightly more IPC traffic than necessary.

[Describe test coverage new/current, TreeHerder]:

There's plenty of test coverage for the sessionHistory property in the tree, and that'll make use of our shimming layer, so we know it works.

[Risks and why]: 

Very little.

[String/UUID change made/needed]:

None.
Flags: needinfo?(mconley)
Attachment #8703799 - Flags: approval-mozilla-aurora?
Comment on attachment 8703800 [details]
MozReview Request: Bug 1233803 - Add RemoteWebNavigation sessionHistory shim for addons. r?krizsa

Approval Request Comment
[Feature/regressing bug #]:

None.

[User impact if declined]:

Slightly more IPC traffic than necessary.

[Describe test coverage new/current, TreeHerder]:

There's plenty of test coverage for the sessionHistory property in the tree, and that'll make use of our shimming layer, so we know it works.

[Risks and why]: 

Very little.

[String/UUID change made/needed]:

None.
Attachment #8703800 - Flags: approval-mozilla-aurora?
Comment on attachment 8705153 [details]
MozReview Request: Bug 1233803 - Register RemoteWebNavigation as a standard js-implemented XPCOM component. r?felipe

Approval Request Comment
[Feature/regressing bug #]:

None.

[User impact if declined]:

Slightly more IPC traffic than necessary.

[Describe test coverage new/current, TreeHerder]:

There's plenty of test coverage for the sessionHistory property in the tree, and that'll make use of our shimming layer, so we know it works.

[Risks and why]: 

Very little.

[String/UUID change made/needed]:

None.
Attachment #8705153 - Flags: approval-mozilla-aurora?
Comment on attachment 8707536 [details]
MozReview Request: Bug 1233803 - Add sessionHistory shim for gBrowser and remote browsers. r?krizsa

Approval Request Comment
[Feature/regressing bug #]:

None.

[User impact if declined]:

Slightly more IPC traffic than necessary.

[Describe test coverage new/current, TreeHerder]:

There's plenty of test coverage for the sessionHistory property in the tree, and that'll make use of our shimming layer, so we know it works.

[Risks and why]: 

Very little.

[String/UUID change made/needed]:

None.
Attachment #8707536 - Flags: approval-mozilla-aurora?
This needs some Aurora rebasing love.
Flags: needinfo?(mconley)
Flags: needinfo?(mconley)
Comment on attachment 8707536 [details]
MozReview Request: Bug 1233803 - Add sessionHistory shim for gBrowser and remote browsers. r?krizsa

needed for the experiment.
Attachment #8707536 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8705153 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8703800 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #8703799 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [e10s-45-uplift]
[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.