From de69d0643c9a3d2d5d01ed1a167f8074beec0706 Mon Sep 17 00:00:00 2001
From: Yad Smood <ys@ysmood.org>
Date: Fri, 18 Sep 2020 04:45:58 +0900
Subject: [PATCH] kill after browser is ready to be killed

---
 browser_test.go               | 17 +++++++++++------
 lib/launcher/launcher.go      | 17 +++++++++--------
 lib/launcher/launcher_test.go | 16 ++++++++--------
 lib/launcher/proxy.go         |  2 +-
 4 files changed, 29 insertions(+), 23 deletions(-)

diff --git a/browser_test.go b/browser_test.go
index 4e777f27..a494647a 100644
--- a/browser_test.go
+++ b/browser_test.go
@@ -146,15 +146,20 @@ func (s *S) TestMonitor() {
 	s.EqualValues(-32602, utils.MustReadJSON(res.Body).Get("code").Int())
 }
 
-func (s *S) TestMonitorEnv() {
-	defaults.Monitor = ":0"
+func (s *S) TestMonitorErr() {
+	defaults.Monitor = "abc"
 	defer defaults.ResetWithEnv()
 
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
+	l := launcher.New()
+	u := l.MustLaunch()
+	defer func() {
+		utils.Sleep(1) // if kill too fast, the parent process of the browser may not be ready
+		l.Kill()
+	}()
 
-	b := rod.New().Context(ctx).MustConnect()
-	defer b.MustClose()
+	s.Panics(func() {
+		rod.New().ControlURL(u).MustConnect()
+	})
 }
 
 func (s *S) TestRemoteLaunch() {
diff --git a/lib/launcher/launcher.go b/lib/launcher/launcher.go
index 55ac8365..f8795b18 100644
--- a/lib/launcher/launcher.go
+++ b/lib/launcher/launcher.go
@@ -330,7 +330,7 @@ func (l *Launcher) Launch() (string, error) {
 
 	u, err := l.getURL()
 	if err != nil {
-		go l.kill()
+		l.Kill()
 		return "", err
 	}
 
@@ -371,6 +371,14 @@ func (l *Launcher) PID() int {
 	return l.pid
 }
 
+// Kill the browser process
+func (l *Launcher) Kill() {
+	p, err := os.FindProcess(l.PID())
+	if err == nil {
+		_ = p.Kill()
+	}
+}
+
 // Cleanup wait until the Browser exits and release related resources
 func (l *Launcher) Cleanup() {
 	<-l.exit
@@ -379,13 +387,6 @@ func (l *Launcher) Cleanup() {
 	_ = os.RemoveAll(dir)
 }
 
-func (l *Launcher) kill() {
-	p, err := os.FindProcess(l.pid)
-	if err == nil {
-		_ = p.Kill()
-	}
-}
-
 func (l *Launcher) normalizeFlag(name string) string {
 	return strings.TrimLeft(name, "-")
 }
diff --git a/lib/launcher/launcher_test.go b/lib/launcher/launcher_test.go
index dcd1fb2d..deec3457 100644
--- a/lib/launcher/launcher_test.go
+++ b/lib/launcher/launcher_test.go
@@ -48,7 +48,10 @@ func TestLaunch(t *testing.T) {
 	defer func() { defaults.ResetWithEnv() }()
 
 	l := launcher.New()
-	defer func() { kill(l.PID()) }()
+	defer func() {
+		utils.Sleep(1)
+		l.Kill()
+	}()
 
 	url := l.MustLaunch()
 
@@ -57,7 +60,10 @@ func TestLaunch(t *testing.T) {
 
 func TestLaunchUserMode(t *testing.T) {
 	l := launcher.NewUserMode()
-	defer func() { kill(l.PID()) }()
+	defer func() {
+		utils.Sleep(1)
+		l.Kill()
+	}()
 
 	_, has := l.Get("not-exists")
 	assert.False(t, has)
@@ -123,9 +129,3 @@ func skipDownload(t *testing.T) {
 		t.SkipNow()
 	}
 }
-
-func kill(pid int) {
-	ps, err := os.FindProcess(pid)
-	utils.E(err)
-	utils.E(ps.Kill())
-}
diff --git a/lib/launcher/proxy.go b/lib/launcher/proxy.go
index c3edc793..8f3f05e9 100644
--- a/lib/launcher/proxy.go
+++ b/lib/launcher/proxy.go
@@ -106,7 +106,7 @@ func (p *Proxy) launch(w http.ResponseWriter, r *http.Request) {
 
 	u := l.MustLaunch()
 	defer func() {
-		l.kill()
+		l.Kill()
 
 		if _, has := l.Get(flagKeepUserDataDir); !has {
 			l.Cleanup()