Browse Source

fix: RenderFrameHost nullptr dereference (#45510)

* fix: add nullptr tests before using render_frame_

Co-authored-by: Charles Kerr <[email protected]>

* refactor: extract-method HasRenderFrame()

Co-authored-by: Charles Kerr <[email protected]>

---------

Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com>
Co-authored-by: Charles Kerr <[email protected]>
trop[bot] 2 months ago
parent
commit
bba618f4d3

+ 3 - 3
shell/browser/api/electron_api_web_frame_main.cc

@@ -184,7 +184,7 @@ void WebFrameMain::UpdateRenderFrameHost(content::RenderFrameHost* rfh) {
 }
 
 bool WebFrameMain::CheckRenderFrame() const {
-  if (render_frame_disposed_) {
+  if (!HasRenderFrame()) {
     v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
     v8::HandleScope scope(isolate);
     gin_helper::ErrorThrower(isolate).ThrowError(
@@ -436,7 +436,7 @@ v8::Local<v8::Promise> WebFrameMain::CollectDocumentJSCallStack(
   gin_helper::Promise<base::Value> promise(args->isolate());
   v8::Local<v8::Promise> handle = promise.GetHandle();
 
-  if (render_frame_disposed_) {
+  if (!HasRenderFrame()) {
     promise.RejectWithErrorMessage(
         "Render frame was disposed before WebFrameMain could be accessed");
     return handle;
@@ -464,7 +464,7 @@ void WebFrameMain::CollectedJavaScriptCallStack(
     gin_helper::Promise<base::Value> promise,
     const std::string& untrusted_javascript_call_stack,
     const std::optional<blink::LocalFrameToken>& remote_frame_token) {
-  if (render_frame_disposed_) {
+  if (!HasRenderFrame()) {
     promise.RejectWithErrorMessage(
         "Render frame was disposed before call stack was received");
     return;

+ 8 - 2
shell/browser/api/electron_api_web_frame_main.h

@@ -101,8 +101,14 @@ class WebFrameMain final : public gin::Wrappable<WebFrameMain>,
   void TeardownMojoConnection();
   void OnRendererConnectionError();
 
-  // WebFrameMain can outlive its RenderFrameHost pointer so we need to check
-  // whether its been disposed of prior to accessing it.
+  [[nodiscard]] constexpr bool HasRenderFrame() const {
+    return !render_frame_disposed_ && render_frame_ != nullptr;
+  }
+
+  // Throws a JS error if HasRenderFrame() is false.
+  // WebFrameMain can outlive its RenderFrameHost pointer,
+  // so we need to check whether its been disposed of
+  // prior to accessing it.
   bool CheckRenderFrame() const;
 
   v8::Local<v8::Promise> ExecuteJavaScript(gin::Arguments* args,