-
Notifications
You must be signed in to change notification settings - Fork 85
[MV3] Dart Debug Extension supports cross-extension communication with AngularDart DevTools #1866
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
Conversation
@@ -17,14 +17,17 @@ import 'logger.dart'; | |||
|
|||
enum StorageObject { | |||
debugInfo, | |||
devToolsOpener; | |||
devToolsOpener, | |||
encodedUri; | |||
|
|||
String get keyName { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enum values now have a property called name
which matches this implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nice! Using that instead
@@ -194,14 +198,12 @@ void _routeDwdsEvent(String eventData, SocketClient client, int tabId) { | |||
final message = serializers.deserialize(jsonDecode(eventData)); | |||
if (message is ExtensionRequest) { | |||
_forwardDwdsEventToChromeDebugger(message, client, tabId); | |||
} else if (message is ExtensionEvent) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ExtensionRequest and ExtensionEvent are mutually exclusive, any reason to remove the else
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason, added back in the else if
@@ -6,6 +6,11 @@ | |||
"action": { | |||
"default_icon": "static_assets/dart_dev.png" | |||
}, | |||
"externally_connectable": { | |||
"ids": [ | |||
"nbkbficgbembimioedhceniahniffgpl" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a comment here indicating this is the ACX extension.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No comments allowed in JSON unfortunately
chrome.runtime.onMessage.addListener( | ||
allowInterop(_handleRuntimeMessages), | ||
); | ||
chrome.runtime.onMessageExternal.addListener( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a comment indicating that we only allow the ACX extension to communicate with this extension due to the manifest config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, added one here and in cross_extension_communication
No description provided.