-
Notifications
You must be signed in to change notification settings - Fork 3
Complete CDP domain proxy system with IPC channels, async communication, RAII memory management, robust conditional compilation support, Log domain implementation, real-time event broadcasting, instance-based communication architecture, singleton client-side logging system, and improved code organization #214
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
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
| pendingRequests_[requestId] = move(pendingRequest); | ||
| } | ||
|
|
||
| // TODO: Send request to content process via inspector IPC channel |
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.
@copilot Please finish the TODO as you described, but you need to introduce an async method to respond to client.
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.
I've implemented the async CDP forwarding system as requested. The new implementation includes:
- Async method: Added
forwardRequestAsync()with callback support - Timeout handling: 30-second timeout with proper cleanup
- Enhanced tracking: PendingRequest struct now supports both sync and async flows
- Error handling: Comprehensive error responses for failed requests
The synchronous forwardRequest() method now internally uses the async version with a condition variable for backward compatibility. The system is ready for full IPC integration when inspector channels are added to TrContentRuntime.
Commit: 3059b13
|
|
||
| TrContentRuntime *ContentDomainProxy::findContentRuntime(const string &clientId) | ||
| { | ||
| // TODO: Implement proper content runtime lookup |
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.
Please implement this or you can call to the ContentManager directly.
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.
I've implemented proper content runtime lookup using the ContentManager as suggested. The new findContentRuntime() method:
- Uses ContentManager: Calls
constellation_->contentManager->getContent()to find content runtimes - Validates client ID: Converts clientId to content ID with proper error handling
- Checks availability: Verifies the content process is running and not being destroyed
- Comprehensive logging: Added debug logs for troubleshooting
The implementation now properly integrates with the TrConstellation/ContentManager architecture instead of returning null.
Commit: 3059b13
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
yorkie
left a comment
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.
@copilot add the ipc channels related code to implement send request to client/inspector and respond reversely.
| return false; | ||
| } | ||
|
|
||
| // TODO: This needs to be implemented when inspector IPC channels are added to TrContentRuntime |
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.
@copilot Implement this TODO
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.
I've implemented the TODO at line 144. The sendCdpRequestToContent method now:
- Creates a real
TrCdpRequestcommand with the CDP message JSON - Sends it to the content process via the inspector IPC channel using
contentRuntime->sendInspectorCommand() - Handles errors properly if the IPC send fails
- Removed all simulation logic
The content processes receive these requests and respond with TrCdpResponse messages that are routed back to the correct proxy instance.
Commit: adc9fff
… processes Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
I've implemented the IPC channels related code for CDP communication between host and content processes. The implementation includes:
The system now supports full bidirectional CDP communication with async forwarding and proper content process lookup. Commit: adc9fff |
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
yorkie
left a comment
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.
@copilot You also need to create inspector channel receiver and sender at content process(TrClientContextPerProcess), and initialize a WorkerThread for processing the incoming inspector requests.
I've implemented the inspector channel receiver and sender at the content process (TrClientContextPerProcess) with a WorkerThread for processing incoming inspector requests. The implementation includes: Content Process Side (TrClientContextPerProcess):
Host Process Side:
Full Communication Flow:
The content processes now receive CDP requests asynchronously in their main tick loop and respond with appropriate CDP responses back to the host process. Commit: 6f4d12e |
…t processing Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
…rr fallback, namespace and convenience functions Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
|
@copilot Update for my reviews. |
Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
…reorder includes, encapsulate inspector fields, configure domains in common Co-authored-by: yorkie <1935767+yorkie@users.noreply.github.com>
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.
Pull Request Overview
This PR implements a comprehensive Chrome DevTools Protocol (CDP) domain proxy system that transforms the JSAR Runtime inspector from a simulation-based system to a real IPC-based communication architecture. The implementation adds bidirectional CDP communication between host and content processes, real-time event broadcasting, modern C++ memory management, and conditional compilation support.
Key changes include:
- Complete IPC-based CDP communication replacing simulation code with actual inter-process messaging
- Real-time event broadcasting system that sends CDP events from content processes to all connected DevTools clients
- Comprehensive Log domain implementation with singleton client-side logging system for automatic DevTools integration
Reviewed Changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/runtime/inspector/*.{hpp,cpp} | Host-side inspector architecture with IPC channels, content domain proxy, and async CDP processing |
| src/common/inspector/*.hpp | Shared IPC message structures and CDP utilities for communication between processes |
| src/client/inspector/*.{hpp,cpp} | Content-side inspector implementation with domain handlers and real-time event broadcasting |
| src/client/logger.{hpp,cpp} | Singleton logging system with automatic CDP Log domain integration |
| src/runtime/{content,content_manager,hive_daemon}.{hpp,cpp} | Inspector IPC channel integration and conditional compilation support |
| docs/internals/CDP_SUPPORT.md | Comprehensive documentation of the new CDP system architecture |
This PR implements a complete Chrome DevTools Protocol (CDP) domain proxy system with full IPC-based communication between host and content processes, eliminating simulation code and adding comprehensive async support with modern C++ memory management, robust conditional compilation support, complete Log domain implementation, real-time event broadcasting to DevTools clients, instance-based communication architecture, singleton client-side logging system, and improved code organization through encapsulation.
Key Implementation Features
🔄 Real IPC Communication
inspectorCommandChanClient,inspectorCommandChanReceiver, andinspectorCommandChanSenderto bothTrContentRuntime(host) andTrClientContextPerProcess(content)TrCdpRequestcommands and receiveTrCdpResponsemessages via actual IPC channels⚡ Async Architecture & Real-time Event Broadcasting
processMessageAsync()method that eliminates synchronous waiting for content process responsesTrCdpEventIPC messages that are broadcast to all connected DevTools clients viaTrInspector::broadcastEventToClients()TrCdpEventIPC → Host process → Broadcast to all CDP clients🎯 Modern C++ Memory Management & Domain Dispatch
unique_ptrfor automatic memory management, following codebase conventions consistently across host and client processesContentCdpHandlerinfrastructure for proper method extraction and domain-specific processing🛡️ Robust Conditional Compilation & Shared Components
TR_ENABLE_INSPECTORpreprocessor directives for optional inspector supportCdpMessage,CdpResponsetosrc/common/inspector/cdp_message.hppas header-only implementations shared between runtime and client processes📝 Complete CDP Log Domain Implementation
enable,disable,clear,startViolationsReport,stopViolationsReportLogEntrystructure supporting source, level, text, timestamp, and optional metadata (URL, line numbers, stack traces)Log.entryAddedevent broadcasting when entries are added or domain is enabledlongTask,longLayout,blockedEvent,blockedParser, etc.🔧 Singleton Client-side Logging System
logging::Logger::GetInstance()provides singleton access throughout the applicationLogInfo(),LogJsError()for easy application loggingstd::cout/std::cerroutput🏗️ Improved Code Organization & Encapsulation
TrClientContextPerProcessTrContentRuntimesrc/common/inspector/cdp_domains.hppfor consistent use across runtime and client processesTechnical Architecture
The system implements a complete multi-process CDP communication architecture with clean separation of concerns:
TrCdpRequest,TrCdpResponse, andTrCdpEventmessagesSupported Domains
Runtime.enable,Runtime.getVersion)DevTools Integration
Applications can now use the singleton logger for immediate DevTools visibility:
Log entries appear immediately in Chrome DevTools Console with proper categorization, timestamps, and metadata.
Code Quality Improvements
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.