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

chore: Move webFrame scheme privilege methods to main process #16416

Merged
merged 12 commits into from Jan 29, 2019

Conversation

@nitsakh
Copy link
Contributor

commented Jan 16, 2019

Description of Change

This PR updates the custom scheme registration APIs and moves them to the browser process.
#15767 was a precursor to the current PR.
Custom schemes have to be registered from the main process and before app ready.

Checklist

Release Notes

Notes: Moved webFrame custom scheme APIs to browser process under protocol

@nitsakh nitsakh requested review from as code owners Jan 16, 2019

@nitsakh nitsakh requested a review from deepak1556 Jan 17, 2019

@nitsakh nitsakh force-pushed the webframe-scheme-api branch from 1a409dc to 3559051 Jan 17, 2019

@deepak1556 deepak1556 force-pushed the webframe-scheme-api branch from 3559051 to d959455 Jan 20, 2019

@deepak1556 deepak1556 requested a review from zcbenz Jan 21, 2019

@zcbenz

zcbenz approved these changes Jan 22, 2019

Copy link
Member

left a comment

👍

@zcbenz

This comment has been minimized.

Copy link
Member

commented Jan 22, 2019

Can you also document this in docs/api/breaking-changes.md?

And do we have deprecation warnings in 4.x?

@codebytere

This comment has been minimized.

Copy link
Member

commented Jan 23, 2019

@nitsakh once the conflict is fixed and the change added to the api contract i think this is good to go 👍

@deepak1556 deepak1556 force-pushed the webframe-scheme-api branch from d959455 to 12b2a6a Jan 23, 2019

@nitsakh nitsakh force-pushed the webframe-scheme-api branch from 12b2a6a to 5fd7239 Jan 24, 2019

@nitsakh nitsakh force-pushed the webframe-scheme-api branch from 5fd7239 to 1caff08 Jan 29, 2019

@zcbenz zcbenz merged commit 940c4c0 into master Jan 29, 2019

24 of 28 checks passed

appveyor: win-ia32-testing AppVeyor build failed
Details
electron-arm-testing Build #20190129.2 has failed
Details
electron-arm64-testing Build #20190129.3 has failed
Details
Artifact Comparison
Details
Absolute Zero
Backportable? - 5-0-x Clean Backport
Details
Semantic Pull Request ready to be squashed
Details
WIP Ready for review
Details
appveyor: win-ia32-debug 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-lint Build #20190129.2 succeeded
Details
electron-mas-testing Build #20190129.3 succeeded
Details
electron-osx-testing Build #20190129.3 succeeded
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Jan 29, 2019

Release Notes Persisted

Moved webFrame custom scheme APIs to browser process under protocol

@trop

This comment has been minimized.

Copy link
Contributor

commented Jan 29, 2019

I have automatically backported this PR to "5-0-x", please check out #16585

@trop trop bot added in-flight/5-0-x and removed target/5-0-x labels Jan 29, 2019

@nitsakh nitsakh deleted the webframe-scheme-api branch Jan 29, 2019

@miniak

This comment has been minimized.

Copy link
Contributor

commented Jan 29, 2019

@nitsakh, @zcbenz this PR broke the electron.d.ts generation

Error: Ruh roh, "Options" is already declared
    at Object.keys.sort.forEach (/Users/miburda/Development/github/electron-gn/src/electron/node_modules/electron-typescript-definitions/lib/dynamic-param-interfaces.js:83:17)
    at Array.forEach (<anonymous>)
    at Object.flushParamInterfaces (/Users/miburda/Development/github/electron-gn/src/electron/node_modules/electron-typescript-definitions/lib/dynamic-param-interfaces.js:70:142)
    at module.exports (/Users/miburda/Development/github/electron-gn/src/electron/node_modules/electron-typescript-definitions/index.js:69:19)
    at apiPromise.then.then.API (/Users/miburda/Development/github/electron-gn/src/electron/node_modules/electron-typescript-definitions/cli.js:56:18)
    at process._tickCallback (internal/process/next_tick.js:43:7)
    at Function.Module.runMain (internal/modules/cjs/loader.js:777:11)
    at executeUserCode (internal/bootstrap/node.js:342:17)
    at startExecution (internal/bootstrap/node.js:276:5)
    at startup (internal/bootstrap/node.js:227:5)
@nitsakh

This comment has been minimized.

Copy link
Contributor Author

commented Jan 29, 2019

Thanks @miniak . Let me look at it now and I'll send a PR with the fix.

@miniak

This comment has been minimized.

Copy link
Contributor

commented Jan 29, 2019

There are even more issues

test/main.ts(830,12): error TS2339: Property 'registerStandardSchemes' does not exist on type 'Protocol'.

test/main.ts(831,12): error TS2339: Property 'registerServiceWorkerSchemes' does not exist on type 'Protocol'.
test/renderer.ts(73,10): error TS2339: Property 'registerURLSchemeAsBypassingCSP' does not exist on type 'WebFrame'.
test/renderer.ts(74,10): error TS2339: Property 'registerURLSchemeAsPrivileged' does not exist on type 'WebFrame'.
test/renderer.ts(75,10): error TS2339: Property 'registerURLSchemeAsPrivileged' does not exist on type 'WebFrame'.
@miniak

This comment has been minimized.

Copy link
Contributor

commented Jan 29, 2019

@nitsakh, @zcbenz personally, I would recommend reverting this PR until the issues are resolved and applying it again when fixed unless it can be fixed promptly.

nitsakh added a commit that referenced this pull request Jan 30, 2019

feat: move webFrame scheme privilege methods to main process (#16416)
* chore: deprecate webFrame.registerURLSchemeAsPrivileged

* Add register schemes protocol api

* update branch to enable browser process API

* Revert deprecation changes

* Fetch API support

* Updated api to take an array, still working on tests

* Update tests

* Remove web frame API

* Minor changes

* update scheme registrations on browser and renderer process

* fix: enable ses.getBlobData spec

* Update breaking changes doc

MarshallOfSound added a commit that referenced this pull request Jan 30, 2019

chore: Move webFrame scheme privilege methods to main process (#16625)
* feat: move webFrame scheme privilege methods to main process (#16416)

* chore: deprecate webFrame.registerURLSchemeAsPrivileged

* Add register schemes protocol api

* update branch to enable browser process API

* Revert deprecation changes

* Fetch API support

* Updated api to take an array, still working on tests

* Update tests

* Remove web frame API

* Minor changes

* update scheme registrations on browser and renderer process

* fix: enable ses.getBlobData spec

* Update breaking changes doc

* fix: update docs for protocol API (#16601)

* fix: update docs for protocol API

* upddate source for new attribute name

* update electron-typescript-definitions package

@sofianguy sofianguy added this to 5.0.0-beta.2 in 5.0.x Feb 4, 2019

deepak1556 added a commit to microsoft/vscode that referenced this pull request Jun 14, 2019

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