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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make devtools extensions load correctly #17614

Merged
merged 1 commit into from Mar 31, 2019

Conversation

@MarshallOfSound
Copy link
Member

commented Mar 30, 2019

This is a multi-part fix as each fix revealed a new bug, so, yay I guess 馃槃

Fixes #17586

send_to_all now works better'er

First off, we should remove this API, it's private and bad and we can remove it as soon as extension support lands for real in #17440

Previously send_to_all was handled on the render_frame side and messages were dispatched to the main frame which then proxied the event to all WebLocalFrame's. This missed out on all WebRemoteFrame's which only became a problem recently.

This PR changes our send_to_all logic where it matters to send to all frames from the browser process. There is a TODO to completely move our send_to_all logic and then remove it 馃憤

Legacy DevToolsAPI

We were still using the legacy DevToolsAPI attach point, updated to use the InspectorFrontendAPI API

chrome.browserAction stubs

Newer versions of React Dev Tools require those APIs to exist or it throws 馃憤

Fixing chrome.runtime.connect

The ipc utils refactor made these APIs stop working so now we use invoke on the renderer side correctly

Notes: DevTools Extensions now load correctly

@nornagon
Copy link
Contributor

left a comment

seems fine

@trop

This comment has been minimized.

Copy link
Contributor

commented Mar 30, 2019

A maintainer has manually backported this PR to "5-0-x", please check out #17616

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

@MarshallOfSound MarshallOfSound merged commit 75442b7 into master Mar 31, 2019

14 of 16 checks passed

build-mac Workflow: build-mac
Details
Backportable? - 5-0-x Cancelled
Details
Artifact Comparison No Changes
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 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
build-linux Workflow: build-linux
Details
electron-arm-testing Build #20190330.1 succeeded
Details
electron-arm64-testing Build #20190330.1 succeeded
Details
lint Workflow: lint
Details
release-notes Release notes found
@release-clerk

This comment has been minimized.

Copy link

commented Mar 31, 2019

Release Notes Persisted

DevTools Extensions now load correctly

@MarshallOfSound MarshallOfSound deleted the fix-extensions branch Mar 31, 2019

@dsabanin

This comment has been minimized.

Copy link

commented Apr 1, 2019

Could you make a new 5 beta build with that change? That'd be very handy as we weren't able to give any betas a fair try with all our devtools being broken.

@MarshallOfSound

This comment has been minimized.

Copy link
Member Author

commented Apr 1, 2019

@dsabanin The 5-0-x backport of this fix can be monitored here --> #17616

@trop

This comment has been minimized.

Copy link
Contributor

commented Apr 2, 2019

A maintainer has manually backported this PR to "5-0-x", please check out #17616

Kiku-Reise added a commit to Kiku-Reise/electron that referenced this pull request May 16, 2019

@ErickTamayo

This comment has been minimized.

Copy link

commented Jun 1, 2019

Will this get to 6 and 7?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can鈥檛 perform that action at this time.