1489257 - Unable to log into any google services with GeckoView

Status: RESOLVED FIXED

(GeckoView :: General, defect, P1)

Description

[Randall Barker [:rbarker]]

Trying to log into any google service such as gmail or youtube fails with recent builds of GeckoView.

Comment 1

[Chris Peterson [:cpeterson]]

Is this a recent regression in GV 63 or 64?

Comment 2

[Dylan Roeh (:droeh) (he/him)]

This is fallout from bug 1441059; if you comment out the load delegate checking in nsDocShell::InternalLoad[0] (and aborting the InternalLoad call, which is probably the actual issue), you can log in to google services just fine -- I've tested gmail and youtube in GVE.

[0]: https://searchfox.org/mozilla-central/source/docshell/base/nsDocShell.cpp#9569

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Do we just want to back that out, then? I think we need to consider a different approach.

Comment 4

[Chris Peterson [:cpeterson]]

Dylan, should we just back out bug 1441059? This fallout seems worse than the problem 1441059 fixed. :)

Comment 5

[Dylan Roeh (:droeh) (he/him)]

All things considered I'd say it's best to back out bug 1441059 and bug 1478171 from the 62 relbranch; I can probably provide a stopgap patch for 1478171 that doesn't rely on 1441059 if needed.

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Ryan is comment 5 something you would handle?

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Depends on whether Dylan wants to include his stopgap for bug 1478171 in it or not.

Comment 8

[David Bolter [:davidb] (NeedInfo me for attention)]

Dylan currently driving here.

Comment 9

[Dylan Roeh (:droeh) (he/him)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #7)
> Depends on whether Dylan wants to include his stopgap for bug 1478171 in it
> or not.

I don't think we need to include the stopgap, I can land/uplift it later if necessary. Talking this over on Slack, it seems like to meet everyone's release needs the best thing to do at the moment is to back these patches (for bug 1441059 and bug 1478171) out of: 64, 63, and 62 relbranch. If you could handle it, that would be greatly appreciated.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

I'll do the relbranch backout, but 63/64 are going to need more manual fixing up than I can help with.

https://hg.mozilla.org/releases/mozilla-release/rev/4cdeecd31350b13f8741202dc06d87d8bc3d84db

Comment 11

[Dylan Roeh (:droeh) (he/him)]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #10)
> I'll do the relbranch backout, but 63/64 are going to need more manual
> fixing up than I can help with.
> 
> https://hg.mozilla.org/releases/mozilla-release/rev/
> 4cdeecd31350b13f8741202dc06d87d8bc3d84db

Alright, I'll take care of them. Thanks!

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/16d2d6dc3cdf
Backed out 3 changesets (bug 1441059, bug 1478171) for causing bug 1489257. r=me

Comment 13

[Arthur Iakab [arthur_iakab]]

Backed out changeset 16d2d6dc3cdf (bug 1489257)on request by droeh for causing geckowiev failures.

Backout revision https://hg.mozilla.org/integration/mozilla-inbound/rev/2b83e35d70dfe4628d63182d6576ddca0bf22b80

Failed push https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=16d2d6dc3cdffd6265ebc3366b23947cdd86f467&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-classifiedState=unclassified

Failure log: https://treeherder.mozilla.org/logviewer.html#?job_id=198663587&repo=mozilla-inbound

:doreh could you please take a look?

Comment 14

[Dylan Roeh (:droeh) (he/him)]

Attachment: Back out async LoadURIDelegate

Jim, it looks to me like we're failing (specifically, hanging in waitForPageStop or waitForInitialLoad) on tests where the NavigationDelegate is null, but I can't see any reason why backing out the async LoadURIDelegate and redirect patches would cause this. If you've got a moment, could you take a look and see if anything comes to mind? Thanks.

Comment 15

[(inactive) Jim Chen [:jchen] [:darchons]]

You should back out bug 1478171 as well.

Otherwise, I bet bug 1480095 is interfering with your backout here. All the test failures here immediately follow tests that were changed by bug 1480095.

Comment 16

[(inactive) Jim Chen [:jchen] [:darchons]]

That should be bug 1486525 in the first sentence.

Comment 17

[Dylan Roeh (:droeh) (he/him)]

(In reply to Jim Chen [:jchen] [:darchons] from comment #16)
> That should be bug 1486525 in the first sentence.

I was intending to just @Ignore that test for the time being, as we can still support that with sync LoadURIDelegate pretty easily. Thanks for your help!

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b12998a7102e
Backed out 3 changesets (bug 1441059, bug 1478171) for causing bug 1489257. r=me

Comment 19

[Noemi Erli[:noemi_erli]]

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

Comment 20

[Dylan Roeh (:droeh) (he/him)]

Comment on attachment 9008438 [details] [diff] [review]
Back out async LoadURIDelegate

Approval Request Comment
[Feature/Bug causing the regression]: 1441059
[User impact if declined]: Google Services logins will not work in GV
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: Login to any Google service (gmail, youtube, etc) using GeckoView Example
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: Low risk.
[Why is the change risky/not risky?]: We're just backing out changes and restoring previous behavior.
[String changes made/needed]: N/A

(Note: I'll handle the back out myself, as it's likely to require some manual intervention.)

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9008438 [details] [diff] [review]
Back out async LoadURIDelegate

Go for it. Approved for 63.0b7.

Comment 22

[Randall Barker [:rbarker]]

I've just verified that this fixes the regression with the latest nightly of GeckoView 64.0.20180914100149 in Firefox Reality and was able to log into youtube.com

Comment 23

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/fedb26132f9a