Basic OAuth2 support using the native app flow.

Basic OAuth2 support using the native app flow.

Currently, OAuth2 does not support redirection to chrome-extension URLs. So,
temporarily, we will use the native app flow.  In the future, this should be
replaced with the "PostMessage" flow.

BUG=none
TEST=Invoke javascript functions by hand and see that we get an access token.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86034

[awong, 2011-05-19 19:04:03]

Here's basic OAuth2 support.  I'll try to wire-in the OAuth2 token into the
support-host-id request page in a sec.

[awong, 2011-05-19 20:38:41]

Ready for review.

[Jamie, 2011-05-19 21:01:04]

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/backg...
File remoting/webapp/me2mom/background.js (right):

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/backg...
remoting/webapp/me2mom/background.js:23: 
Does this belong in this file? It doesn't seem to have anything to do with the
background page. Perhaps it should have a file to itself?

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/backg...
remoting/webapp/me2mom/background.js:26: // Ideally, this should impelment the
OAuth2 PostMessage flow to avoid needing
Nit: implement.

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/backg...
remoting/webapp/me2mom/background.js:50: OAuth2.prototype.getAccessToken =
function() {
I would expect a get method to have a return statement. Perhaps this should be
renamed ensureAccessToken()?

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/backg...
remoting/webapp/me2mom/background.js:100: var that = this;
:)

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/main.css
File remoting/webapp/me2mom/main.css (right):

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/main....
remoting/webapp/me2mom/main.css:143: div#xmpp_div {
Why bother with this? IMO, it looks kind of weird to have it on only one of the
auth panels, and the UI is very much bare-bones at the moment anyway.

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/remot...
File remoting/webapp/me2mom/remoting.js (right):

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/remot...
remoting/webapp/me2mom/remoting.js:292: if
(remoting.oauth2.needsNewAccessToken()) {
Should needsNewAccessToken take RTT into account, and refresh the access token a
minute or two early to make sure that it's still valid when the server verifies
it. Otherwise won't we see (very infrequent) failures?

[awong, 2011-05-19 21:33:05]

PTAL

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/backg...
File remoting/webapp/me2mom/background.js (right):

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/backg...
remoting/webapp/me2mom/background.js:23: 
On 2011/05/19 21:01:04, Jamie wrote:
> Does this belong in this file? It doesn't seem to have anything to do with the
> background page. Perhaps it should have a file to itself?

Moved out.

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/backg...
remoting/webapp/me2mom/background.js:26: // Ideally, this should impelment the
OAuth2 PostMessage flow to avoid needing
On 2011/05/19 21:01:04, Jamie wrote:
> Nit: implement.

Done.

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/backg...
remoting/webapp/me2mom/background.js:50: OAuth2.prototype.getAccessToken =
function() {
On 2011/05/19 21:01:04, Jamie wrote:
> I would expect a get method to have a return statement. Perhaps this should be
> renamed ensureAccessToken()?

Cut and paste error.  This function is defined lower.

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/backg...
remoting/webapp/me2mom/background.js:100: var that = this;
On 2011/05/19 21:01:04, Jamie wrote:
> :)

Heh...apparently "var that = this;" is a common javascript idiom...at least
according to StackOverflow. :-/

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/main.css
File remoting/webapp/me2mom/main.css (right):

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/main....
remoting/webapp/me2mom/main.css:143: div#xmpp_div {
On 2011/05/19 21:01:04, Jamie wrote:
> Why bother with this? IMO, it looks kind of weird to have it on only one of
the
> auth panels, and the UI is very much bare-bones at the moment anyway.

...I was going to deprecate the XMPP token in the next CL, so I wanted to shade
it. but I can remove it.

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/remot...
File remoting/webapp/me2mom/remoting.js (right):

http://codereview.chromium.org/7046012/diff/1006/remoting/webapp/me2mom/remot...
remoting/webapp/me2mom/remoting.js:292: if
(remoting.oauth2.needsNewAccessToken()) {
On 2011/05/19 21:01:04, Jamie wrote:
> Should needsNewAccessToken take RTT into account, and refresh the access token
a
> minute or two early to make sure that it's still valid when the server
verifies
> it. Otherwise won't we see (very infrequent) failures?

I timeshifted it on the setting of hte expiration.

[Jamie, 2011-05-19 21:50:50]

One comment, which you may or may not agree with. LGTM either way.

http://codereview.chromium.org/7046012/diff/1009/remoting/webapp/me2mom/remot...
File remoting/webapp/me2mom/remoting.js (right):

http://codereview.chromium.org/7046012/diff/1009/remoting/webapp/me2mom/remot...
remoting/webapp/me2mom/remoting.js:11: 
I think this would still be better in a separate file. It feels independent and
complete enough to warrant that and would help keep this file more UI-focussed.

[awong, 2011-05-20 01:37:55]

moved. committing.