Add indication of camera use on SAML webcam logins

Add indication of camera use on SAML webcam logins

SAML logins on Chrome OS currently display a banner showing the
"authDomain" of the sign-in service.  If the sign-in service tries to
enable the user's webcam, change this banner to warn the user by
adding "is using your camera" verbiage and a red "recording" dot.

BUG=606979
TEST=manual
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/a965a46038909b25ed0d7b14590c8d97df36f780
Cr-Commit-Position: refs/heads/master@{#394519}

[Kevin Cernekee, 2016-05-10 20:57:34]

Description was changed from

==========
Add indication of camera use on SAML webcam logins

SAML logins on Chrome OS currently display a banner showing the
"authDomain" of the sign-in service.  If the sign-in service tries to
enable the user's webcam, change this banner to warn the user by
adding "is using your camera" verbiage and a red "recording" dot.

BUG=606979
TEST=manual
==========

to

==========
Add indication of camera use on SAML webcam logins

SAML logins on Chrome OS currently display a banner showing the
"authDomain" of the sign-in service.  If the sign-in service tries to
enable the user's webcam, change this banner to warn the user by
adding "is using your camera" verbiage and a red "recording" dot.

BUG=606979
TEST=manual
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

[Kevin Cernekee, 2016-05-10 21:05:41]

cernekee@chromium.org changed reviewers:
+ bartfab@chromium.org, emaxx@chromium.org

[Kevin Cernekee, 2016-05-10 21:05:42]

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/screen_gaia_signin.html (right):

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/screen_gaia_signin.html:22: <img
src="chrome://theme/IDR_RESET_WARNING">
This is the wrong icon.

I tried IDR_TAB_RECORDING_INDICATOR but it looks like all platforms except OSX
have switched this from a raster to a vector icon, so it is unavailable on
Chrome OS.

I think I want gfx::VectorIconId::TAB_MEDIA_RECORDING but I don't know how to
get to it from the UI HTML.  Any ideas on how to proceed?

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/screen_gaia_signin.js (right):

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/screen_gaia_signin.js:616:
$('saml-notice-container').style.justifyContent = 'left';
This part doesn't actually work.  Not sure why.

I also tried setAttribute() but that seems to be used for something else.

[dskaram1, 2016-05-13 14:59:10]

https://codereview.chromium.org/1966853002/diff/1/chrome/app/chromeos_strings...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/1966853002/diff/1/chrome/app/chromeos_strings...
chrome/app/chromeos_strings.grdp:1644: This sign-in service, hosted by <ph
name="SAML_DOMAIN">$1<ex>saml.com</ex></ph>, is using your camera.
From UI review, preference is for saying "accessing your camera" vs. "using your
camera" to retain consistency with how Chrome browser says it.

[emaxx, 2016-05-13 17:44:04]

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/screen_gaia_signin.html (right):

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/screen_gaia_signin.html:22: <img
src="chrome://theme/IDR_RESET_WARNING">
On 2016/05/10 21:05:42, Kevin Cernekee wrote:
> This is the wrong icon.
> 
> I tried IDR_TAB_RECORDING_INDICATOR but it looks like all platforms except OSX
> have switched this from a raster to a vector icon, so it is unavailable on
> Chrome OS.
> 
> I think I want gfx::VectorIconId::TAB_MEDIA_RECORDING but I don't know how to
> get to it from the UI HTML.  Any ideas on how to proceed?

I haven't found any way how the vector icon can be requested from an HTML page
(though probably worth asking estade@ - the /ui/gfx/vector_icons/ owner).

Otherwise the necessary IDR_TAB_RECORDING_INDICATOR raster icon could be
requested for Chrome OS too (looks like this stuff is configured in the
/chrome/app/theme/theme_resources.grd file).

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/screen_gaia_signin.js (right):

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/screen_gaia_signin.js:616:
$('saml-notice-container').style.justifyContent = 'left';
On 2016/05/10 21:05:42, Kevin Cernekee wrote:
> This part doesn't actually work.  Not sure why.
> 
> I also tried setAttribute() but that seems to be used for something else.

There is no "left" value among the allowed values for the "justify-content" CSS
property. Probably the "flex-start" value is the right one.

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/screen_gaia_signin.js:621: }
Even though the videoEnabled should never change from true to false, hiding the
recording indicator and restoring the notice container styling in this case
should be a bit cleaner.

[Kevin Cernekee, 2016-05-13 18:39:03]

cernekee@chromium.org changed reviewers:
+ achuith@chromium.org, oshima@chromium.org

[Kevin Cernekee, 2016-05-13 18:39:04]

https://codereview.chromium.org/1966853002/diff/1/chrome/app/chromeos_strings...
File chrome/app/chromeos_strings.grdp (right):

https://codereview.chromium.org/1966853002/diff/1/chrome/app/chromeos_strings...
chrome/app/chromeos_strings.grdp:1644: This sign-in service, hosted by <ph
name="SAML_DOMAIN">$1<ex>saml.com</ex></ph>, is using your camera.
On 2016/05/13 14:59:10, dskaram1 wrote:
> From UI review, preference is for saying "accessing your camera" vs. "using
your
> camera" to retain consistency with how Chrome browser says it.

