[CLEAN] Synthetic Benchmark PR #48762 - refactor: make api::WebRequest inherit from gin::Wrappable

[tomerqodo]

User description

Benchmark PR electron#48762

Type: Clean (correct implementation)

Original PR Title: refactor: make api::WebRequest inherit from gin::Wrappable
Original PR Description: #### Description of Change

Part of electron#47922

• Refactor electron::api::WebRequest to inherit from gin::Wrappable instead of gin_helper::DeprecatedWrappable
• Refactor electron::api::Session::web_request_ to be a cppgc::Member<WebRequest> instead of a v8::TracedReference<v8::Value>
• Remove unused v8::Isolate* argument from electron::api::WebRequest constructor
• Modify WebRequest::Create(), WebRequest::Find(), and WebRequest::FindOrCreate() to return an api::WebRequest*

CC @deepak1556 as a electron#47922 stakeholder. All reviews welcomed.

Checklist

• PR description included and stakeholders cc'd
• npm test passes
• PR release notes describe the change in a way relevant to app developers, and are capitalized, punctuated, and past tense.

Release Notes

Notes: none
Original PR URL: electron#48762

---

PR Type

Enhancement

---

Description

• Refactor WebRequest to inherit from gin::Wrappable instead of gin_helper::DeprecatedWrappable

• Change web_request_ storage from v8::TracedReference to cppgc::Member for garbage collection

• Update WebRequest::Create() and related methods to return raw pointers instead of handles

• Implement proper garbage collection tracing with cppgc::WeakPersistent for dependent objects

---

Diagram Walkthrough

flowchart LR
  A["WebRequest<br/>DeprecatedWrappable"] -->|refactor| B["WebRequest<br/>gin::Wrappable"]
  C["v8::TracedReference<br/>web_request_"] -->|change storage| D["cppgc::Member<br/>web_request_"]
  E["gin_helper::Handle<br/>return type"] -->|simplify| F["raw pointer<br/>return type"]
  B -->|enable| G["cppgc garbage<br/>collection"]
  D -->|support| G
  F -->|simplify| G

Loading

File Walkthrough

	Relevant files
Enhancement	8 files electron_api_web_request.h Change base class and storage mechanism for WebRequest      +21/-13  electron_api_web_request.cc Implement gin::Wrappable interface and garbage collection +27/-21  electron_api_session.h Update WebRequest storage to cppgc::Member                              +3/-2      electron_api_session.cc Simplify WebRequest getter to return raw pointer                  +4/-6      electron_browser_client.cc Update WebRequest usage to work with raw pointers                +18/-12  proxying_url_loader_factory.h Change web_request_ to cppgc::WeakPersistent                          +2/-3      proxying_url_loader_factory.cc Update web_request_ member access for new storage type      +16/-16  proxying_websocket.h Change web_request_ to cppgc::WeakPersistent                          +2/-1
Configuration changes	1 files chore_add_electron_objects_to_wrappablepointertag.patch Add kElectronWebRequest to wrappable pointer tags                +4/-3

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢	No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: No audit logs: New networking interception paths (e.g., calls to web_request_->OnBeforeRequest, OnCompleted, OnErrorOccurred) add/modify critical request handling without any added logging of actions, actors, or outcomes to an audit trail. Referred Code // In this case we do nothing because extensions should see nothing. return; } else { continuation = base::BindRepeating(&InProgressRequest::ContinueToBeforeSendHeaders, weak_factory_.GetWeakPtr()); } redirect_url_ = GURL(); int result = factory_->web_request_->OnBeforeRequest( &info_.value(), request_, continuation, &redirect_url_); Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Weak callback change: The change from base::Unretained(this) to a weak persistent (weak_factory_) in HandleOnBeforeRequestResponseEvent improves safety but may introduce null-target scenarios without explicit fallback handling or logging if the object is collected before the callback runs. Referred Code auto& allocation_handle = isolate->GetCppHeap()->GetAllocationHandle(); ResponseCallback response = base::BindOnce( &WebRequest::OnBeforeRequestListenerResult, gin::WrapPersistent(weak_factory_.GetWeakCell(allocation_handle)), request_info->id); info.listener.Run(gin::ConvertToV8(isolate, details), std::move(response)); Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Remove incorrect tracing of non-GC object Remove the incorrect tracing of weak_factory_ from the Trace method, as it is not a garbage-collected object. shell/browser/api/electron_api_web_request.cc [354-357] void WebRequest::Trace(cppgc::Visitor* visitor) const { gin::Wrappable<WebRequest>::Trace(visitor); - visitor->Trace(weak_factory_); } Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that weak_factory_ is not a garbage-collected object and should not be traced, preventing a compilation error introduced in the PR.	High
	Fix incorrect usage of base::BindOnce Fix the incorrect usage of base::BindOnce by using a lambda to correctly capture and resolve the weak pointer to the garbage-collected WebRequest object before calling the member function. shell/browser/api/electron_api_web_request.cc [394-398] auto& allocation_handle = isolate->GetCppHeap()->GetAllocationHandle(); +cppgc::WeakPersistent<WebRequest> weak_this = + weak_factory_.GetWeakCell(allocation_handle); ResponseCallback response = base::BindOnce( - &WebRequest::OnBeforeRequestListenerResult, - gin::WrapPersistent(weak_factory_.GetWeakCell(allocation_handle)), - request_info->id); + [](cppgc::WeakPersistent<WebRequest> weak_this, uint64_t id, + gin_helper::Dictionary result) { + if (weak_this) { + weak_this->OnBeforeRequestListenerResult(id, std::move(result)); + } + }, + weak_this, request_info->id); Apply / Chat Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies a type mismatch in base::BindOnce that would cause a compilation error and provides a valid fix using a lambda to correctly handle the weak pointer to the garbage-collected object.	High
More