Run 5493 external window id updates #939

dhamberlin · 2019-09-16T18:40:50Z

Description of Change

Jira: https://appoji.jira.com/browse/RUN-5493

This PR makes a few changes to the ExternalWindow API:

ExternalWindows can be registered by nativeId (nativeId === hwnd in Windows) OR uuid (if the uuid has already been assigned to an external process via e.g. System.launchExternalProcess
ExternalWindow.wrap returns a uuid, name, and nativeId to caller, ensuring that render-side ExternalWindows will always have all three, even if user wrapped the window by uuid or nativeId alone.
ExternalWindow uuids are locked on registration, released on window close.
nativeId is exposed on all ExternalWindow info objects, such as those returned by System.getAllExternalWindows as well as on 'external-window' events.
uuid in included on all ExternalWindow info objects and external window events if a) the window has already been wrapped as an ExternalWindow or b) the pid matches a process that is being tracked by ProcessTracker (e.g. those launched by launchExternalProcess)

Related js-adapter PR: HadoukenIO/js-adapter#341

Checklist

PR description included and stakeholders cc'd
npm test passes
automated tests are changed or added ( a couple private ones I will make public post-merge, and more to come)
relevant documentation is changed or added (more to come here, working on adding/updating tutorials now)
PR title starts with the JIRA ticket pull request process
PR release notes describe the change in a way relevant to app-developers (included in js-adapter PR)
PR has assigned reviewers

Release Notes

Notes:

openfin-github-bot · 2019-09-16T19:09:04Z

Test Results for `97047d0`

Asars
- app.asar
- adapter.asar
JavaScript
- 😱 Windows 10 (32-bit runtime): Failed 8/664
- 😱 Windows 7 (32-bit runtime): Failed 8/664
- 😱 Windows 7 (64-bit runtime): Failed 8/664
- 😱 Windows 10 (64-bit runtime): Failed 8/664

openfin-github-bot · 2019-09-17T19:37:04Z

Test Results for `d62fe42`

Asars
- app.asar
- adapter.asar
JavaScript
- 😰 Windows 7 (32-bit runtime): Failed 1/664
- 😰 Windows 10 (32-bit runtime): Failed 1/664
- ✅ Windows 7 (64-bit runtime): Failed 0/664
- ✅ Windows 10 (64-bit runtime): Failed 0/664

openfin-github-bot · 2019-10-08T16:34:04Z

0b4518a

Git

core: develop <= RUN-5493-external-window-id-updates (0b4518a)
js-adapter: develop <= RUN-5493-external-window-id-updates (5fa4838)
javascript-adapter: develop

Asars used for testing

Test results

😱 Windows 10 (64-bit runtime): Failed 9/665
✅ Windows 7 (64-bit runtime): Failed 0/665
😰 Windows 7 (32-bit runtime): Failed 1/665
😱 Windows 10 (32-bit runtime): Failed 7/665

licui3936 · 2019-10-11T14:20:46Z

src/browser/api/external_window.ts

 // Window grouping stub (makes external windows work with our original disabled frame group tracker)
 // Also some of the _options' values are needed in OpenFin Layouts
