Ensure that a document has been created before sending IPC messages

* Reverts 370476c4af in favor of moving the previous logic to the new RenderFrameObserver instead of RenderViewObserver

Fixes #12045

atom/renderer/atom_render_frame_observer.cc

@@ -6,27 +6,70 @@
 
 #include <vector>
 
+#include "atom/common/native_mate_converters/string16_converter.h"
+
 #include "atom/common/api/api_messages.h"
+#include "atom/common/api/event_emitter_caller.h"
+#include "atom/common/native_mate_converters/value_converter.h"
+#include "atom/common/node_includes.h"
+#include "atom/renderer/atom_renderer_client.h"
+#include "base/command_line.h"
+#include "base/strings/string_number_conversions.h"
+#include "base/trace_event/trace_event.h"
 #include "content/public/renderer/render_frame.h"
 #include "content/public/renderer/render_view.h"
+#include "native_mate/dictionary.h"
 #include "third_party/WebKit/public/web/WebDocument.h"
 #include "third_party/WebKit/public/web/WebDraggableRegion.h"
+#include "third_party/WebKit/public/web/WebElement.h"
+#include "third_party/WebKit/public/web/WebFrame.h"
+#include "third_party/WebKit/public/web/WebKit.h"
 #include "third_party/WebKit/public/web/WebLocalFrame.h"
 #include "third_party/WebKit/public/web/WebScriptSource.h"
