[1.Description]: [Aries KK v2.5][Flame KK v2.5][Browser]Open a private page which need login first from normal window, the target private page will be displayed first before loading the login page. Found at:17:03 See attchment:logcat2.txt, Aries3.3gp. [2.Testing Steps]: 1. Log in social network (such as facebook) via normal browser 2. Long press any hyperlink(such as friend profile) which only can be seen if you are login and select to open in private mode [3.Expected Result]: step2. The target private window should only show after user login successfully. [4.Actual Result]: step2. The target page will be displayed first before loading the login page. [5.Reproduction build]: Device: Aries KK 2.5(Affected) Build ID 20150803195455 Gaia Revision dbacf8364f4505d021b7d8fb9cabea325004dbcc Gaia Date 2015-08-03 16:38:49 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/abc56d57f6e1 Gecko Version 42.0a1 Device Name aries Firmware(Release) 4.4.2 Firmware(Incremental) eng.worker.20150803.191800 Firmware Date Mon Aug 3 19:18:08 UTC 2015 Bootloader s1 Device: Flame KK 2.2(Unaffected) Build ID 20150805032505 Gaia Revision f8b119ac30e97df991c97682ac4d4f9ca22e1793 Gaia Date 2015-07-31 13:20:55 Gecko Revision https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/0c7a85251e10 Gecko Version 37.0 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150805.065842 Firmware Date Wed Aug 5 06:58:54 EDT 2015 Firmware Version v18D v4 Bootloader L1TC000118D0 Device:Flame KK 2.5(Affected) Build ID 20150805150207 Gaia Revision 581de383687dc441a878d2c91a0167c6ec688fef Gaia Date 2015-08-05 01:48:40 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/b12a261ee32e Gecko Version 42.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150805.195702 Firmware Date Wed Aug 5 19:57:12 EDT 2015 Firmware Version v18D v4 Bootloader L1TC000118D0 [6.Reproduction Frequency]: Always Recurrence,5/5 [7.TCID]: 16498

The oldest build in b2g-inbound / mozilla-inbound is 20150609153752 / 20150609172153, which is later than the first broken build & last working build, so I try to do the regression in Nightly builds. Nightly Regression Window: Build ID 20150311010231 Gaia Revision 943c8b4039f59b08ba100390e164a076a20c892e Gaia Date 2015-03-10 20:35:07 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/fd8e079d6335 Gecko Version 39.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150311.043838 Firmware Date Wed Mar 11 04:38:50 EDT 2015 Bootloader L1TC000118D0 First Broken Environmental Variables: Build ID 20150311170314 Gaia Revision 2b87ee8e7e2ec30a9851b6b59a899006a98767ab Gaia Date 2015-03-11 02:14:27 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/c33922ee3ac3 Gecko Version 39.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20150311.203536 Firmware Date Wed Mar 11 20:35:47 EDT 2015 Bootloader L1TC000118D0 First Broken Gaia & Last Working Gecko - issue DOES repro Gaia Revision 2b87ee8e7e2ec30a9851b6b59a899006a98767ab Gecko Revision https://hg.mozilla.org/mozilla-central/rev/fd8e079d6335 First Broken Gecko & Last Working Gaia - issue DOES NOT repro Gecko Revision https://hg.mozilla.org/mozilla-central/rev/c33922ee3ac3 Gaia Revision 943c8b4039f59b08ba100390e164a076a20c892e Gaia pushlog: https://github.com/mozilla-b2g/gaia/compare/943c8b4039f59b08ba100390e164a076a20c892e...2b87ee8e7e2ec30a9851b6b59a899006a98767ab

[Blocking Requested - why for this release]: Regression issue

Nothing really sticks out in the regression range. Michael, can you double check?

