Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix missing remote object error when calling remote function created in preload script #15444

Merged
merged 2 commits into from Oct 31, 2018

Conversation

Projects
None yet
5 participants
@zcbenz
Copy link
Member

commented Oct 29, 2018

Description of Change

Previously we were using sender.getProcessId() to determine whether the context of a remote function still lives. But since sender.getProcessId() may still return the ID of previous page when the new page is still loading, we will wrongly report "missing remote object error" when the remote function is created in preload script.

This PR removes the check of sender.getProcessId() in the main process, and instead checks the contextId in the renderer process which is reliable.

Close #14965.

Checklist

  • PR description included and stakeholders cc'd
  • npm test passes
  • tests are changed or added

Release Notes

Notes: Fix missing remote object error when calling remote function created in preload script

@zcbenz

This comment has been minimized.

Copy link
Member Author

commented Oct 29, 2018

Manually backported as in above links.

@MarshallOfSound

This comment has been minimized.

Copy link
Member

commented Oct 29, 2018

@zcbenz There is a consistent test failure here

ipc renderer module remote listeners detaches listeners subscribed to destroyed renderers, and shows a warning - detaches listeners subscribed to destroyed renderers, and shows a warning
@zcbenz

This comment has been minimized.

Copy link
Member Author

commented Oct 30, 2018

I have fixed the failing test.

There is a slight change that, the warning is now printed asynchronously after the renderer process checked the contextId and reported back to the main process. This is because we don't have a reliable way to know whether a remote function still lives in the main process, we have to explicitly ask the renderer process.

w = new BrowserWindow({
show: false,
webPreferences: {
preload: preload

This comment has been minimized.

Copy link
@codebytere

codebytere Oct 31, 2018

Member

nit: preload: preload can just be preload

@jkleinsc jkleinsc self-requested a review Oct 31, 2018

@jkleinsc
Copy link
Contributor

left a comment

LGTM

@jkleinsc jkleinsc merged commit a8f2646 into master Oct 31, 2018

26 checks passed

Absolute Zero
Semantic Pull Request ready to be merged or rebased
Details
WIP ready for review
Details
appveyor: win-ia32-debug AppVeyor build succeeded
Details
appveyor: win-ia32-testing AppVeyor build succeeded
Details
appveyor: win-ia32-testing-pr AppVeyor build succeeded
Details
appveyor: win-x64-debug AppVeyor build succeeded
Details
appveyor: win-x64-testing AppVeyor build succeeded
Details
appveyor: win-x64-testing-pr AppVeyor build succeeded
Details
ci/circleci: linux-arm-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-arm-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-arm64-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-arm64-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-checkout Your tests passed on CircleCI!
Details
ci/circleci: linux-ia32-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-ia32-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-ia32-testing-tests Your tests passed on CircleCI!
Details
ci/circleci: linux-x64-debug Your tests passed on CircleCI!
Details
ci/circleci: linux-x64-testing Your tests passed on CircleCI!
Details
ci/circleci: linux-x64-testing-tests Your tests passed on CircleCI!
Details
electron-arm-testing Build #20181030.2 succeeded
Details
electron-arm64-testing Build #20181030.2 succeeded
Details
electron-lint Build #20181030.1 succeeded
Details
electron-mas-testing Build #20181030.1 succeeded
Details
electron-osx-testing Build #20181030.1 succeeded
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Oct 31, 2018

Release Notes Persisted

Fix missing remote object error when calling remote function created in preload script

@jkleinsc jkleinsc deleted the fix-missing-object branch Oct 31, 2018

@jjcm

This comment has been minimized.

Copy link

commented Oct 31, 2018

Thanks for this fix @zcbenz and props for the quick review @codebytere and @jkleinsc ! This fix really helps me out :) 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.