-function setAdditionalProperties(externalWindow: Shapes.ExternalWindow, properIdentity: Shapes.NativeWindowIdentity): Shapes.GroupWindow {
+function setAdditionalProperties(externalWindow: Shapes.ExternalWindow, requestedId: Shapes.NativeWindowIdentity): Shapes.GroupWindow {


Feel like 'requestedId' is a string. How about 'requestedIdentity' instead?

Makes sense to me. Updated.

openfin-github-bot · 2019-10-11T15:39:04Z

1b8ed89

Git

core: develop <= RUN-5493-external-window-id-updates (1b8ed89)
js-adapter: develop <= RUN-5493-external-window-id-updates (5fa4838)
javascript-adapter: develop

Asars used for testing

Test results

✅ Windows 7 (32-bit runtime): Failed 0/665
✅ Windows 7 (64-bit runtime): Failed 0/665
😰 Windows 10 (64-bit runtime): Failed 1/665
✅ Windows 10 (32-bit runtime): Failed 0/665

openfin-github-bot · 2019-10-15T15:13:04Z

4c2e3ba

Git

core: develop <= RUN-5493-external-window-id-updates (4c2e3ba)
js-adapter: develop <= RUN-5493-external-window-id-updates (5fa4838)
javascript-adapter: develop

Asars used for testing

Test results

✅ Windows 7 (64-bit runtime): Failed 0/665
😰 Windows 10 (64-bit runtime): Failed 1/665
✅ Windows 10 (32-bit runtime): Failed 0/665
✅ Windows 7 (32-bit runtime): Failed 0/665

tomer-openfin

added 1 comment.
except that it lgtm

tomer-openfin · 2019-10-16T14:25:09Z

src/browser/api/external_window.ts

  const { nativeId } = externalWindow;
-  const uuid = properIdentity.uuid || nativeId;
-  const name = properIdentity.name || nativeId;
+  const uuid = externalWindow.uuid || requestedIdentity.uuid || nativeId;


is it possible that the nativeId here is already taken as a uuid by some other window?

Thanks for catching this. Actually not a problem because we ensure that we have a usable uuid before we call this function (line 273: const uuid = identity.uuid || electronApp.generateGUID();, so we'll try a user-provided uuid if included, otherwise use a new guid).

This does mean that these || are unnecessary. Removed them accordingly.

…berlin/core into RUN-5493-external-window-id-updates

openfin-github-bot · 2019-10-16T18:42:05Z

884685b

Git

core: develop <= RUN-5493-external-window-id-updates (884685b)
js-adapter: develop <= RUN-5493-external-window-id-updates (5fa4838)
javascript-adapter: develop

Asars used for testing

Test results

😱 Windows 10 (64-bit runtime): Failed 4/665
😱 Windows 7 (32-bit runtime): Failed 17/665
😱 Windows 7 (64-bit runtime): Failed 17/665
😰 Windows 10 (32-bit runtime): Failed 1/665

dhamberlin added 13 commits August 19, 2019 21:03

RUN-5493: add nativeId to NW info objects

df79dc5

RUN-5493: Differentiate nativeId and uuid

ec89940

WIP: uuid enforcement for external windows

f0ac310

Merge branch 'develop' into RUN-5493-nativeId-refactor

1fc4274

RUN-5493: Remove uuids for unregistered NWs

e0917a5

RUN-5493: Fix NWI cleanup

36cadf9

RUN-5493: Include uuid in events and getAllExternalWindows

51715e8

RUN-5493: use external process uuid for NWI events when relevant

b5abd41

Resolve merge conflicts

d103e35

Merge branch 'develop' into RUN-5493-external-window-id-updates

fabdf72

remove unwanted changes

11f14fc

RUN-5493 fix js-adapter version

c09baa4

RUN-5493 alphabetize

97047d0

dhamberlin requested review from datamadic, licui3936, nicholasdgoodman and rdepena September 16, 2019 18:40

finos-admin added the cla-present label Sep 16, 2019

openfin-github-bot bot added the auto testing started Bot started automated testing label Sep 16, 2019

openfin-github-bot bot added auto testing done Bot completed automated testing and removed auto testing started Bot started automated testing labels Sep 16, 2019

dhamberlin added the do not merge PR is not ready to be merged label Sep 17, 2019

Run-5492: fix NWI eventing

d62fe42

openfin-github-bot bot added auto testing started Bot started automated testing and removed auto testing done Bot completed automated testing labels Sep 17, 2019

openfin-github-bot bot added auto testing done Bot completed automated testing and removed auto testing started Bot started automated testing labels Sep 17, 2019

RUN-5493 get correct window info from getInfo call

0ea5335

openfin-github-bot bot added auto testing started Bot started automated testing and removed auto testing done Bot completed automated testing labels Oct 8, 2019

openfin-github-bot bot added auto testing done Bot completed automated testing and removed auto testing started Bot started automated testing labels Oct 8, 2019

licui3936 reviewed Oct 11, 2019

View reviewed changes

Rename requestedId -> requestedIdentity

1b8ed89

dhamberlin dismissed rdepena’s stale review via 1b8ed89 October 11, 2019 15:01

openfin-github-bot bot added auto testing started Bot started automated testing and removed auto testing done Bot completed automated testing labels Oct 11, 2019

openfin-github-bot bot added auto testing done Bot completed automated testing and removed auto testing started Bot started automated testing labels Oct 11, 2019

Merge branch 'develop' into RUN-5493-external-window-id-updates

4c2e3ba

openfin-github-bot bot added auto testing started Bot started automated testing and removed auto testing done Bot completed automated testing labels Oct 15, 2019

openfin-github-bot bot added auto testing done Bot completed automated testing and removed auto testing started Bot started automated testing labels Oct 15, 2019

rdepena previously approved these changes Oct 16, 2019

View reviewed changes

tomer-openfin reviewed Oct 16, 2019

View reviewed changes

dhamberlin added 2 commits October 16, 2019 13:05

Remove unneeded uuid backup

c451547

Merge branch 'RUN-5493-external-window-id-updates' of github.com:dham…

884685b

…berlin/core into RUN-5493-external-window-id-updates

dhamberlin dismissed rdepena’s stale review via 884685b October 16, 2019 17:07

openfin-github-bot bot added auto testing started Bot started automated testing and removed auto testing done Bot completed automated testing labels Oct 16, 2019

openfin-github-bot bot added auto testing done Bot completed automated testing and removed auto testing started Bot started automated testing labels Oct 16, 2019

tomer-openfin approved these changes Oct 16, 2019

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Run 5493 external window id updates #939

Run 5493 external window id updates #939

Uh oh!

dhamberlin commented Sep 16, 2019

Uh oh!

openfin-github-bot bot commented Sep 16, 2019

Uh oh!

openfin-github-bot bot commented Sep 17, 2019

Uh oh!

openfin-github-bot bot commented Oct 8, 2019

Uh oh!

licui3936 Oct 11, 2019

Uh oh!

dhamberlin Oct 11, 2019

Uh oh!

openfin-github-bot bot commented Oct 11, 2019

Uh oh!

openfin-github-bot bot commented Oct 15, 2019

Uh oh!

tomer-openfin left a comment

Uh oh!

tomer-openfin Oct 16, 2019

Uh oh!

dhamberlin Oct 16, 2019 •

edited

Loading

Uh oh!

openfin-github-bot bot commented Oct 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Run 5493 external window id updates #939

Are you sure you want to change the base?

Run 5493 external window id updates #939

Uh oh!

Conversation

dhamberlin commented Sep 16, 2019

Description of Change

Checklist

Release Notes

Uh oh!

openfin-github-bot bot commented Sep 16, 2019

Test Results for 97047d0

Uh oh!

openfin-github-bot bot commented Sep 17, 2019

Test Results for d62fe42

Uh oh!

openfin-github-bot bot commented Oct 8, 2019

Git

Asars used for testing

Test results

Uh oh!

licui3936 Oct 11, 2019

Choose a reason for hiding this comment

Uh oh!

dhamberlin Oct 11, 2019

Choose a reason for hiding this comment

Uh oh!

openfin-github-bot bot commented Oct 11, 2019

Git

Asars used for testing

Test results

Uh oh!

openfin-github-bot bot commented Oct 15, 2019

Git

Asars used for testing

Test results

Uh oh!

tomer-openfin left a comment

Choose a reason for hiding this comment

Uh oh!

tomer-openfin Oct 16, 2019

Choose a reason for hiding this comment

Uh oh!

dhamberlin Oct 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openfin-github-bot bot commented Oct 16, 2019

Git

Asars used for testing

Test results

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Test Results for `97047d0`

Test Results for `d62fe42`

dhamberlin Oct 16, 2019 •

edited

Loading