Hangout remote desktop part II - background.html and AppLauncher

remoting/webapp/background/background.js

Index: remoting/webapp/background/background.js
diff --git a/remoting/webapp/background/background.js b/remoting/webapp/background/background.js
new file mode 100644
index 0000000000000000000000000000000000000000..db2b5e209bef7adf9ec30ce6a23902e3feffd3fa
--- /dev/null
+++ b/remoting/webapp/background/background.js
@@ -0,0 +1,72 @@
+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/** @suppress {duplicate} */
+var remoting = remoting || {};
+
+(function(){

[Jamie, 2014/08/12 02:24:57]

We don't use this construct elsewhere, and IIRC, it doesn't work well with
JSCompile.

Speaking of JSCompile, the background page has enough complexity that I think it
needs to be run through it. The right way to do that is probably to modify the
gyp rule that runs jscompile to operate at the level of an html page to verify
that not only does our Javascript compile, we're not missing any <script> tags.
That doesn't need to be in this CL, but I think it's a high priority work item.

[kelvinp, 2014/08/12 18:22:44]

Before this CL, JSCompile is not run on background.js, but now it does!

This construct known as IIFE (immediately invoked function execution) prevents
us from leaking variables and functions into the global namespace.  Consider the
scenario when two JavaScript files declares a helper function with the same
name, logDebugInfo.  Without wrapping the definitions in the IIFE, the later
definition will override the previous one, which will lead to bugs. 

IIFE is an industrial wide practice.  You will find this practice in every major
JavaScript library (case and point, the polymer project source code! See
https://github.com/Polymer/polymer-dev/blob/master/src/lib/dom.js.)  I would
argue that as long as JSCompile permit, we should use it everywhere in our code
base.

You may wonder.  Does it break JSCompile in this case?  No, JSCompile cannot
extract type information inside an IIFE.   As long as our file, background.js,
does not define new types, but only uses it, we can use this construct.

[Jamie, 2014/08/12 20:42:53]

Acknowledged.

+
+/** @return {boolean} */
+function isAppsV2() {
+  var manifest = chrome.runtime.getManifest();
+  if (manifest && manifest.app && manifest.app.background) {
+    return true;
+  }
+  return false;
+}
+
+/** @param {remoting.AppLauncher} appLauncher */
+function initializeAppV2(appLauncher) {
+  /** @type {string} */
+  var kNewWindowId = 'new-window';
+
+  /** @param {OnClickData} info */
+  function onContextMenu(info) {
+    if (info.menuItemId == kNewWindowId) {
+      appLauncher.launch();
+    }
+  }
+
+  function initializeContextMenu() {
+    chrome.contextMenus.create({
+       id: kNewWindowId,
+       contexts: ['launcher'],
+       title: chrome.i18n.getMessage(/*i18n-content*/'NEW_WINDOW')

[Jamie, 2014/08/12 02:24:57]

Please verify that moving this string to a new file doesn't introduce any build
warnings.

[kelvinp, 2014/08/12 18:22:44]

Hence the renaming of the file in remoting.gyp. 

+    });
+    chrome.contextMenus.onClicked.addListener(onContextMenu);
+  }
+
+  initializeContextMenu();
+  chrome.app.runtime.onLaunched.addListener(appLauncher.launch);

[Jamie, 2014/08/12 02:24:57]

Does appLauncher.launch need a bind()?

[kelvinp, 2014/08/12 18:22:44]

No, AppLauncher.launch does not refer to any local state.

[Jamie, 2014/08/12 20:42:52]

The question is whether it's a static function or not. Functions in a classes
prototype are non-static and need to be bound--the fact that the current
implementation doesn't access 'this' is an implementation detail that this
initializeContextMenu can't (and shouldn't) know about.

+}
+
+/**
+ * The background service is responsible for listening to incoming connection
+ * requests from the hangout page and the webapp.

[Jamie, 2014/08/12 02:24:57]

s/hangout/Hangouts/ (branding)

[kelvinp, 2014/08/12 18:22:44]

Done.

+ * @param {remoting.AppLauncher} appLauncher
+ */
+function initializeBackgroundService(appLauncher) {
+  /** @type {remoting.It2MeService} */
+  var it2meService = new remoting.It2MeService(appLauncher);
+  it2meService.init();
+  remoting.it2meService = it2meService;
+
+  chrome.runtime.onSuspend.addListener(function() {
+    it2meService.dispose();
+  });
+}
+
+function main() {
+  /** @type {remoting.AppLauncher} */
+  var appLauncher = new remoting.V1AppLauncher();
+  if (isAppsV2()) {
+    appLauncher = new remoting.V2AppLauncher();
+    initializeAppV2(appLauncher);
+  }
+  initializeBackgroundService(appLauncher);
+}
+
+main();
+
+}());