Done.

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/screen_gaia_signin.html (right):

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/screen_gaia_signin.html:22: <img
src="chrome://theme/IDR_RESET_WARNING">
On 2016/05/13 17:44:03, emaxx wrote:
> On 2016/05/10 21:05:42, Kevin Cernekee wrote:
> > This is the wrong icon.
> > 
> > I tried IDR_TAB_RECORDING_INDICATOR but it looks like all platforms except
OSX
> > have switched this from a raster to a vector icon, so it is unavailable on
> > Chrome OS.
> > 
> > I think I want gfx::VectorIconId::TAB_MEDIA_RECORDING but I don't know how
to
> > get to it from the UI HTML.  Any ideas on how to proceed?
> 
> I haven't found any way how the vector icon can be requested from an HTML page
> (though probably worth asking estade@ - the /ui/gfx/vector_icons/ owner).
> 
> Otherwise the necessary IDR_TAB_RECORDING_INDICATOR raster icon could be
> requested for Chrome OS too (looks like this stuff is configured in the
> /chrome/app/theme/theme_resources.grd file).

Done.

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
File chrome/browser/resources/chromeos/login/screen_gaia_signin.js (right):

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/screen_gaia_signin.js:616:
$('saml-notice-container').style.justifyContent = 'left';
On 2016/05/13 17:44:03, emaxx wrote:
> On 2016/05/10 21:05:42, Kevin Cernekee wrote:
> > This part doesn't actually work.  Not sure why.
> > 
> > I also tried setAttribute() but that seems to be used for something else.
> 
> There is no "left" value among the allowed values for the "justify-content"
CSS
> property. Probably the "flex-start" value is the right one.

Done.

https://codereview.chromium.org/1966853002/diff/1/chrome/browser/resources/ch...
chrome/browser/resources/chromeos/login/screen_gaia_signin.js:621: }
On 2016/05/13 17:44:03, emaxx wrote:
> Even though the videoEnabled should never change from true to false, hiding
the
> recording indicator and restoring the notice container styling in this case
> should be a bit cleaner.

Done.

[emaxx, 2016-05-17 11:12:23]

https://codereview.chromium.org/1966853002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1966853002/diff/20001/chrome/browser/resource...
chrome/browser/resources/gaia_auth_host/authenticator.js:198:
Authenticator.prototype.resetStates_ = function() {
You mentioned in the other CL that manual canceling does not reset the
videoEnabled flag.
Looks like this method is the place where it should happen.

Though I don't know how to verify whether any other code path is left uncovered.

[achuithb, 2016-05-17 17:22:31]

The CQ bit was checked by achuith@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-05-17 17:22:51]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1966853002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1966853002/20001

[commit-bot: I haz the power, 2016-05-17 18:35:46]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-05-17 18:35:47]

Dry run: This issue passed the CQ dry run.

[Kevin Cernekee, 2016-05-17 21:31:05]

https://codereview.chromium.org/1966853002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/gaia_auth_host/authenticator.js (right):

https://codereview.chromium.org/1966853002/diff/20001/chrome/browser/resource...
chrome/browser/resources/gaia_auth_host/authenticator.js:198:
Authenticator.prototype.resetStates_ = function() {
On 2016/05/17 11:12:23, emaxx wrote:
> You mentioned in the other CL that manual canceling does not reset the
> videoEnabled flag.
> Looks like this method is the place where it should happen.
> 
> Though I don't know how to verify whether any other code path is left
uncovered.

Done.

[achuithb, 2016-05-17 22:14:50]

achuith@chromium.org changed reviewers:
+ afakhry@chromium.org

[achuithb, 2016-05-17 22:14:51]

Ahmed, can you review this CL please?

[afakhry, 2016-05-18 00:35:31]

afakhry@chromium.org changed reviewers:
+ xiyuan@chromium.org

[afakhry, 2016-05-18 00:35:32]

lgtm. +xiyuan who is the owner.

[emaxx, 2016-05-18 01:34:12]

lgtm

[oshima, 2016-05-18 16:40:07]

c/a/theme lgtm

[achuithb, 2016-05-18 19:07:02]

lgtm

[achuithb, 2016-05-18 19:08:16]

You should git branch --set-upstream-to=origin/master and re-upload to get rid
of the dependent patchsets

[Kevin Cernekee, 2016-05-18 19:38:26]

The CQ bit was checked by cernekee@chromium.org

[commit-bot: I haz the power, 2016-05-18 19:39:18]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1966853002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1966853002/40001

[commit-bot: I haz the power, 2016-05-18 20:03:24]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-18 20:05:10]

Description was changed from

==========
Add indication of camera use on SAML webcam logins

SAML logins on Chrome OS currently display a banner showing the
"authDomain" of the sign-in service.  If the sign-in service tries to
enable the user's webcam, change this banner to warn the user by
adding "is using your camera" verbiage and a red "recording" dot.

BUG=606979
TEST=manual
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation
==========

to

==========
Add indication of camera use on SAML webcam logins

SAML logins on Chrome OS currently display a banner showing the
"authDomain" of the sign-in service.  If the sign-in service tries to
enable the user's webcam, change this banner to warn the user by
adding "is using your camera" verbiage and a red "recording" dot.

BUG=606979
TEST=manual
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/a965a46038909b25ed0d7b14590c8d97df36f780
Cr-Commit-Position: refs/heads/master@{#394519}
==========

[commit-bot: I haz the power, 2016-05-18 20:05:14]

Patchset 3 (id:??) landed as
https://crrev.com/a965a46038909b25ed0d7b14590c8d97df36f780
Cr-Commit-Position: refs/heads/master@{#394519}