Fix some flaky tests in CI (#12153)

* Guard whole InitPrefs with ScopedAllowIO

Saw a crash:
0 0x7f8d2f7d918d base::debug::StackTrace::StackTrace()
1 0x7f8d2f7d755c base::debug::StackTrace::StackTrace()
2 0x7f8d2f867caa logging::LogMessage::~LogMessage()
3 0x7f8d2fa157c7 base::ThreadRestrictions::AssertIOAllowed()
4 0x7f8d2f83453a base::OpenFile()
5 0x7f8d2f82a967 base::ReadFileToStringWithMaxSize()
6 0x7f8d2f82ad44 base::ReadFileToString()
7 0x7f8d2f846f73 JSONFileValueDeserializer::ReadFileToString()
8 0x7f8d2f84738c JSONFileValueDeserializer::Deserialize()
9 0x7f8d35a5d1f6 <unknown>
10 0x7f8d35a5c217 JsonPrefStore::ReadPrefs()
11 0x7f8d35a87d3e PrefService::InitFromStorage()
12 0x7f8d35a87c60 PrefService::PrefService()
13 0x7f8d35a91a10 PrefServiceFactory::Create()
14 0x000000e86e1b brightray::BrowserContext::InitPrefs()
15 0x000000c2bd64 atom::AtomBrowserContext::AtomBrowserContext()
16 0x000000c320db atom::AtomBrowserContext::From()
17 0x000000b4b8b5 atom::api::Session::FromPartition()

* Fix done being called twice in setInterval test

The callback passed to browser process is called asyncly, so it is
possible that multiple callbacks has already been scheduled before we
can clearInternval.

* Fix failing test when dir name has special chars

The pdfSource is not escaped while parsedURL.search is.

* Call done with Error instead of string

* Fix crash caused by not removing input observer

Solve crash:
0 libcontent.dylib content::RenderWidgetHostImpl::DispatchInputEventWithLatencyInfo(blink::WebInputEvent const&, ui::LatencyInfo*) + 214
1 libcontent.dylib content::RenderWidgetHostImpl::ForwardMouseEventWithLatencyInfo(blink::WebMouseEvent const&, ui::LatencyInfo const&) + 1350
2 libcontent.dylib content::RenderWidgetHostViewMac::ProcessMouseEvent(blink::WebMouseEvent const&, ui::LatencyInfo const&) + 44
3 libcontent.dylib content::RenderWidgetHostInputEventRouter::RouteMouseEvent(content::RenderWidgetHostViewBase*, blink::WebMouseEvent*, ui::LatencyInfo const&) + 1817

* Print detailed error

* Run tests after server is ready

atom/browser/api/atom_api_browser_window.cc

@@ -275,8 +275,7 @@ bool BrowserWindow::OnMessageReceived(const IPC::Message& message,
 }
 
 void BrowserWindow::OnCloseContents() {
-  if (!web_contents())
-    return;
+  DCHECK(web_contents());
 
   // Close all child windows before closing current window.
   v8::Locker locker(isolate());
@@ -296,9 +295,6 @@ void BrowserWindow::OnCloseContents() {
 
   // Do not sent "unresponsive" event after window is closed.
   window_unresponsive_closure_.Cancel();
-
-  // Clear the web_contents() at last.
-  Observe(nullptr);
 }
 
 void BrowserWindow::OnRendererResponsive() {
@@ -338,8 +334,13 @@ void BrowserWindow::OnCloseButtonClicked(bool* prevent_default) {
 }
 
 void BrowserWindow::OnWindowClosed() {
+  auto* host = web_contents()->GetRenderViewHost();
+  if (host)
+    host->GetWidget()->RemoveInputEventObserver(this);
+
   api_web_contents_->DestroyWebContents(true /* async */);
 
+  Observe(nullptr);
   RemoveFromWeakMap();
   window_->RemoveObserver(this);
 

brightray/browser/browser_context.cc

@@ -101,13 +101,11 @@ BrowserContext::~BrowserContext() {
 
 void BrowserContext::InitPrefs() {
   auto prefs_path = GetPath().Append(FILE_PATH_LITERAL("Preferences"));
+  base::ThreadRestrictions::ScopedAllowIO allow_io;
   PrefServiceFactory prefs_factory;
   scoped_refptr<JsonPrefStore> pref_store =
       base::MakeRefCounted<JsonPrefStore>(prefs_path);
-  {
-    base::ThreadRestrictions::ScopedAllowIO allow_io;
-    pref_store->ReadPrefs();  // Synchronous.
-  }
+  pref_store->ReadPrefs();  // Synchronous.
   prefs_factory.set_user_prefs(pref_store);
 
   auto registry = make_scoped_refptr(new PrefRegistrySimple);

spec/api-app-spec.js

@@ -44,7 +44,7 @@ describe('app module', () => {
   let server, secureUrl
   const certPath = path.join(__dirname, 'fixtures', 'certificates')
 
-  before(() => {
+  before((done) => {
     const options = {
       key: fs.readFileSync(path.join(certPath, 'server.key')),
       cert: fs.readFileSync(path.join(certPath, 'server.pem')),
@@ -69,11 +69,12 @@ describe('app module', () => {
     server.listen(0, '127.0.0.1', () => {
       const port = server.address().port
       secureUrl = `https://127.0.0.1:${port}`
+      done()
     })
   })
 
-  after(() => {
-    server.close()
+  after((done) => {
+    server.close(() => done())
   })
 
   describe('app.getVersion()', () => {
@@ -507,7 +508,6 @@ describe('app module', () => {
     it('can respond with empty certificate list', (done) => {
       w.webContents.on('did-finish-load', () => {
         assert.equal(w.webContents.getTitle(), 'denied')
-        server.close()
         done()
       })
 

spec/chromium-spec.js

@@ -139,7 +139,7 @@ describe('chromium feature', () => {
         if (args[0] === 'reload') {
           w.webContents.reload()
         } else if (args[0] === 'error') {
-          done(`unexpected error : ${args[1]}`)
+          done(new Error(`unexpected error : ${JSON.stringify(args[1])}`))
         } else if (args[0] === 'response') {
           assert.equal(args[1], 'Hello from serviceWorker!')
           session.defaultSession.clearStorageData({
@@ -1047,7 +1047,7 @@ describe('chromium feature', () => {
         assert.equal(parsedURL.hostname, 'pdf-viewer')
         assert.equal(parsedURL.query.src, pdfSourceWithParams)
         assert.equal(parsedURL.query.b, undefined)
-        assert.equal(parsedURL.search, `?src=${pdfSource}%3Fa%3D1%26b%3D2`)
+        assert(parsedURL.search.endsWith('%3Fa%3D1%26b%3D2'))
         assert.equal(w.webContents.getTitle(), 'cat.pdf')
       })
       w.webContents.loadURL(pdfSourceWithParams)

spec/node-spec.js

@@ -183,10 +183,13 @@ describe('node feature', () => {
 
     describe('setInterval called under Chromium event loop in browser process', () => {
       it('can be scheduled in time', (done) => {
-        let clear
-        let interval
-        clear = () => {
+        let interval = null
+        const clear = () => {
+          if (interval === null) {
+            return
+          }
           remote.getGlobal('clearInterval')(interval)
+          interval = null
           done()
         }
         interval = remote.getGlobal('setInterval')(clear, 10)