Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(445)

Issue 2082533002: Reland of Clear the login webview when SAML flow is canceled (Closed)

Created:
4 years, 6 months ago by emaxx
Modified:
4 years, 6 months ago
Reviewers:
xiyuan, achuithb
CC:
chromium-reviews, dzhioev+watch_chromium.org, achuith+watch_chromium.org, arv+watch_chromium.org, oshima+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Reland of Clear the login webview when SAML flow is canceled (original CL crrev.com/1988113004). The original CL was later reverted (CL crrev.com/2068953003) due to frequent failures of the SAMLPolicyTest.SAMLInterstitialNext test. This follow-up CL should resolve the tests flakiness. The problem with the original CL was that it could introduce raising the "ready" event too early, so that the test code could start working when the webview was still pointing to the blank page. BUG=613245, 620353 CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation Committed: https://crrev.com/93dc10b6eaaf3be6c4634a2e91e671da21338424 Cr-Commit-Position: refs/heads/master@{#400703}

Patch Set 1 #

Patch Set 2 : Avoid raising "ready" event too early due to redirection of an empty webview #

Unified diffs Side-by-side diffs Delta from patch set Stats (+23 lines, -5 lines) Patch
M chrome/browser/resources/chromeos/login/screen_gaia_signin.js View 2 chunks +8 lines, -1 line 0 comments Download
M chrome/browser/resources/gaia_auth_host/authenticator.js View 1 6 chunks +15 lines, -4 lines 0 comments Download

Messages

Total messages: 12 (6 generated)
emaxx
Hi Xiyuan, Achuith, PTAL. I'm not sure, however, that this update resolves all the issues ...
4 years, 6 months ago (2016-06-20 13:01:30 UTC) #4
xiyuan
lgtm Thanks for digging into it. I think this is fine.
4 years, 6 months ago (2016-06-20 15:00:54 UTC) #5
achuithb
lgtm
4 years, 6 months ago (2016-06-20 15:08:49 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/2082533002/20001
4 years, 6 months ago (2016-06-20 16:57:32 UTC) #8
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 6 months ago (2016-06-20 17:10:01 UTC) #10
commit-bot: I haz the power
4 years, 6 months ago (2016-06-20 17:12:05 UTC) #12
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/93dc10b6eaaf3be6c4634a2e91e671da21338424
Cr-Commit-Position: refs/heads/master@{#400703}

Powered by Google App Engine
This is Rietveld 408576698