fix: notify focus change right away rather not on next tick (#14453)

* fix: Notify focus change right away, not on next tick

* fix: emit the JS blur/focus events on next tick to avoid race condition

* address feedback from review

* fix: bind deferred Emit() calls to a WeakPtr

This is so that the deferred Emit() calls will be canceled
if the TopLevelWindow is destroyed.

* chore: remove wip/test code cruft

* fix: make linter happy

* Enable disabled tests

* refactor: cleaner impl of EmitEventSoon()

* Revert "Merge branch 'fix-win-focus' of github.com:electron/electron into fix-win-focus"

This reverts commit 90576806eb, reversing
changes made to 9c13e47779.

* Restore 704722c1, which was removed in error.

We apologise again for the fault in the subtitles. Those responsible for sacking the people who have just been sacked have been sacked.

atom/browser/api/atom_api_top_level_window.cc

@@ -163,11 +163,11 @@ void TopLevelWindow::OnWindowEndSession() {
 }
 
 void TopLevelWindow::OnWindowBlur() {
-  Emit("blur");
+  EmitEventSoon("blur");
 }
 
 void TopLevelWindow::OnWindowFocus() {
-  Emit("focus");
+  EmitEventSoon("focus");
 }
 
 void TopLevelWindow::OnWindowShow() {

atom/browser/api/atom_api_top_level_window.h

@@ -14,6 +14,7 @@
 #include "atom/browser/native_window.h"
 #include "atom/browser/native_window_observer.h"
 #include "atom/common/api/atom_api_native_image.h"
+#include "content/public/browser/browser_thread.h"
 #include "native_mate/handle.h"
 
 namespace atom {
@@ -215,6 +216,14 @@ class TopLevelWindow : public mate::TrackableObject<TopLevelWindow>,
   // Remove this window from parent window's |child_windows_|.
   void RemoveFromParentChildWindows();
 
+  template<typename... Args>
+  void EmitEventSoon(base::StringPiece eventName) {
+    content::BrowserThread::PostTask(
+        content::BrowserThread::UI, FROM_HERE,
+        base::BindOnce(base::IgnoreResult(&TopLevelWindow::Emit<Args...>),
+                       weak_factory_.GetWeakPtr(), eventName));
+  }
+
 #if defined(OS_WIN)
   typedef std::map<UINT, MessageCallback> MessageCallbackMap;
   MessageCallbackMap messages_callback_map_;

atom/browser/native_window_views.cc

@@ -1104,12 +1104,10 @@ void NativeWindowViews::OnWidgetActivationChanged(views::Widget* changed_widget,
   if (changed_widget != widget())
     return;
 
-  // Post the notification to next tick.
-  content::BrowserThread::PostTask(
-      content::BrowserThread::UI, FROM_HERE,
-      base::Bind(active ? &NativeWindow::NotifyWindowFocus
-                        : &NativeWindow::NotifyWindowBlur,
-                 GetWeakPtr()));
+  if (active)
+    NativeWindow::NotifyWindowFocus();
+  else
+    NativeWindow::NotifyWindowBlur();
 
   // Hide menu bar when window is blured.
   if (!active && IsMenuBarAutoHide() && IsMenuBarVisible())

spec/api-browser-window-spec.js

@@ -399,8 +399,7 @@ describe('BrowserWindow module', () => {
     })
   })
 
-  // TODO(alexeykuzmin): [Ch66] Enable the test. Passes locally.
-  xdescribe('BrowserWindow.getFocusedWindow()', (done) => {
+  describe('BrowserWindow.getFocusedWindow()', (done) => {
     it('returns the opener window when dev tools window is focused', (done) => {
       w.show()
       w.webContents.once('devtools-focused', () => {
@@ -611,8 +610,7 @@ describe('BrowserWindow module', () => {
     })
   })
 
-  // TODO(alexeykuzmin): [Ch66] Enable the test. Passes locally.
-  xdescribe('BrowserWindow.alwaysOnTop() resets level on minimize', () => {
+  describe('BrowserWindow.alwaysOnTop() resets level on minimize', () => {
     before(function () {
       if (process.platform !== 'darwin') {
         this.skip()
@@ -1817,8 +1815,7 @@ describe('BrowserWindow module', () => {
     })
   })
 
-  // TODO(alexeykuzmin): [Ch66] Enable the tests. They pass locally.
-  xdescribe('document.visibilityState/hidden', () => {
+  describe('document.visibilityState/hidden', () => {
     beforeEach(() => { w.destroy() })
 
     function onVisibilityChange (callback) {
@@ -2421,7 +2418,6 @@ describe('BrowserWindow module', () => {
       })
     })
 
-    // TODO(alexeykuzmin): [Ch66] Enable the test. It passes locally.
     describe('kiosk state', () => {
       before(function () {
         // Only implemented on macOS.
@@ -2469,8 +2465,7 @@ describe('BrowserWindow module', () => {
       })
     })
 
-    // TODO(alexeykuzmin): [Ch66] Enable the tests. They pass locally.
-    xdescribe('fullscreen state', () => {
+    describe('fullscreen state', () => {
       before(function () {
         // Only implemented on macOS.
         if (process.platform !== 'darwin') {
@@ -2618,8 +2613,7 @@ describe('BrowserWindow module', () => {
       }
     })
 
-    // TODO(alexeykuzmin): [Ch66] Enable the test. Fails on CI bots, passes locally.
-    xit('exits HTML fullscreen when window leaves fullscreen', (done) => {
+    it('exits HTML fullscreen when window leaves fullscreen', (done) => {
       w.destroy()
       w = new BrowserWindow()
       w.webContents.once('did-finish-load', () => {

spec/api-web-contents-spec.js

@@ -335,8 +335,7 @@ describe('webContents module', () => {
     })
   })
 
-  // TODO(alexeykuzmin): [Ch66] Enable the test. Passes locally.
-  xdescribe('focus()', () => {
+  describe('focus()', () => {
     describe('when the web contents is hidden', () => {
       it('does not blur the focused window', (done) => {
         ipcMain.once('answer', (event, parentFocused, childFocused) => {