Fix nullptr deref in maybeRenderFallbackContent()

content/renderer/render_frame_impl.cc

Index: content/renderer/render_frame_impl.cc
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index f67f5c53fe22700e41cf783b81aad5be84e4d395..a24bbc1b4441677d33c11fd30723e545d361f7ee 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -5258,7 +5258,15 @@ void RenderFrameImpl::OnFailedNavigation(
 
   // On load failure, a frame can ask its owner to render fallback content.
   // When that happens, don't load an error page.
+  bool was_loading = frame_->IsLoading();
   if (frame_->MaybeRenderFallbackContent(error)) {
+    // If the frame wasn't loading but was fallback-elligible, the fallback

[Charlie Reis, 2017/04/11 21:08:40]

nit: eligible

+    // content won't be shown. However, showing an error page isn't right
+    // either, as the frame has already been populated with something unrelated
+    // to this navigation failure. In that case, just send a stop IPC to the
+    // browser to unwind its state, and leave the frame as-is.

[Charlie Reis, 2017/04/11 21:08:40]

Sorry, I'm getting confused by this comment and method. 
MaybeRenderFallbackContent sounds like the kind of method that would return true
if it actually rendered the fallback content, but now it apparently returns true
even when it doesn't render the fallback in some cases.  (I'm also having
trouble reading the comment on the declaration of MaybeRenderFallbackContent,
since it might have a typo in it?)

Can we rename or restructure this (e.g., into two separate calls?) to make it
clearer?

[Nate Chapin, 2017/04/12 21:58:01]

I don't really like having two separate public/ API calls for this, so I changed
this to return an enum, which should give sufficient infromation for
RenderFrameImpl to do the right thing. Is that OK with you?

I think MaybeRenderFallbackContent is still an ok name with the enum return
value, let me know if you feel otherwise.

+    if (!was_loading)
+      Send(new FrameHostMsg_DidStopLoading(routing_id_));
     browser_side_navigation_pending_ = false;
     return;
   }