Refactor Page.screenshot() api

This patch refactors Page.screenshot api, accoring to the discussion
in #5:
- Page.screenshot accepts single optional options object
- Page.saveScreenshot is removed
- Page.screenshot assumes 'png' screenshot if no type is set and no
  'path' property is given

Fixes #5.

docs/api.md

@@ -22,7 +22,6 @@
     - [page.evaluate(fun, args)](#pageevaluatefun-args)
     - [page.evaluateOnInitialized(fun, args)](#pageevaluateoninitializedfun-args)
     - [page.screenshot(type[, clipRect])](#pagescreenshottype-cliprect)
-    - [page.saveScreenshot(filePath[, clipRect])](#pagesavescreenshotfilepath-cliprect)
     - [page.printToPDF(filePath[, options])](#pageprinttopdffilepath-options)
     - [page.plainText()](#pageplaintext)
     - [page.title()](#pagetitle)
@@ -157,11 +156,6 @@ Pages could be closed by `page.close()` method.
 	- `height` [<number>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#Number_type) height of clipping area
 - returns: [<Promise<Buffer>>](https://developer.mozilla.org/en/docs/Web/JavaScript/Reference/Global_Objects/Promise) Promise which resolves to buffer with captured screenshot
 
-#### page.saveScreenshot(filePath[, clipRect])
-
-- `filePath` [<string>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type) The file path to save image to. The screenshot type will be inferred from file extension
-- `clipRect` [<Object>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object) Clip rect object which will be passed over to `page.screenshot` method.
-
 #### page.printToPDF(filePath[, options])
 
 - `filePath` [<string>](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Data_structures#String_type) The file path to save image to. The screenshot type will be inferred from file extension

lib/Page.js

@@ -387,20 +387,52 @@ class Page extends EventEmitter {
     }
 
     /**
-     * @param {!Page.ScreenshotType} screenshotType
+     * @param {!Object=} options
-     * @param {?{x: number, y: number, width: number, height: number}} clipRect
      * @return {!Promise<!Buffer>}
      */
-    async screenshot(screenshotType, clipRect) {
+    async screenshot(options) {
-        if (clipRect) {
+        var screenshotType = null;
+        if (options.path) {
+            var mimeType = mime.lookup(options.path);
+            if (mimeType === 'image/png')
+                screenshotType = 'png';
+            else if (mimeType === 'image/jpeg')
+                screenshotType = 'jpeg';
+            else
+                throw new Error('Unsupported screenshot mime type: ' + mimeType);
+        }
+        if (options.type) {
+            if (screenshotType && options.type !== screenshotType)
+                throw new Error(`Passed screenshot type '${options.type}' does not match to the type inferred from the file path: '${screenshotType}'`);
+            if (options.type !== 'png' && options.type !== 'jpeg')
+                throw new Error('Unknown screenshot type: ' + options.type);
+            screenshotType = options.type;
+        }
+        if (!screenshotType)
+            screenshotType = 'png';
+
+        if (options.quality) {
+            console.assert(screenshotType === 'jpeg', 'options.quality is unsupported for the ' + screenshotType + ' screenshots');
+            console.assert(typeof options.quality === 'number', 'Expected options.quality to be a number but found ' + (typeof options.quality));
+            console.assert(Number.isInteger(options.quality), 'Expected options.quality to be an integer');
+            console.assert(options.quality >= 0 && options.quality <= 100, 'Expected options.quality to be between 0 and 100 (inclusive), got ' + options.quality);
+        }
+        if (options.clip) {
+            console.assert(typeof options.clip.x === 'number', 'Expected options.clip.x to be a number but found ' + (typeof options.clip.x));
+            console.assert(typeof options.clip.y === 'number', 'Expected options.clip.y to be a number but found ' + (typeof options.clip.y));
+            console.assert(typeof options.clip.width === 'number', 'Expected options.clip.width to be a number but found ' + (typeof options.clip.width));
+            console.assert(typeof options.clip.height === 'number', 'Expected options.clip.height to be a number but found ' + (typeof options.clip.height));
+        }
+
+        if (options.clip) {
             await Promise.all([
                 this._client.send('Emulation.setVisibleSize', {
-                    width: Math.ceil(clipRect.width / this._screenDPI),
+                    width: Math.ceil(options.clip.width / this._screenDPI),
-                    height: Math.ceil(clipRect.height / this._screenDPI),
+                    height: Math.ceil(options.clip.height / this._screenDPI),
                 }),
                 this._client.send('Emulation.forceViewport', {
-                    x: clipRect.x / this._screenDPI,
+                    x: options.clip.x / this._screenDPI,
-                    y: clipRect.y / this._screenDPI,
+                    y: options.clip.y / this._screenDPI,
                     scale: 1,
                 })
             ]);
@@ -408,32 +440,18 @@ class Page extends EventEmitter {
         var result = await this._client.send('Page.captureScreenshot', {
             fromSurface: true,
             format: screenshotType,
+            quality: options.quality
         });
-        if (clipRect) {
+        if (options.clip) {
             await Promise.all([
                 this.setViewportSize(this.viewportSize()),
                 this._client.send('Emulation.resetViewport')
             ]);
         }
-        return new Buffer(result.data, 'base64');
+        var buffer = new Buffer(result.data, 'base64');
-    }
+        if (options.path)
-
+            fs.writeFileSync(options.path, buffer);
-    /**
+        return buffer;
-     * @param {string} filePath
-     * @param {?{x: number, y: number, width: number, height: number}} clipRect
-     * @return {!Promise}
-     */
-    async saveScreenshot(filePath, clipRect) {
-        var mimeType = mime.lookup(filePath);
-        var screenshotType = null;
-        if (mimeType === 'image/png')
-            screenshotType = Page.ScreenshotTypes.PNG;
-        else if (mimeType === 'image/jpeg')
-            screenshotType = Page.ScreenshotTypes.JPG;
-        if (!screenshotType)
-            throw new Error(`Cannot render to file ${filePath} - unsupported mimeType ${mimeType}`);
-        var buffer = await this.screenshot(screenshotType, clipRect);
-        fs.writeFileSync(filePath, buffer);
     }
 
     /**
@@ -522,12 +540,6 @@ class InPageCallback {
     }
 }
 
-/** @enum {string} */
-Page.ScreenshotTypes = {
-    PNG: 'png',
-    JPG: 'jpeg',
-};
-
 /** @enum {string} */
 Page.PaperFormats = {
     Letter: {width: 8.5, height: 11},

phantom_shim/WebPage.js

@@ -285,15 +285,6 @@ class WebPage {
      * {string} fileName
      */
     render(fileName) {
-        var clipRect = null;
-        if (this.clipRect && (this.clipRect.left || this.clipRect.top || this.clipRect.width || this.clipRect.height)) {
-            clipRect = {
-                x: this.clipRect.left,
-                y: this.clipRect.top,
-                width: this.clipRect.width,
-                height: this.clipRect.height
-            };
-        }
         if (fileName.endsWith('pdf')) {
             var options = {};
             var paperSize = this.paperSize || {};
@@ -304,6 +295,16 @@ class WebPage {
             options.height = paperSize.height;
             await(this._page.printToPDF(fileName, options));
         } else {
+            var options = {};
+            if (this.clipRect && (this.clipRect.left || this.clipRect.top || this.clipRect.width || this.clipRect.height)) {
+                options.clipRect = {
+                    x: this.clipRect.left,
+                    y: this.clipRect.top,
+                    width: this.clipRect.width,
+                    height: this.clipRect.height
+                };
+            }
+            options.path = fileName;
             await(this._page.saveScreenshot(fileName, clipRect));
         }
     }

test/goldentest.js

@@ -55,17 +55,19 @@ describe('GoldenTests', function() {
     imageTest('screenshot-sanity.png', async function() {
         await page.setViewportSize({width: 500, height: 500});
         await page.navigate(STATIC_PREFIX + '/grid.html');
-        return page.screenshot('png');
+        return await page.screenshot();
     });
 
     imageTest('screenshot-clip-rect.png', async function() {
         await page.setViewportSize({width: 500, height: 500});
         await page.navigate(STATIC_PREFIX + '/grid.html');
-        return page.screenshot('png', {
+        return await page.screenshot({
-            x: 50,
+            clip: {
-            y: 100,
+                x: 50,
-            width: 150,
+                y: 100,
-            height: 100
+                width: 150,
+                height: 100
+            }
         });
     });
 });