[DevTools] Add UI for adding remote targets

[DevTools] Add UI for adding remote targets

BUG=636084
R=dgozman@chromium.org, pfeldman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[eostroukhov, 2016-08-16 19:06:42]

Description was changed from

==========
[DevTools] Add UI for adding remote targets

BUG=636084
R=dgozman@chromium.org, pfeldman@chromium.org
==========

to

==========
[DevTools] Add UI for adding remote targets

BUG=636084
R=dgozman@chromium.org, pfeldman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[eostroukhov, 2016-08-16 19:06:45]

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

[commit-bot: I haz the power, 2016-08-16 19:07:10]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-16 19:57:27]

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

[commit-bot: I haz the power, 2016-08-16 19:57:28]

Dry run: This issue passed the CQ dry run.

[eostroukhov-old, 2016-08-17 17:27:35]

The CQ bit was checked by eostroukhov@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-08-17 17:28:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eostroukhov-old, 2016-08-17 17:29:25]

The CQ bit was unchecked by eostroukhov@google.com

[eostroukhov, 2016-08-17 17:29:58]

Rebased

[eostroukhov, 2016-08-17 17:30:00]

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

[commit-bot: I haz the power, 2016-08-17 17:30:11]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eostroukhov, 2016-08-17 17:32:45]

eostroukhov@chromium.org changed reviewers:
- eostroukhov@google.com

[eostroukhov, 2016-08-17 17:32:46]

Please consider.

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/inspect/inspect.js (right):

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
chrome/browser/resources/inspect/inspect.js:613:
$('config-dialog').commit(true);
I changed the behavior for consistency with the dialog close (e.g. when Esc is
pressed) - previously Esc would've saved the input while this button would not.

I am not sure saving is a desirable behaviour here, still want the consistency.

[commit-bot: I haz the power, 2016-08-17 18:19:31]

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

[commit-bot: I haz the power, 2016-08-17 18:19:32]

Dry run: This issue passed the CQ dry run.

[dgozman, 2016-08-18 01:17:18]

dgozman@chromium.org changed reviewers:
+ lushnikov@chromium.org
- pfeldman@chromium.org

[dgozman, 2016-08-18 01:17:19]

Please ad a screenshot to the bug.

Andrey, mind taking a look at inspect.js?

[eostroukhov, 2016-08-18 17:34:01]

On 2016/08/18 01:17:19, dgozman wrote:
> Please ad a screenshot to the bug.
> 
> Andrey, mind taking a look at inspect.js?

Added a screenshot:
https://storage.googleapis.com/monorail-prod.appspot.com/16/attachments/ef9f8...

[lushnikov, 2016-08-18 20:42:15]

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/inspect/inspect.js (right):

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
chrome/browser/resources/inspect/inspect.js:698: function
commitPortForwardingConfig(closeConfig) {
what's this closeConfig?

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
chrome/browser/resources/inspect/inspect.js:743: element.onclose = null;
don't you want to save dialog here as well? (like you do for "cross" button)

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
chrome/browser/resources/inspect/inspect.js:755: var config = {};
unused

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
chrome/browser/resources/inspect/inspect.js:811: var newSelection =
line.nextElementSibling || line.previousElementSimpling;
typo: line.previousElementSibling

[eostroukhov, 2016-08-18 21:39:51]

Comments addressed

[eostroukhov, 2016-08-18 21:39:53]

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

[eostroukhov, 2016-08-18 21:40:42]

Thanks for the review. Please take another look.

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/inspect/inspect.js (right):

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
chrome/browser/resources/inspect/inspect.js:698: function
commitPortForwardingConfig(closeConfig) {
On 2016/08/18 20:42:14, lushnikov wrote:
> what's this closeConfig?

Not used anymore. Thanks!

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
chrome/browser/resources/inspect/inspect.js:743: element.onclose = null;
On 2016/08/18 20:42:14, lushnikov wrote:
> don't you want to save dialog here as well? (like you do for "cross" button)

Actually, selectTab can no longer be invoked since the dialog is shown with
showModal - so I will inline this function.

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
chrome/browser/resources/inspect/inspect.js:755: var config = {};
On 2016/08/18 20:42:14, lushnikov wrote:
> unused

Thanks.

https://codereview.chromium.org/2248133002/diff/20001/chrome/browser/resource...
chrome/browser/resources/inspect/inspect.js:811: var newSelection =
line.nextElementSibling || line.previousElementSimpling;
On 2016/08/18 20:42:14, lushnikov wrote:
> typo: line.previousElementSibling

Done.

[commit-bot: I haz the power, 2016-08-18 21:41:02]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 00:00:48]

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

[commit-bot: I haz the power, 2016-08-19 00:00:52]

Dry run: This issue passed the CQ dry run.

[eostroukhov, 2016-08-19 16:28:22]

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

[commit-bot: I haz the power, 2016-08-19 16:28:45]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-19 16:32:22]

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