+#include "third_party/WebKit/public/web/WebView.h"
 
 namespace atom {
 
+bool GetIPCObject(v8::Isolate* isolate,
+                  v8::Local<v8::Context> context,
+                  v8::Local<v8::Object>* ipc) {
+  v8::Local<v8::String> key = mate::StringToV8(isolate, "ipc");
+  v8::Local<v8::Private> privateKey = v8::Private::ForApi(isolate, key);
+  v8::Local<v8::Object> global_object = context->Global();
+  v8::Local<v8::Value> value;
+  if (!global_object->GetPrivate(context, privateKey).ToLocal(&value))
+    return false;
+  if (value.IsEmpty() || !value->IsObject())
+    return false;
+  *ipc = value->ToObject();
+  return true;
+}
+
+std::vector<v8::Local<v8::Value>> ListValueToVector(
+    v8::Isolate* isolate,
+    const base::ListValue& list) {
+  v8::Local<v8::Value> array = mate::ConvertToV8(isolate, list);
+  std::vector<v8::Local<v8::Value>> result;
+  mate::ConvertFromV8(isolate, array, &result);
+  return result;
+}
+
 AtomRenderFrameObserver::AtomRenderFrameObserver(
     content::RenderFrame* frame,
     RendererClientBase* renderer_client)
   : content::RenderFrameObserver(frame),
     render_frame_(frame),
-    renderer_client_(renderer_client) {}
+    renderer_client_(renderer_client),
+    document_created_(false) {}
 
 void AtomRenderFrameObserver::DidClearWindowObject() {
   renderer_client_->DidClearWindowObject(render_frame_);
 }
 
+void AtomRenderFrameObserver::DidCreateDocumentElement() {
+  document_created_ = true;
+}
+
 void AtomRenderFrameObserver::DidCreateScriptContext(
     v8::Handle<v8::Context> context,
     int world_id) {
@@ -99,4 +142,78 @@ bool AtomRenderFrameObserver::ShouldNotifyClient(int world_id) {
     return IsMainWorld(world_id);
 }
 
+bool AtomRenderFrameObserver::OnMessageReceived(const IPC::Message& message) {
+  bool handled = true;
+  IPC_BEGIN_MESSAGE_MAP(AtomRenderFrameObserver, message)
+    IPC_MESSAGE_HANDLER(AtomViewMsg_Message, OnBrowserMessage)
+    IPC_MESSAGE_HANDLER(AtomViewMsg_Offscreen, OnOffscreen)
+    IPC_MESSAGE_UNHANDLED(handled = false)
+  IPC_END_MESSAGE_MAP()
+
+  return handled;
+}
+
+void AtomRenderFrameObserver::OnOffscreen() {
+  blink::WebView::SetUseExternalPopupMenus(false);
+}
+
+void AtomRenderFrameObserver::OnBrowserMessage(bool send_to_all,
+                                              const base::string16& channel,
+                                              const base::ListValue& args) {
+  // Don't handle browser messages before document element is created.
+  // When we receive a message from the browser, we try to transfer it
+  // to a web page, and when we do that Blink creates an empty
+  // document element if it hasn't been created yet, and it makes our init
+  // script to run while `window.location` is still "about:blank".
+  if (!document_created_)
+    return;
+
+  if (!render_frame()->GetRenderView()->GetWebView())
+    return;
+
+  blink::WebFrame* frame = render_frame()->GetRenderView()->GetWebView()->MainFrame();
+  if (!frame || !frame->IsWebLocalFrame())
+    return;
+
+  EmitIPCEvent(frame->ToWebLocalFrame(), channel, args);
+
+  // Also send the message to all sub-frames.
+  if (send_to_all) {
+    for (blink::WebFrame* child = frame->FirstChild(); child;
+         child = child->NextSibling())
+      if (child->IsWebLocalFrame()) {
+        EmitIPCEvent(child->ToWebLocalFrame(), channel, args);
+      }
+  }
+}
+
+void AtomRenderFrameObserver::EmitIPCEvent(blink::WebLocalFrame* frame,
+                                          const base::string16& channel,
+                                          const base::ListValue& args) {
+  if (!frame)
+    return;
+
+  v8::Isolate* isolate = blink::MainThreadIsolate();
+  v8::HandleScope handle_scope(isolate);
+
+  v8::Local<v8::Context> context = renderer_client_->GetContext(frame, isolate);
+  v8::Context::Scope context_scope(context);
+
+  // Only emit IPC event for context with node integration.
+  node::Environment* env = node::Environment::GetCurrent(context);
+  if (!env)
+    return;
+
+  v8::Local<v8::Object> ipc;
+  if (GetIPCObject(isolate, context, &ipc)) {
+    TRACE_EVENT0("devtools.timeline", "FunctionCall");
+    auto args_vector = ListValueToVector(isolate, args);
+    // Insert the Event object, event.sender is ipc.
+    mate::Dictionary event = mate::Dictionary::CreateEmpty(isolate);
+    event.Set("sender", ipc);
+    args_vector.insert(args_vector.begin(), event.GetHandle());
+    mate::EmitEvent(isolate, ipc, channel, args_vector);
+  }
+}
+
 }  // namespace atom

atom/renderer/atom_render_frame_observer.h

@@ -6,6 +6,7 @@
 #define ATOM_RENDERER_ATOM_RENDER_FRAME_OBSERVER_H_
 
 #include "atom/renderer/renderer_client_base.h"
+#include "base/values.h"
 #include "content/public/renderer/render_frame_observer.h"
 
 namespace atom {
@@ -31,15 +32,28 @@ class AtomRenderFrameObserver : public content::RenderFrameObserver {
   void WillReleaseScriptContext(v8::Local<v8::Context> context,
                                 int world_id) override;
   void OnDestruct() override;
+  bool OnMessageReceived(const IPC::Message& message) override;
+  void DidCreateDocumentElement() override;
+
+ protected:
+  virtual void EmitIPCEvent(blink::WebLocalFrame* frame,
+                            const base::string16& channel,
+                            const base::ListValue& args);
 
  private:
   bool ShouldNotifyClient(int world_id);
   void CreateIsolatedWorldContext();
   bool IsMainWorld(int world_id);
   bool IsIsolatedWorld(int world_id);
+  void OnBrowserMessage(bool send_to_all,
+                        const base::string16& channel,
+                        const base::ListValue& args);
+
+  void OnOffscreen();
 
   content::RenderFrame* render_frame_;
   RendererClientBase* renderer_client_;
+  bool document_created_;
 
   DISALLOW_COPY_AND_ASSIGN(AtomRenderFrameObserver);
 };

atom/renderer/atom_render_view_observer.cc

@@ -35,30 +35,6 @@ namespace atom {
 
 namespace {
 
-bool GetIPCObject(v8::Isolate* isolate,
-                  v8::Local<v8::Context> context,
-                  v8::Local<v8::Object>* ipc) {
-  v8::Local<v8::String> key = mate::StringToV8(isolate, "ipc");
-  v8::Local<v8::Private> privateKey = v8::Private::ForApi(isolate, key);
-  v8::Local<v8::Object> global_object = context->Global();
-  v8::Local<v8::Value> value;
-  if (!global_object->GetPrivate(context, privateKey).ToLocal(&value))
-    return false;
-  if (value.IsEmpty() || !value->IsObject())
-    return false;
-  *ipc = value->ToObject();
-  return true;
-}
-
-std::vector<v8::Local<v8::Value>> ListValueToVector(
-    v8::Isolate* isolate,
-    const base::ListValue& list) {
-  v8::Local<v8::Value> array = mate::ConvertToV8(isolate, list);
-  std::vector<v8::Local<v8::Value>> result;
-  mate::ConvertFromV8(isolate, array, &result);
-  return result;
-}
-
 base::StringPiece NetResourceProvider(int key) {
   if (key == IDR_DIR_HEADER_HTML) {
     base::StringPiece html_data =
@@ -74,8 +50,7 @@ base::StringPiece NetResourceProvider(int key) {
 AtomRenderViewObserver::AtomRenderViewObserver(
     content::RenderView* render_view,
     AtomRendererClient* renderer_client)
-    : content::RenderViewObserver(render_view),
-      renderer_client_(renderer_client) {
+    : content::RenderViewObserver(render_view) {
   // Initialise resource for directory listing.
   net::NetModule::SetResourceProvider(NetResourceProvider);
 }
@@ -83,85 +58,8 @@ AtomRenderViewObserver::AtomRenderViewObserver(
 AtomRenderViewObserver::~AtomRenderViewObserver() {
 }
 
-void AtomRenderViewObserver::EmitIPCEvent(blink::WebLocalFrame* frame,
-                                          const base::string16& channel,
-                                          const base::ListValue& args) {
-  if (!frame)
-    return;
-
-  v8::Isolate* isolate = blink::MainThreadIsolate();
-  v8::HandleScope handle_scope(isolate);
-
-  v8::Local<v8::Context> context = renderer_client_->GetContext(frame, isolate);
-  v8::Context::Scope context_scope(context);
-
-  // Only emit IPC event for context with node integration.
-  node::Environment* env = node::Environment::GetCurrent(context);
-  if (!env)
-    return;
-
-  v8::Local<v8::Object> ipc;
-  if (GetIPCObject(isolate, context, &ipc)) {
-    TRACE_EVENT0("devtools.timeline", "FunctionCall");
-    auto args_vector = ListValueToVector(isolate, args);
-    // Insert the Event object, event.sender is ipc.
-    mate::Dictionary event = mate::Dictionary::CreateEmpty(isolate);
-    event.Set("sender", ipc);
-    args_vector.insert(args_vector.begin(), event.GetHandle());
-    mate::EmitEvent(isolate, ipc, channel, args_vector);
-  }
-}
-
-bool AtomRenderViewObserver::OnMessageReceived(const IPC::Message& message) {
-  bool handled = true;
-  IPC_BEGIN_MESSAGE_MAP(AtomRenderViewObserver, message)
-    IPC_MESSAGE_HANDLER(AtomViewMsg_Message, OnBrowserMessage)
-    IPC_MESSAGE_HANDLER(AtomViewMsg_Offscreen, OnOffscreen)
-    IPC_MESSAGE_UNHANDLED(handled = false)
-  IPC_END_MESSAGE_MAP()
-
-  return handled;
-}
-
 void AtomRenderViewObserver::OnDestruct() {
   delete this;
 }
 
-void AtomRenderViewObserver::OnBrowserMessage(bool send_to_all,
-                                              const base::string16& channel,
-                                              const base::ListValue& args) {
-  if (!render_view()->GetWebView())
-    return;
-
-  blink::WebFrame* frame = render_view()->GetWebView()->MainFrame();
-  if (!frame || !frame->IsWebLocalFrame())
-    return;
-
-  // Don't handle browser messages before document element is created.
-  // When we receive a message from the browser, we try to transfer it
-  // to a web page, and when we do that Blink creates an empty
-  // document element if it hasn't been created yet, and it makes our init
-  // script to run while `window.location` is still "about:blank".
-  blink::WebDocument document = frame->ToWebLocalFrame()->GetDocument();
-  blink::WebElement html_element = document.DocumentElement();
-  if (html_element.IsNull()) {
-    return;
-  }
-
-  EmitIPCEvent(frame->ToWebLocalFrame(), channel, args);
-
-  // Also send the message to all sub-frames.
-  if (send_to_all) {
-    for (blink::WebFrame* child = frame->FirstChild(); child;
-         child = child->NextSibling())
-      if (child->IsWebLocalFrame()) {
-        EmitIPCEvent(child->ToWebLocalFrame(), channel, args);
-      }
-  }
-}
-
-void AtomRenderViewObserver::OnOffscreen() {
-  blink::WebView::SetUseExternalPopupMenus(false);
-}
-
 }  // namespace atom

atom/renderer/atom_render_view_observer.h

@@ -25,23 +25,9 @@ class AtomRenderViewObserver : public content::RenderViewObserver {
  protected:
   virtual ~AtomRenderViewObserver();
 
-  virtual void EmitIPCEvent(blink::WebLocalFrame* frame,
-                            const base::string16& channel,
-                            const base::ListValue& args);
-
  private:
-  // content::RenderViewObserver implementation.
-  bool OnMessageReceived(const IPC::Message& message) override;
   void OnDestruct() override;
 
-  void OnBrowserMessage(bool send_to_all,
-                        const base::string16& channel,
-                        const base::ListValue& args);
-
-  void OnOffscreen();
-
-  AtomRendererClient* renderer_client_;
-
   DISALLOW_COPY_AND_ASSIGN(AtomRenderViewObserver);
 };
 

atom/renderer/atom_sandboxed_renderer_client.cc

@@ -108,9 +108,10 @@ class AtomSandboxedRenderViewObserver : public AtomRenderViewObserver {
     }
 
  protected:
+  // TODO(MarshallOfSound): This needs to be `AtomRenderFrameObserver`
   void EmitIPCEvent(blink::WebLocalFrame* frame,
                     const base::string16& channel,
-                    const base::ListValue& args) override {
+                    const base::ListValue& args) {
     if (!frame)
       return;