(In reply to Gregor Wagner [:gwagner] from comment #4) > Nothing really sticks out in the regression range. Michael, can you double > check? Don't block on me, whoever picks this bug up should do the investigation.

We show the content and login on dekstop as well. Not sure if we should still block on this issue.

(In reply to Gregor Wagner [:gwagner] from comment #6) > We show the content and login on dekstop as well. Not sure if we should > still block on this issue. And even less block in the Gaia:browser component :) Getting a valuable regression range might be tricky since this looks caching-related. And FWIW I can't reproduce it with twitter (the private tab has me unlogged from the get-go)

Something is leaking through, because even after the page loads, I see the "logged-in" sidebar with my account info. (and there's no userID or anything in the URL.)

Stepping through, I can confirm that we set mozprivatebrowsing="true" on the incoming iframe before setting the frame src. I don't see anything wrong on the Gaia side. I observe the following behavior: - It is only reproducible on the first load of a given URL; subsequent attempts for the same URL correctly display the "You must log in first" page immediately. Other URLs (other profile pages) similarly fail once, and succeed on subsequent attempts. - When reproducing, the "wrong" page is shown fully as it loads (i.e. the person's profile page); when the load completes, the page mostly reverts back into a login page -- however, I am still able to see the sidebar that shows my account details. Trying to actually visit any other page results in (correctly) the login page. See a video at <https://youtu.be/3N-GyRpehDE>. - I grabbed a HTTP log capture using [1], and it appears that when requesting the user's profile page (ostensibly in a private window), we're still sending up a bunch of cookies. (I can send these, and the log, privately if needed.) [1] https://developer.mozilla.org/en-US/docs/Mozilla/Debugging/HTTP_logging Based on this, I think we should pass this to the "Private Browsing" component, and potentially unblock (although this issue is somewhat concerning in general). Gregor, what do you think?

Ehsan, any idea whats going on here? Seems like there is not much we can do from the gaia side.

I'm not sure if I even understand the bug description. It seems like this is complaining about some part of the page loading appearing to happen in normal mode, but it's very hard to tell since the reporter is using Facebook where some people have public versions of their profile and it's not clear to me what the reporter expected to see. Also Facebook is a terrible place to test this in general, it should be easy to write a couple of small test pages, one setting a cookie and the other changing its background color based on the cookie. At any rate, we don't have this bug in Firefox. If I'm understanding the report correctly, this is very bad, and it's the kind of thing that can be caused either by a rendering bug (painting the content from the previous page at the bottom) or by a race condition which causes us to set the mozprivatebrowsing attribute after the loading has begun (in which case you should be getting an error reported to the console such as "We should not switch to Private Browsing after loading a document.") I'm not familiar with the b2g side of things, I think either Fabrice or KanRu may be able to help.

Additional testing reveals the following: - This bug only happens when the frame is OOP (remote="true"). - I no longer think this is caching-related; reloading the original page before "open in a new private window" makes this 100% reproducible. - We're definitely sending cookies up to Facebook from a mozprivatebrowsing="true" frame. I've verified this by clearing cookies and observing IFRAME creation and HTTP logs. - It does not reproduce when simply copy-pasting the URL into an empty private window, which seems strange to me because the code path for creating that IFRAME is the same as the code path triggered from the context menu. - I have attempted to isolate the problem in a test case as suggested by Ehsan, but have been unsuccessful in reproducing the problem. Simple cookie tests work as expected. I realize that a test case would be ideal, but I haven't been able to isolate it any further. Since this only occurs on OOP frames, it seems very much like a platform race; unfortunately, I don't have the expertise to dig deeper without help.

I can't immediately find any code that would make @mozprivatebrowsing work with out of process frames. Fabrice, you implemented this feature IIRC. Is this supposed to work with OOP frames?

We changed the way we set the docshell state in http://hg.mozilla.org/mozilla-central/rev/6e90fd9bf6e2 but this should still work with OOP iframes. Marcus, do you get a console message like "We should not switch to Private Browsing after loading a document." ?

No, but I do see: "Only internal code is allowed to set the usePrivateBrowsing attribute" {file: "about:blank" line: 0}

That comes from http://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#2204 I'm not sure why we emit that unconditionally. Ehsan, do you know?

It depends on where the message actually comes from; it may or may not be OK. Marcus, please set a breakpoint on nsDocShell::SetUsePrivateBrowsing() and get a backtrace so that we can tell what is happening. That being said, it seems like the code for setting the private flag on the docshell has changed tremendously and I haven't reviewed any of the changes so I can't really say how it works any more, or whether it's correct. :( Perhaps Josh and/or Olli know more.

(In reply to Gregor Wagner [:gwagner] from comment #19) > Created attachment 8680635 [details] > gdb bt

Whoa! If I understand what this code is doing, this is completely broken. It seems to me that we are setting the privacy status on the docshell when we're "showing the frame" which, if I understand the meaning of RecvShow() correctly) is way after we have started to load the page. This is completely unsupported, and the symptoms in comment 0 may be caused by this. This problem will probably manifest itself in different ways in different websites. The docshell's privacy state needs to be set soon after it is created, before any content is loaded. Ideally we should make it work here: <https://dxr.mozilla.org/mozilla-central/rev/1e700005a0ddf2b17803213e1f3f8d78a7a618b8/docshell/base/nsDocShell.cpp#3297>. Also, private browsing is enabled in 2.5, right? If yes, we need to disable it immediately, since it's probably completely broken, and we won't have time to fix it.

Ehsan, why is this not enough to at least know if we started to load the page: http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp#1586 ? Marcus didn't get any console message... I will double check when TabChild::RecvLoadURL() is called but I'm pretty sure this was after RecvShow() when I landed this patch.

I do recall too that at least originally we did set private browsing mode early enough.

(and doesn't sounds like something which couldn't be fixed for 2.5)

(In reply to [:fabrice] Fabrice Desré from comment #22) > Ehsan, why is this not enough to at least know if we started to load the > page: > http://mxr.mozilla.org/mozilla-central/source/dom/ipc/TabChild.cpp#1586 ? > Marcus didn't get any console message... It _should_ be sufficient, at least assuming that we are checking the correct docshell. I can't really say for sure without debugging... :/ > I will double check when TabChild::RecvLoadURL() is called but I'm pretty > sure this was after RecvShow() when I landed this patch. Do you know exactly when RecvShow() happens? The other thing to look into would be why the code I linked to in comment 21 is not sufficient.

(In reply to Olli Pettay [:smaug] from comment #23) > I do recall too that at least originally we did set private browsing mode > early enough. Maybe... You seem to have reviewed more of this code so you probably know better than I do. Also I just remembered having heard of a perhaps related issue on e10s. I'll see if I can find out more about what that was.

Disabling this can not be the option - this is our first "moment in time" release aligning our campaign across all of the mozilla products.

(In reply to Wilfred Mathanaraj [:WDM] from comment #27) > Disabling this can not be the option Given that, by when do we need a fix? I've heard 2.5 won't be "released" until a few weeks from now; is that true?

(In reply to Ehsan Akhgari (don't ask for review please) [Away Nov 3-19] from comment #26) > Also I just remembered having heard of a perhaps related issue on e10s. > I'll see if I can find out more about what that was. I checked, and that was unrelated. Really what we should be doing here is to set the privacy state right after we've created the docshell, not at some point later.

fwiw I can't reproduce at all, which doesn't help investigating :(

(In reply to Andrew Overholt [:overholt] from comment #28) > (In reply to Wilfred Mathanaraj [:WDM] from comment #27) > > Disabling this can not be the option > > Given that, by when do we need a fix? I've heard 2.5 won't be "released" > until a few weeks from now; is that true? RA for 2.5 is tomorrow 10/30/15

I haven't managed to reproduce , but this is possibly a regression from bug 1121905

I think shipping without private browsing is much much better that shipping with a known broken one, if our choices come down to that.

Do we know what it takes to fix/ reproduce this?

I have a possible fix coming, if someone can try a patch. (just trying to be super careful to not regress anything else so verifying various things, so takes still couple of minutes before I upload it.)

So I haven't managed to reproduce this, nor have I ffos dev environment atm, but based on code inspection this might help. The suspicious code is http://mxr.mozilla.org/mozilla-central/source/dom/base/nsFrameLoader.cpp?rev=e8c7dfe727cd&mark=340-340,342-342#326 So that patch makes parent to always send up-to-date ShowInfo, but child actually cares about the first one. The sent amount of data isn't much and LoadURL doesn't happen often. I tried hard to not change name handling (although I think it is wrong on ffos, which is why ApplyShowInfo has IsBrowserOrApp()). https://treeherder.mozilla.org/#/jobs?repo=try&revision=3026b78968d3

"// Don't allow unsetting private browsing mode." comment is wrong. Should be "// No need to re-set private browsing mode."

Comment on attachment 8680948 [details] [diff] [review] possible patch Review of attachment 8680948 [details] [diff] [review]: ----------------------------------------------------------------- That patch works very well for me. Thanks smaug!

Comment on attachment 8680948 [details] [diff] [review] possible patch Want to review too? :)

(I know the patch is a bit ugly, but we should clean this all up once we clean up also ffos window.name handling.)

Comment on attachment 8680948 [details] [diff] [review] possible patch Review of attachment 8680948 [details] [diff] [review]: ----------------------------------------------------------------- lgtm. Can you file the followup for the window.name issues?

Just one small tweak, don't change fullscreenallowed handling at all, so pass false for the fakeShowInfo https://treeherder.mozilla.org/#/jobs?repo=try&revision=c6be160a2587

(In reply to [:fabrice] Fabrice Desré from comment #42) > > lgtm. Can you file the followup for the window.name issues? https://bugzilla.mozilla.org/show_bug.cgi?id=1219998

The original bug only shows up if the pictures have certain sharing restrictions. I guess only 'share with friends'? I tried the 'even safer' patch and I don't see the content in the new window any more. I get now a message within the facebook page saying: "The page you requested cannot be displayed right now..... or you may not have permission to view this page." I would say it fixes the bug. \o/

The patch which moved ::ProvideWindow to ContentChild landed yesterday, so, need to merge with that change.

Verified on [Flame] Build ID 20151102150204 Gaia Revision 7954ff0cbd794a35499a1082bed273598f82ee6f Gaia Date 2015-11-02 17:35:17 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/6275cd9c71b76891f6b6585dabc687bc443ab877 Gecko Version 45.0a1 Device Name flame Firmware(Release) 4.4.2 Firmware(Incremental) eng.cltbld.20151102.182914 Firmware Date Mon Nov 2 18:29:28 EST 2015 Bootloader L1TC000118D0 [Aries] Build ID 20151103000930 Gaia Revision 7954ff0cbd794a35499a1082bed273598f82ee6f Gaia Date 2015-11-02 17:35:17 Gecko Revision https://hg.mozilla.org/mozilla-central/rev/9f69202d82752e093a653a8f15b0274e347db33a Gecko Version 45.0a1 Device Name aries Firmware(Release) 4.4.2 Firmware(Incremental) eng.worker.20151102.233051 Firmware Date Mon Nov 2 23:30:58 UTC 2015 Bootloader s1