Use window singleton API in the App Developer Tool and simplify the code.

Use window singleton API in the App Developer Tool and simplify the code.

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

[dvh, 2013-05-23 02:51:56]

PTAL.

[Gaurav, 2013-05-23 17:15:26]

https://codereview.chromium.org/15747009/diff/1/chrome/browser/resources/apps...
File chrome/browser/resources/apps_debugger/background.js (right):

https://codereview.chromium.org/15747009/diff/1/chrome/browser/resources/apps...
chrome/browser/resources/apps_debugger/background.js:8: id: 'apps_debugger',
Please rename this to apps_devtool.

[Dan Beam, 2013-05-23 19:58:42]

I don't see your use of a new API, am I missing something?

https://codereview.chromium.org/15747009/diff/5001/chrome/browser/resources/a...
File chrome/browser/resources/apps_debugger/background.js (left):

https://codereview.chromium.org/15747009/diff/5001/chrome/browser/resources/a...
chrome/browser/resources/apps_debugger/background.js:7: if (mainWindow &&
!mainWindow.contentWindow.closed) {
so removing this lets there be multiple apps instead of re-focusing the same
one?

[dvh, 2013-05-23 20:11:03]

On 2013/05/23 19:58:42, Dan Beam wrote:
> I don't see your use of a new API, am I missing something?
> 
>
https://codereview.chromium.org/15747009/diff/5001/chrome/browser/resources/a...
> File chrome/browser/resources/apps_debugger/background.js (left):
> 
>
https://codereview.chromium.org/15747009/diff/5001/chrome/browser/resources/a...
> chrome/browser/resources/apps_debugger/background.js:7: if (mainWindow &&
> !mainWindow.contentWindow.closed) {
> so removing this lets there be multiple apps instead of re-focusing the same
> one?

Actually, this API is not really new, when we set an identifier for the window,
calling chrome.app.window.create() and using the same identifier (here
"apps_devtools") will focus an existing window with the same id and if this kind
of window doesn't exist, it will create a new one.

Here's the documentation for reference:
http://developer.chrome.com/apps/app.window.html

[Dan Beam, 2013-05-23 20:12:38]

https://codereview.chromium.org/15747009/diff/5001/chrome/browser/resources/a...
File chrome/browser/resources/apps_debugger/background.js (right):

https://codereview.chromium.org/15747009/diff/5001/chrome/browser/resources/a...
chrome/browser/resources/apps_debugger/background.js:12: width: 800,
where's

  singleton: true,

?

[dvh, 2013-05-23 20:15:30]

Thanks for noticing. I forgot that.

Though, the default value of singleton is true. That might be why it's just
working right now.
Is it worth to add this value even if the default value is correct?

[Gaurav, 2013-05-23 20:23:59]

https://chromiumcodereview.appspot.com/15747009/diff/5001/chrome/browser/reso...
File chrome/browser/resources/apps_debugger/background.js (right):

https://chromiumcodereview.appspot.com/15747009/diff/5001/chrome/browser/reso...
chrome/browser/resources/apps_debugger/background.js:12: width: 800,
I think it will be good to add it explicity.
On 2013/05/23 20:12:38, Dan Beam wrote:
> where's
> 
>   singleton: true,
> 
> ?

[dvh, 2013-05-23 20:42:41]

On 2013/05/23 20:23:59, Gaurav wrote:
>
https://chromiumcodereview.appspot.com/15747009/diff/5001/chrome/browser/reso...
> File chrome/browser/resources/apps_debugger/background.js (right):
> 
>
https://chromiumcodereview.appspot.com/15747009/diff/5001/chrome/browser/reso...
> chrome/browser/resources/apps_debugger/background.js:12: width: 800,
> I think it will be good to add it explicity.
> On 2013/05/23 20:12:38, Dan Beam wrote:
> > where's
> > 
> >   singleton: true,
> > 
> > ?

Done.

[Dan Beam, 2013-05-23 21:06:47]

lgtm

[commit-bot: I haz the power, 2013-05-23 21:08:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dvh@chromium.org/15747009/12001

[commit-bot: I haz the power, 2013-05-23 21:08:40]

Failed to apply patch for chrome/browser/resources/apps_debugger/background.js:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/resources/apps_debugger/background.js
  Hunk #1 FAILED at 4.
  1 out of 1 hunk FAILED -- saving rejects to file
chrome/browser/resources/apps_debugger/background.js.rej

Patch:       chrome/browser/resources/apps_debugger/background.js
Index: chrome/browser/resources/apps_debugger/background.js
diff --git a/chrome/browser/resources/apps_debugger/background.js
b/chrome/browser/resources/apps_debugger/background.js
index
d5042da674f96253b73ca5b698473784f320f2cd..8d047a761fd517f593e74f7444072e956dc5b666
100644
--- a/chrome/browser/resources/apps_debugger/background.js
+++ b/chrome/browser/resources/apps_debugger/background.js
@@ -4,17 +4,12 @@
 
 var mainWindow = null;
 chrome.app.runtime.onLaunched.addListener(function() {
-  if (mainWindow && !mainWindow.contentWindow.closed) {
-    mainWindow.focus();
-  } else {
-    chrome.app.window.create('main.html', {
-      id: 'apps_debugger',
-      minHeight: 600,
-      minWidth: 800,
-      height: 600,
-      width: 800,
-    }, function(win) {
-      mainWindow = win;
-    });
-  }
+  chrome.app.window.create('main.html', {
+    id: 'apps_devtool',
+    minHeight: 600,
+    minWidth: 800,
+    height: 600,
+    width: 800,
+    singleton: true
+  });
 });

[commit-bot: I haz the power, 2013-05-29 03:06:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/dvh@chromium.org/15747009/12001

[commit-bot: I haz the power, 2013-05-29 11:28:20]

Change committed as 202839