1278133 - Private tabs forget current page upon detach e10s

Status: RESOLVED DUPLICATE of bug 1278664

(Firefox :: Private Browsing, defect, P1)

Description

[Bram Speeckaert]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20160605030215

Steps to reproduce:

0) Make sure e10s is enabled.
1) Open a private window and open at least two tabs. 
2) Load a website in on of the tabs (e.g. https://html5zombo.com/).
3) Detach the website tab to spawn a new window.

Strangely this does not seem to happen with any of the about: pages (e.g. about:robots).


Actual results:

The new window does not retain the loaded page and instead shows a blank page.


Expected results:

The new window should retain the previously loaded page.

Comment 1

[Alice0775 White]

[Tracking Requested - why for this release]:

Regression window:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=16b4946069a9470a94f33791de7440966844747d&tochange=ff298d2993a32aeaca0eacac32d8e673298cf5c5

Regressed by: ff298d2993a3	James Andreou — Bug 1269361 - Add mPrivateBrowsingId to OriginAttributes r=ehsan,jdm

Comment 2

[James Andreou]

Investigating!

Comment 3

[James Andreou]

Attachment: Bug1278133.patch

Comment 4

[Andrea Marchesini [:baku]]

Comment on attachment 8760838 [details] [diff] [review]
Bug1278133.patch

Review of attachment 8760838 [details] [diff] [review]:
-----------------------------------------------------------------

This makes sense to me, but smaug (or another docShell peer) should review it.

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8760838 [details] [diff] [review]
Bug1278133.patch

I don't understand this. Why would the chrome docshell (mOwnerContent->OwnerDoc()->GetDocShell()) have private browsing flag set in its origin attributes. That looks like a bug to me if that is the case.

Comment 6

[(no longer active)]

This is an e10s regression, should definitely be tracked.

Comment 7

[James Andreou]

Smaug is correct that currently chrome docshells have a private browsing origin attribute. I created Bug 1278664 to address this issue.

Comment 8

[James Andreou]

Attachment: Bug1278133.patch

Using the privatebrowsingflag in docshell will still work after removing privateBrowsingId from chrome docshells.

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8760891 [details] [diff] [review]
Bug1278133.patch


>+  nsresult rv = parentContext->GetUsePrivateBrowsing(&isPrivate);
>+  NS_ENSURE_SUCCESS(rv, rv);

nsILoadContext has a C++ helper for getting the flag, so you could just
bool isPrivate = parentContext->UsePrivateBrowsing();


So this is fine to fix this regression, but
why doesn't the ownercontent have mozprivatebrowsing attribute? Bug in tabbrowser.xml?

Comment 10

[Jim Mathies [:jimm]]

(In reply to :Ehsan Akhgari (busy, don't ask for review please) from comment #6)
> This is an e10s regression, should definitely be tracked.

This is a regression in 49, and should be tracked by that release through relman.

Comment 11

[James Andreou]

Attachment: Bug1278133.patch

https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c407ce5676639dbd942ec16f333a44a3cdb81b2

Comment 12

[Mike Conley (:mconley) (:⚙️)]

Isn't this more or less the same patch from bug 1278664, which just landed on central? If so, should we close this one out and clear the checkin-needed?

Comment 13

[James Andreou]

This patch was landed in bug 1278664.

Comment 14

[Mike Conley (:mconley) (:⚙️)]

Do we think this fix warrants uplift to aurora?

Comment 15

[(no longer active)]

(In reply to Mike Conley (:mconley) - (needinfo me!) from comment #14)
> Do we think this fix warrants uplift to aurora?

Yes.  James, can you please nominate?  Thanks!

Comment 16

[James Andreou]

Comment on attachment 8761240 [details] [diff] [review]
Bug1278133.patch

Approval Request Comment
[Feature/regressing bug #]: 1269361
[User impact if declined]:
Users would be unable to drag a tab into a new window while using private browsing and e10s.
[Describe test coverage new/current, TreeHerder]:
This was manually tested.
[Risks and why]:
Self contained bug, very low risk of causing more regressions.
[String/UUID change made/needed]:
None.

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Recent regression, tracking.

Comment 19

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8761240 [details] [diff] [review]
Bug1278133.patch

Important to have tabs work, let's uplift to aurora. 
We should verify the fix  in aurora once it lands.

Comment 20

[Carsten Book [:Tomcat]]

in bug 1278664 2 patches landed, which does need uplift, both ?

Comment 21

[James Andreou]

https://hg.mozilla.org/mozilla-central/rev/3fb62649e99a should fix the regression alone.

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4ce5bd90da474685cf582e46109cbde62b69bb2d

Comment 23

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

I reproduced this bug using Fx 49.0a1 build ID:20160605030215, on Windows 10 x64.

I can confirm the fix on Fx 49.0a2, build ID  20160701004039 on Windows 10 x64, Ubuntu 16.04 LTS and Mac OSX 10.9.5 .