fix nil pointer in screenshot

[yusufozturk]

This PR simply adds some additional nil checks to prevent panics in screenshot.

Fixes #631

Development guide

Link

Test on local before making the PR

go run ./lib/utils/simple-check

[yusufozturk]

I cant repro the issue for tests at the moment. If I can in the future, I can reopen this PR.

If anybody has same issue, they can see the PR details.

Thanks.

[alexferrari88]

I'm having a stab at this but I'm running into issues in the test. This would be my addition to TestPageScreenshot:

func TestPageScreenshot(t *testing.T) {
	g := setup(t)

	f := filepath.Join("tmp", "screenshots", g.RandStr(16)+".png")
	p := g.page.MustNavigate(g.srcFile("fixtures/click.html"))
	p.MustElement("button")
	p.MustScreenshot()
	data := p.MustScreenshot(f)
	img, err := png.Decode(bytes.NewBuffer(data))
	g.E(err)
	g.Eq(1280, img.Bounds().Dx())
	g.Eq(800, img.Bounds().Dy())
	g.Nil(os.Stat(f))

	p.MustScreenshot("")

	g.Panic(func() {
		g.mc.stubErr(1, proto.PageCaptureScreenshot{})
		p.MustScreenshot()
	})

	g.Panic(func() {
		g.mc.stub(1, proto.PageGetLayoutMetrics{}, func(send StubSend) (gson.JSON, error) {
			return gson.New(proto.PageGetLayoutMetricsResult{}), nil
		})
		p.MustScreenshot()
	})
}

When executing go test I get:

--- FAIL: TestPageScreenshot (0.49s)
    page_test.go:619:  ⦗should panic⦘ 

The code under test is:

func (p *Page) Screenshot(fullpage bool, req *proto.PageCaptureScreenshot) ([]byte, error) {
	if req == nil {
		req = &proto.PageCaptureScreenshot{}
	}
	if fullpage {
		metrics, err := proto.PageGetLayoutMetrics{}.Call(p)
		if err != nil {
			return nil, err
		}

		if metrics.CSSContentSize == nil {
			return nil, errors.New("failed to get css content size")
		}

		oldView := proto.EmulationSetDeviceMetricsOverride{}
		set := p.LoadState(&oldView)
		view := oldView
		view.Width = int(metrics.CSSContentSize.Width)
		view.Height = int(metrics.CSSContentSize.Height)

		err = p.SetViewport(&view)
		if err != nil {
			return nil, err
		}

		defer func() { // try to recover the viewport
			if !set {
				_ = proto.EmulationClearDeviceMetricsOverride{}.Call(p)
				return
			}

			_ = p.SetViewport(&oldView)
		}()
	}

	shot, err := req.Call(p)
	if err != nil {
		return nil, err
	}
	return shot.Data, nil
}

What am I doing wrong?

[ysmood]

@alexferrari88 because fullpage is false when you call p.MustScreenshot().

[yusufozturk]

Ups sorry, I clicked a wrong button while reading your comment @ysmood

I was in holiday and I can get back on this.

[alexferrari88]

this can be closed because of #699