DescriptionImplement inline signin with iframe
=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=
This is a dup of https://codereview.chromium.org/130963006/ since I cannot
upload to that issue. The only change is to address Xiyuan's two comments
in patchset 3 of that CL.
=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=+=
Inline signin chrome://chrome-signin is currently implemented using webview embedded in webUI, which breaks a couple of features in webUI and has serious accessbility issues. Since webview will be reimplemented based on OOPIF in the near future, and all the issues we have today will no longer apply, thus it is not worth the effort to fix them as they are throw away work. Instead, as suggested by John and prototyped in https://codereview.chromium.org/141363006/, we decide to switch to iframe instead. A few issues worth to mention,
1. The iframe shares the same renderer as the embedder webUI, and thus could be potentially exposed to dangerous webUI privileges. John suggested a fix by assigning a unique storage partition ID to the inline signin page. As a result the inline signin and its embedded web content should never share the same renderer with other webUI pages.
2. webview provides a direct API to inject script and to monitor requests/responses, which is not (directly) available with iframe. The CL works around the issue using content script and background script, quite similar to what CrOS is doing for SAML flow today. Thus it is also the first step towards unifying SAML flows on CrOS and desktop.
3. with webview approach, we used to have a unique temporary partition for each instance of inline signin, in order to make sure multiple instances do not interfere with each other. This is more difficult with the iframe approach, since the partition ID is hardcoded in a quite low layer. In this CL, all inline signin pages share the same persistent partition, which means we have to handle the case when user loads the sign in page with a dirty cookie jar, and thus the newly connected user may not be stored in the primary session. The CL solves the issue by reading 'session_index' from 'google-accounts-signin' header.
BUG=338127
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=251503
Patch Set 1 #
Total comments: 2
Patch Set 2 : Address review comments #Patch Set 3 : rebased #Patch Set 4 : rebased #Patch Set 5 : Make embedded view messaging work #
Total comments: 4
Patch Set 6 : Added browser tests for inline sign in #Patch Set 7 : Rebased and merged #Patch Set 8 : Better fix for messaging #Patch Set 9 : Better fix for message, take 2. Fix CRLF #Patch Set 10 : rebased #Patch Set 11 : Fix endless loop when switching to full tab #Patch Set 12 : rebased #Patch Set 13 : Test for no webui in iframe #
Total comments: 2
Patch Set 14 : rebased #Patch Set 15 : Fix typo #
Total comments: 2
Patch Set 16 : Don't check title1.html ids #
Total comments: 1
Patch Set 17 : Reapply CL #Patch Set 18 : fix for various iframe bugs #Messages
Total messages: 23 (0 generated)
|