[commit-bot: I haz the power, 2016-08-19 16:32:23]

Dry run: This issue passed the CQ dry run.

[lushnikov, 2016-08-23 19:23:02]

thanks

Are there any tests/lints for the webui?

[eostroukhov, 2016-08-23 22:15:26]

On 2016/08/23 19:23:02, lushnikov wrote:
> thanks
> 
> Are there any tests/lints for the webui?

I do not see anything for these dialogs...

[lushnikov, 2016-08-24 21:57:00]

lgtm for the html/js then

[eostroukhov, 2016-08-24 22:00:06]

The CQ bit was checked by eostroukhov@chromium.org

[commit-bot: I haz the power, 2016-08-24 22:00:59]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-24 23:36:46]

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

[commit-bot: I haz the power, 2016-08-24 23:36:47]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[eostroukhov, 2016-08-24 23:47:44]

The CQ bit was checked by eostroukhov@chromium.org

[commit-bot: I haz the power, 2016-08-24 23:48:51]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 00:15:14]

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

[commit-bot: I haz the power, 2016-08-25 00:15:15]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[dgozman, 2016-08-25 00:19:19]

lgtm

[eostroukhov, 2016-08-25 16:45:55]

The CQ bit was checked by eostroukhov@chromium.org

[commit-bot: I haz the power, 2016-08-25 16:46:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-25 16:55:21]

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

[commit-bot: I haz the power, 2016-08-25 16:55:23]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[eostroukhov, 2016-08-25 16:58:24]

eostroukhov@chromium.org changed reviewers:
+ pfeldman@chromium.org

[eostroukhov, 2016-08-30 17:14:05]

Added a checkbox

[eostroukhov, 2016-08-30 17:14:07]

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

[commit-bot: I haz the power, 2016-08-30 17:14:29]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eostroukhov, 2016-08-30 17:14:52]

I added a checkbox. Please take another look.

[commit-bot: I haz the power, 2016-08-30 18:13:04]

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

[commit-bot: I haz the power, 2016-08-30 18:13:05]

Dry run: This issue passed the CQ dry run.

[eostroukhov, 2016-08-30 21:50:30]

The CQ bit was checked by eostroukhov@chromium.org

[eostroukhov, 2016-08-30 21:50:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
dgozman@chromium.org, lushnikov@chromium.org
Link to the patchset: https://codereview.chromium.org/2248133002/#ps80001
(title: "[DevTools] Add UI for adding remote targets")

[eostroukhov, 2016-08-30 21:50:37]

The CQ bit was unchecked by eostroukhov@chromium.org

[eostroukhov, 2016-08-30 21:51:18]

The CQ bit was checked by eostroukhov@chromium.org

[commit-bot: I haz the power, 2016-08-30 21:52:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 22:01:43]

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

[commit-bot: I haz the power, 2016-08-30 22:01:45]

Exceeded global retry quota

[pfeldman, 2016-08-30 22:55:36]

lgtm

[eostroukhov, 2016-08-30 22:59:56]

The CQ bit was checked by eostroukhov@chromium.org

[commit-bot: I haz the power, 2016-08-30 23:00:44]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-30 23:25:44]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-08-30 23:29:38]

Description was changed from

==========
[DevTools] Add UI for adding remote targets

BUG=636084
R=dgozman@chromium.org, pfeldman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
[DevTools] Add UI for adding remote targets

BUG=636084
R=dgozman@chromium.org, pfeldman@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

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

[commit-bot: I haz the power, 2016-08-30 23:29:40]

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