Add 'New window' option to context menu.

Add 'New window' option to context menu.

See https://codereview.chromium.org/18024002 for a similar example.

Note that this API doesn't work on UNIX, so we still need an in-app method of
opening a new window.

BUG=252881
R=lambroslambrou@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221852

[Jamie, 2013-09-06 17:59:41]

ptal

[Lambros, 2013-09-06 22:16:14]

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/background.js
File remoting/webapp/background.js (right):

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/background.js...
remoting/webapp/background.js:27: // try to recreate it every time.
This is the only place you're adding the context menu, and you only call it
once. Why worry about it? :)

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/background.js...
remoting/webapp/background.js:38: initializeContextMenu();
Shouldn't we call this from remoting.init() (or the window 'load' handler)?

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/jscompiler_ha...
File remoting/webapp/jscompiler_hacks.js (right):

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/jscompiler_ha...
remoting/webapp/jscompiler_hacks.js:133: * @param {function()=} opt_callback
nit: I think "opt_" is confusing, because it could mean option or optional.

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/jscompiler_ha...
remoting/webapp/jscompiler_hacks.js:134: */
nit: Add @return, since create() returns integer or string.

[Jamie, 2013-09-06 23:45:50]

ptal

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/background.js
File remoting/webapp/background.js (right):

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/background.js...
remoting/webapp/background.js:27: // try to recreate it every time.
On 2013/09/06 22:16:14, Lambros wrote:
> This is the only place you're adding the context menu, and you only call it
> once. Why worry about it? :)

I added it because the sample I based it on did so. However, I can't see any
reason for it, so I'm removing it.

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/background.js...
remoting/webapp/background.js:38: initializeContextMenu();
On 2013/09/06 22:16:14, Lambros wrote:
> Shouldn't we call this from remoting.init() (or the window 'load' handler)?

This is the background page. It gets run when the extension is added or when the
app launcher starts up.

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/jscompiler_ha...
File remoting/webapp/jscompiler_hacks.js (right):

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/jscompiler_ha...
remoting/webapp/jscompiler_hacks.js:133: * @param {function()=} opt_callback
On 2013/09/06 22:16:14, Lambros wrote:
> nit: I think "opt_" is confusing, because it could mean option or optional.

opt_ is a standard prefix for optional parameters (and is in fact required by
the linter).

https://codereview.chromium.org/23536033/diff/1/remoting/webapp/jscompiler_ha...
remoting/webapp/jscompiler_hacks.js:134: */
On 2013/09/06 22:16:14, Lambros wrote:
> nit: Add @return, since create() returns integer or string.

Done.

[Lambros, 2013-09-07 00:24:47]

lgtm

[commit-bot: I haz the power, 2013-09-07 00:37:04]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jamiewalch@chromium.org/23536033/7001

[commit-bot: I haz the power, 2013-09-07 00:55:20]

Step "update" is always a major failure.
Look at the try server FAQ for more details.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android_cl...

[Jamie, 2013-09-07 01:40:04]

Committed patchset #2 manually as r221852 (presubmit successful).