fix: ensure ready-to-show event is fired (electron#25632)

CezaryKulakowski · web-flow · commit b85195ee5fd8 · 2020-09-28T13:36:45.000-07:00
diff --git a/lib/browser/api/web-contents.ts b/lib/browser/api/web-contents.ts
@@ -615,6 +615,15 @@ WebContents.prototype._init = function () {
     app.emit('login', event, this, ...args);
   });
 
+  this.on('ready-to-show' as any, () => {
+    const owner = this.getOwnerBrowserWindow();
+    if (owner && !owner.isDestroyed()) {
+      process.nextTick(() => {
+        owner.emit('ready-to-show');
+      });
+    }
+  });
+
   const event = process._linkedBinding('electron_browser_event').createEmpty();
   app.emit('web-contents-created', event, this);
 
diff --git a/shell/browser/api/electron_api_browser_window.cc b/shell/browser/api/electron_api_browser_window.cc
@@ -152,35 +152,6 @@ void BrowserWindow::DidFirstVisuallyNonEmptyPaint() {
   auto* const view = web_contents()->GetRenderWidgetHostView();
   view->Show();
   view->SetSize(window()->GetContentSize());
-
-  // Emit the ReadyToShow event in next tick in case of pending drawing work.
-  base::ThreadTaskRunnerHandle::Get()->PostTask(
-      FROM_HERE, base::BindOnce(
-                     [](base::WeakPtr<BrowserWindow> self) {
-                       if (self && !self->did_ready_to_show_fired_) {
-                         self->did_ready_to_show_fired_ = true;
-                         self->Emit("ready-to-show");
-                       }
-                     },
-                     GetWeakPtr()));
-}
-
-void BrowserWindow::DidFinishLoad(content::RenderFrameHost* render_frame_host,
-                                  const GURL& validated_url) {
-  // The DidFirstVisuallyNonEmptyPaint event is not very stable that, sometimes
-  // on some machines it might not be fired, and the actual behavior depends on
-  // the version of Chromium.
-  // To work around this bug, we ensure the ready-to-show event is emitted if it
-  // has not been emitted in did-finish-load event.
-  // Note that we use did-finish-load event instead of dom-ready event because
-  // the latter may actually be emitted before the ready-to-show event.
-  // See also https://github.com/electron/electron/issues/7779.
-  if (window()->IsVisible() || did_ready_to_show_fired_)
-    return;
-  if (render_frame_host->GetParent())  // child frame
-    return;
-  did_ready_to_show_fired_ = true;
-  Emit("ready-to-show");
 }
 
 void BrowserWindow::BeforeUnloadDialogCancelled() {
diff --git a/shell/browser/api/electron_api_browser_window.h b/shell/browser/api/electron_api_browser_window.h
@@ -49,8 +49,6 @@ class BrowserWindow : public BaseWindow,
                              content::RenderViewHost* new_host) override;
   void RenderViewCreated(content::RenderViewHost* render_view_host) override;
   void DidFirstVisuallyNonEmptyPaint() override;
-  void DidFinishLoad(content::RenderFrameHost* render_frame_host,
-                     const GURL& validated_url) override;
   void BeforeUnloadDialogCancelled() override;
   void OnRendererUnresponsive(content::RenderProcessHost*) override;
   void OnRendererResponsive(
@@ -121,8 +119,6 @@ class BrowserWindow : public BaseWindow,
   // it should be cancelled when we can prove that the window is responsive.
   base::CancelableClosure window_unresponsive_closure_;
 
-  bool did_ready_to_show_fired_ = false;
-
 #if defined(OS_MAC)
   std::vector<mojom::DraggableRegionPtr> draggable_regions_;
 #endif
diff --git a/shell/browser/api/electron_api_web_contents.cc b/shell/browser/api/electron_api_web_contents.cc
@@ -1240,6 +1240,12 @@ void WebContents::Invoke(bool internal,
                  std::move(callback), internal, channel, std::move(arguments));
 }
 
+void WebContents::OnFirstNonEmptyLayout() {
+  if (receivers_.current_context() == web_contents()->GetMainFrame()) {
+    Emit("ready-to-show");
+  }
+}
+
 void WebContents::ReceivePostMessage(const std::string& channel,
                                      blink::TransferableMessage message) {
   v8::Isolate* isolate = JavascriptEnvironment::GetIsolate();
diff --git a/shell/browser/api/electron_api_web_contents.h b/shell/browser/api/electron_api_web_contents.h
@@ -631,6 +631,7 @@ class WebContents : public gin::Wrappable<WebContents>,
               const std::string& channel,
               blink::CloneableMessage arguments,
               InvokeCallback callback) override;
+  void OnFirstNonEmptyLayout() override;
   void ReceivePostMessage(const std::string& channel,
                           blink::TransferableMessage message) override;
   void MessageSync(bool internal,
diff --git a/shell/common/api/api.mojom b/shell/common/api/api.mojom
@@ -49,6 +49,10 @@ interface ElectronBrowser {
       string channel,
       blink.mojom.CloneableMessage arguments) => (blink.mojom.CloneableMessage result);
 
+  // Informs underlying WebContents that first non-empty layout was performed
+  // by compositor.
+  OnFirstNonEmptyLayout();
+
   ReceivePostMessage(string channel, blink.mojom.TransferableMessage message);
 
   // Emits an event on |channel| from the ipcMain JavaScript object in the main
diff --git a/shell/renderer/electron_render_frame_observer.cc b/shell/renderer/electron_render_frame_observer.cc
@@ -128,6 +128,16 @@ void ElectronRenderFrameObserver::OnDestruct() {
   delete this;
 }
 
+void ElectronRenderFrameObserver::DidMeaningfulLayout(
+    blink::WebMeaningfulLayout layout_type) {
+  if (layout_type == blink::WebMeaningfulLayout::kVisuallyNonEmpty) {
+    mojo::Remote<mojom::ElectronBrowser> browser_remote;
+    render_frame_->GetRemoteInterfaces()->GetInterface(
+        browser_remote.BindNewPipeAndPassReceiver());
+    browser_remote->OnFirstNonEmptyLayout();
+  }
+}
+
 void ElectronRenderFrameObserver::CreateIsolatedWorldContext() {
   auto* frame = render_frame_->GetWebFrame();
   blink::WebIsolatedWorldInfo info;
diff --git a/shell/renderer/electron_render_frame_observer.h b/shell/renderer/electron_render_frame_observer.h
@@ -33,6 +33,7 @@ class ElectronRenderFrameObserver : public content::RenderFrameObserver {
   void WillReleaseScriptContext(v8::Local<v8::Context> context,
                                 int world_id) override;
   void OnDestruct() override;
+  void DidMeaningfulLayout(blink::WebMeaningfulLayout layout_type) override;
 
  private:
   bool ShouldNotifyClient(int world_id);
