767978 - Create an shistory object for top-level b2g <iframe mozbrowser>.

Status: RESOLVED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[Justin Lebar (not reading bugmail)]

Bug 757137 appears to have caused a gaia bug [1], in which apps don't have session histories.

bz suggested we could ensure that all <iframe mozbrowser>'s have a shistory.  That seems like the easiest thing to do here.

[1] https://github.com/mozilla-b2g/gaia/issues/1869#issuecomment-6520055

Comment 1

[Justin Lebar (not reading bugmail)]

Attachment: Patch v1

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 636308 [details] [diff] [review]
Patch v1

How could GetSessionHistory possibly return non-null here?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 636308 [details] [diff] [review]
Patch v1

Also, you need to fix the various places that assume that only same-type roots will have a non-null mSessionHistory (e.g. nsDocShell::RemoveFromSessionHistory, various consumers of GetRootSessionHistory, etc).

Comment 4

[Justin Lebar (not reading bugmail)]

(In reply to Boris Zbarsky (:bz) from comment #2)
> Comment on attachment 636308 [details] [diff] [review]
> Patch v1
> 
> How could GetSessionHistory possibly return non-null here?

> Also, you need to fix the various places that assume that only same-type roots will have 
> a non-null mSessionHistory (e.g. nsDocShell::RemoveFromSessionHistory, various consumers 
> of GetRootSessionHistory, etc).

Ah, I didn't realize that's what mSessionHistory was.  Dang.

Let's just fix this in the caller, then.  I've been trying to avoid hacking this docshell shistory stuff; I'm afraid of falling into a rathole.  (e.g. mozbrowser.canGoBack is only correct OOP, atm.)

Comment 5

[Justin Lebar (not reading bugmail)]

Attachment: Patch, v1

Comment 6

[Boris Zbarsky [:bzbarsky]]

So is the shell.js code creating a toplevel docshell?

Comment 7

[Justin Lebar (not reading bugmail)]

(In reply to Boris Zbarsky (:bz) from comment #6)
> So is the shell.js code creating a toplevel docshell?

The only thing above this <iframe> is a <xul:window windowtype='navigator:browser'>.  I'm not sure if that answers your question...

Comment 8

[Boris Zbarsky [:bzbarsky]]

Not quite.  Let's try again.

1)  Does this <iframe> have a typeContent docshell?
2)  If so, is its parent docshell, if any, typeChrome?

Comment 9

[Justin Lebar (not reading bugmail)]

(gdb) p mItemType
$1 = 1             # typeContent
(gdb) p mParent->mItemType
$2 = 0             # typeChrome

Comment 10

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 636370 [details] [diff] [review]
Patch, v1

r=me

Comment 11

[Justin Lebar (not reading bugmail)]

Thanks. https://hg.mozilla.org/integration/mozilla-inbound/rev/c94c8777d7eb

Comment 12

[Justin Lebar (not reading bugmail)]

For my information, what was the gotcha we were checking for here?

Comment 13

[Boris Zbarsky [:bzbarsky]]

Just the same "docshell looks for the session history on the root same-type docshell" bit.

Comment 14

[Justin Lebar (not reading bugmail)]

(In reply to Boris Zbarsky (:bz) from comment #13)
> Just the same "docshell looks for the session history on the root same-type
> docshell" bit.

Thanks.

Sicking realized that, if we don't fix docshell shistory for in-process apps (comment 4), in-process apps should simply not have shistory.  IOW, we should back this out once we're running OOP all the apps we intent to run OOP.  (The plan is to run everything except the browser app OOP, because if we want to run browser content OOP, we'd need nested content processes, bug 761935.)

Comment 15

[Ed Morley [:emorley]]

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