From e5f4ec183a1dbd8165e27abeda611de8585c3fba Mon Sep 17 00:00:00 2001 From: Jim Mathies Date: Tue, 22 Jul 2014 07:17:45 -0500 Subject: [PATCH 01/11] Bug 948574 - Provide limited access to nsISiteSecurityService apis in the child, and prevent the direct creation of nsISiteSecurityService. r=bz, sr=ptheriault, original author: David Keeler --- docshell/base/nsDocShell.cpp | 27 ++++++++++++------- dom/ipc/ContentParent.cpp | 18 +++++++++++++ dom/ipc/ContentParent.h | 3 +++ dom/ipc/PContent.ipdl | 3 +++ .../boot/src/nsSiteSecurityService.cpp | 6 +++++ 5 files changed, 48 insertions(+), 9 deletions(-) diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 3234bfaab92f..3c69180a3263 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -22,6 +22,7 @@ #include "mozilla/Telemetry.h" #include "mozilla/unused.h" #include "mozilla/VisualEventTracer.h" +#include "URIUtils.h" #ifdef MOZ_LOGGING // so we can get logging even in release builds (but only for some things) @@ -4559,16 +4560,24 @@ nsDocShell::DisplayLoadError(nsresult aError, nsIURI *aURI, // if this is a Strict-Transport-Security host and the cert // is bad, don't allow overrides (STS Spec section 7.3). - nsCOMPtr sss = - do_GetService(NS_SSSERVICE_CONTRACTID, &rv); - NS_ENSURE_SUCCESS(rv, rv); - uint32_t flags = - mInPrivateBrowsing ? nsISocketProvider::NO_PERMANENT_STORAGE : 0; - + uint32_t type = nsISiteSecurityService::HEADER_HSTS; + uint32_t flags = mInPrivateBrowsing + ? nsISocketProvider::NO_PERMANENT_STORAGE + : 0; bool isStsHost = false; - rv = sss->IsSecureURI(nsISiteSecurityService::HEADER_HSTS, - aURI, flags, &isStsHost); - NS_ENSURE_SUCCESS(rv, rv); + if (XRE_GetProcessType() == GeckoProcessType_Default) { + nsCOMPtr sss = + do_GetService(NS_SSSERVICE_CONTRACTID, &rv); + NS_ENSURE_SUCCESS(rv, rv); + rv = sss->IsSecureURI(type, aURI, flags, &isStsHost); + NS_ENSURE_SUCCESS(rv, rv); + } else { + mozilla::dom::ContentChild* cc = + mozilla::dom::ContentChild::GetSingleton(); + mozilla::ipc::URIParams uri; + SerializeURI(aURI, uri); + cc->SendIsSecureURI(type, uri, flags, &isStsHost); + } uint32_t bucketId; if (isStsHost) { diff --git a/dom/ipc/ContentParent.cpp b/dom/ipc/ContentParent.cpp index 1a0fd0facdbd..3dfe0bf6d73c 100644 --- a/dom/ipc/ContentParent.cpp +++ b/dom/ipc/ContentParent.cpp @@ -98,6 +98,7 @@ #include "nsIPresShell.h" #include "nsIRemoteBlob.h" #include "nsIScriptError.h" +#include "nsISiteSecurityService.h" #include "nsIStyleSheet.h" #include "nsISupportsPrimitives.h" #include "nsIURIFixup.h" @@ -3239,6 +3240,23 @@ ContentParent::RecvGetSystemMemory(const uint64_t& aGetterId) return true; } +bool +ContentParent::RecvIsSecureURI(const uint32_t& type, + const URIParams& uri, + const uint32_t& flags, + bool* isSecureURI) +{ + nsCOMPtr sss(do_GetService(NS_SSSERVICE_CONTRACTID)); + if (!sss) { + return false; + } + nsCOMPtr ourURI = DeserializeURI(uri); + if (!ourURI) { + return false; + } + nsresult rv = sss->IsSecureURI(type, ourURI, flags, isSecureURI); + return NS_SUCCEEDED(rv); +} bool ContentParent::RecvLoadURIExternal(const URIParams& uri) diff --git a/dom/ipc/ContentParent.h b/dom/ipc/ContentParent.h index f2546c1b8445..f29895120cf7 100644 --- a/dom/ipc/ContentParent.h +++ b/dom/ipc/ContentParent.h @@ -422,6 +422,9 @@ private: virtual bool RecvGetRandomValues(const uint32_t& length, InfallibleTArray* randomValues) MOZ_OVERRIDE; + virtual bool RecvIsSecureURI(const uint32_t& type, const URIParams& uri, + const uint32_t& flags, bool* isSecureURI); + virtual bool DeallocPHalParent(PHalParent*) MOZ_OVERRIDE; virtual bool DeallocPIndexedDBParent(PIndexedDBParent* aActor) MOZ_OVERRIDE; diff --git a/dom/ipc/PContent.ipdl b/dom/ipc/PContent.ipdl index 3f6a86529a4e..e3581bec1a34 100644 --- a/dom/ipc/PContent.ipdl +++ b/dom/ipc/PContent.ipdl @@ -478,6 +478,9 @@ parent: async GetSystemMemory(uint64_t getterId); + sync IsSecureURI(uint32_t type, URIParams uri, uint32_t flags) + returns (bool isSecureURI); + PHal(); PIndexedDB(); diff --git a/security/manager/boot/src/nsSiteSecurityService.cpp b/security/manager/boot/src/nsSiteSecurityService.cpp index 3cab048ec8f6..30cc0de889bd 100644 --- a/security/manager/boot/src/nsSiteSecurityService.cpp +++ b/security/manager/boot/src/nsSiteSecurityService.cpp @@ -20,6 +20,7 @@ #include "mozilla/Preferences.h" #include "mozilla/LinkedList.h" #include "nsSecurityHeaderParser.h" +#include "nsXULAppAPI.h" // A note about the preload list: // When a site specifically disables sts by sending a header with @@ -87,6 +88,11 @@ NS_IMPL_ISUPPORTS(nsSiteSecurityService, nsresult nsSiteSecurityService::Init() { + // Child processes are not allowed direct access to this. + if (XRE_GetProcessType() != GeckoProcessType_Default) { + MOZ_CRASH("Child process: no direct access to nsSiteSecurityService"); + } + nsresult rv; mPermMgr = do_GetService(NS_PERMISSIONMANAGER_CONTRACTID, &rv); From 719a823ea59d8ff1bdbc389e44b619434abca1cd Mon Sep 17 00:00:00 2001 From: Jim Mathies Date: Tue, 22 Jul 2014 08:56:59 -0500 Subject: [PATCH 02/11] Bug 1040080 - Update nsIMessageManager.idl notes about in-process child process message manager. r=smaug --- content/base/public/nsIMessageManager.idl | 24 ++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/content/base/public/nsIMessageManager.idl b/content/base/public/nsIMessageManager.idl index 4afa64e33820..fb72bc876cd9 100644 --- a/content/base/public/nsIMessageManager.idl +++ b/content/base/public/nsIMessageManager.idl @@ -125,27 +125,29 @@ interface nsIPrincipal; * * Parent process Child processes * ---------------- ----------------- - * global PPMM + * global (GPPMM) * | - * +<----> child PPMM + * +-->parent in-process PIPMM<-->child in-process CIPPMM * | - * +-->parent PMM1<------------------>child process CMM1 + * +-->parent (PPMM1)<------------------>child (CPMM1) * | - * +-->parent PMM2<------------------>child process PMM2 + * +-->parent (PPMM2)<------------------>child (CPMM2) * ... * - * For example: the parent-process PMM1 sends messages directly to - * only the child-process CMM1. + * Note, PIPMM and CIPPMM both run in the parent process. * - * For example: CMM1 sends messages directly to PMM1. The global PPMM + * For example: the parent-process PPMM1 sends messages to the + * child-process CPMM1. + * + * For example: CPMM1 sends messages directly to PPMM1. The global GPPMM * will also notify their message listeners when the message arrives. * - * For example: messages sent through the global PPMM will be - * dispatched to the listeners of the same-process, "child PPMM". - * They will also be broadcast to PPM1, PPM2, etc. + * For example: messages sent through the global GPPMM will be + * dispatched to the listeners of the same-process, CIPPMM, CPMM1, + * CPMM2, etc. * * ***** PERFORMANCE AND SECURITY WARNING ***** - * Messages broadcast through the global PPMM can result in messages + * Messages broadcast through the GPPMM can result in messages * being dispatched across many OS processes, and to many processes * with different permissions. Great care should be taken when * broadcasting. From e3093a68aea1cfab1f2041f9c33fd4b203511ae8 Mon Sep 17 00:00:00 2001 From: Eric Edens Date: Mon, 21 Jul 2014 13:36:23 -0700 Subject: [PATCH 03/11] Bug 1040994 - Add LIMIT support for search history content provider. r=rnewman --HG-- extra : rebase_source : b210034222a4a47d23a441a1915461374827471e --- .../base/db/SearchHistoryProvider.java | 8 +- .../base/tests/testSearchHistoryProvider.java | 79 ++++++++++++++++++- 2 files changed, 83 insertions(+), 4 deletions(-) diff --git a/mobile/android/base/db/SearchHistoryProvider.java b/mobile/android/base/db/SearchHistoryProvider.java index 06ecd40389a3..05d31fefd6aa 100644 --- a/mobile/android/base/db/SearchHistoryProvider.java +++ b/mobile/android/base/db/SearchHistoryProvider.java @@ -4,6 +4,7 @@ package org.mozilla.gecko.db; +import org.mozilla.gecko.db.BrowserContract; import org.mozilla.gecko.db.BrowserContract.SearchHistory; import android.content.ContentUris; @@ -110,10 +111,11 @@ public class SearchHistoryProvider extends SharedBrowserDatabaseProvider { @Override public Cursor query(Uri uri, String[] projection, String selection, String[] selectionArgs, String sortOrder) { - String groupBy = null; - String having = null; + final String groupBy = null; + final String having = null; + final String limit = uri.getQueryParameter(BrowserContract.PARAM_LIMIT); final Cursor cursor = getReadableDatabase(uri).query(SearchHistory.TABLE_NAME, projection, - selection, selectionArgs, groupBy, having, sortOrder); + selection, selectionArgs, groupBy, having, sortOrder, limit); cursor.setNotificationUri(getContext().getContentResolver(), uri); return cursor; } diff --git a/mobile/android/base/tests/testSearchHistoryProvider.java b/mobile/android/base/tests/testSearchHistoryProvider.java index c1d487706ba6..e0ae0d871c4c 100644 --- a/mobile/android/base/tests/testSearchHistoryProvider.java +++ b/mobile/android/base/tests/testSearchHistoryProvider.java @@ -13,11 +13,13 @@ import org.mozilla.gecko.db.SearchHistoryProvider; import android.content.ContentProvider; import android.content.ContentValues; import android.database.Cursor; +import android.net.Uri; public class testSearchHistoryProvider extends ContentProviderTest { // Translations of "United Kingdom" in several different languages - private static final String[] testStrings = {"An Ríocht Aontaithe", // Irish + private static final String[] testStrings = { + "An Ríocht Aontaithe", // Irish "Angli", // Albanian "Britanniarum Regnum", // Latin "Britio", // Esperanto @@ -95,6 +97,7 @@ public class testSearchHistoryProvider extends ContentProviderTest { mTests.add(new TestInsert()); mTests.add(new TestUnicodeQuery()); mTests.add(new TestTimestamp()); + mTests.add(new TestLimit()); mTests.add(new TestDelete()); mTests.add(new TestIncrement()); } @@ -111,6 +114,80 @@ public class testSearchHistoryProvider extends ContentProviderTest { } } + /** + * Verify that we can pass a LIMIT clause using a query parameter. + */ + private class TestLimit extends TestCase { + @Override + public void test() throws Exception { + ContentValues cv; + for (int i = 0; i < testStrings.length; i++) { + cv = new ContentValues(); + cv.put(SearchHistory.QUERY, testStrings[i]); + mProvider.insert(SearchHistory.CONTENT_URI, cv); + } + + final int limit = 5; + + // Test 1: Handle proper input. + + Uri uri = SearchHistory.CONTENT_URI + .buildUpon() + .appendQueryParameter(BrowserContract.PARAM_LIMIT, String.valueOf(limit)) + .build(); + + Cursor c = mProvider.query(uri, null, null, null, null); + try { + mAsserter.is(c.getCount(), limit, + String.format("Should have %d results", limit)); + } finally { + c.close(); + } + + // Test 2: Empty input yields all results. + + uri = SearchHistory.CONTENT_URI + .buildUpon() + .appendQueryParameter(BrowserContract.PARAM_LIMIT, "") + .build(); + + c = mProvider.query(uri, null, null, null, null); + try { + mAsserter.is(c.getCount(), testStrings.length, "Should have all results"); + } finally { + c.close(); + } + + // Test 3: Illegal params. + + String[] illegalParams = new String[] {"a", "-1"}; + boolean success = true; + + for (String param : illegalParams) { + success = true; + + uri = SearchHistory.CONTENT_URI + .buildUpon() + .appendQueryParameter(BrowserContract.PARAM_LIMIT, param) + .build(); + + try { + c = mProvider.query(uri, null, null, null, null); + success = false; + } catch(IllegalArgumentException e) { + // noop. + } finally { + if (c != null) { + c.close(); + } + } + + mAsserter.ok(success, "LIMIT", param + " should have been an invalid argument"); + } + + } + } + /** * Verify that we can insert values into the DB, including unicode. */ From 03114230a0f4b8201c2947d9d159e1d84b515a43 Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Tue, 22 Jul 2014 12:43:22 -0400 Subject: [PATCH 04/11] Bug 1041149 - Remove unused variable in generateScreenshotFor in canvas.js, r=past --- toolkit/devtools/server/actors/canvas.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/toolkit/devtools/server/actors/canvas.js b/toolkit/devtools/server/actors/canvas.js index 391e4f413a01..2c7ea74c4b4c 100644 --- a/toolkit/devtools/server/actors/canvas.js +++ b/toolkit/devtools/server/actors/canvas.js @@ -156,8 +156,6 @@ let FrameSnapshotActor = protocol.ActorClass({ last: index }); - // To keep things fast, generate an image that's relatively small. - let dimensions = Math.min(CanvasFront.SCREENSHOT_HEIGHT_MAX, canvas.height); let screenshot; // Depending on the canvas' context, generating a screenshot is done From a9a0c9ee2b048fa86e1f9e21d1f4fc5905c3ae32 Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Tue, 22 Jul 2014 12:43:23 -0400 Subject: [PATCH 05/11] Bug 1041157 - Add link to the discussion about using the same context with multiple canvases, for the replayAnimationFrame function in canvas.js, r=past --- toolkit/devtools/server/actors/canvas.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/toolkit/devtools/server/actors/canvas.js b/toolkit/devtools/server/actors/canvas.js index 2c7ea74c4b4c..5aa2556cd6e6 100644 --- a/toolkit/devtools/server/actors/canvas.js +++ b/toolkit/devtools/server/actors/canvas.js @@ -564,8 +564,11 @@ let ContextUtils = { * the respective canvas, and the rendering will be performed into it. * This is necessary because some state (like shaders, textures etc.) can't * be shared between two different WebGL contexts. - * Hopefully, once SharedResources are a thing this won't be necessary: - * http://www.khronos.org/webgl/wiki/SharedResouces + * - Hopefully, once SharedResources are a thing this won't be necessary: + * http://www.khronos.org/webgl/wiki/SharedResouces + * - Alternatively, we could pursue the idea of using the same context + * for multiple canvases, instead of trying to share resources: + * https://www.khronos.org/webgl/public-mailing-list/archives/1210/msg00058.html * * In case of a 2D context, a new canvas is created, since there's no * intrinsic state that can't be easily duplicated. From fb723a53a71efa1462457ce6c0e202b79fa42593 Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Tue, 22 Jul 2014 12:43:23 -0400 Subject: [PATCH 06/11] Bug 1041237 - The bitmask enums flags calculation for getBitToEnumValue is flawed, r=jsantell --- .../canvasdebugger/test/browser_canvas-actor-test-09.js | 5 +++++ browser/devtools/canvasdebugger/test/doc_webgl-enum.html | 1 + toolkit/devtools/server/actors/call-watcher.js | 7 +++++-- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-09.js b/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-09.js index 229f3618bc83..8a8e0883409f 100644 --- a/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-09.js +++ b/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-09.js @@ -26,6 +26,11 @@ function ifTestingSupported() { is(functionCalls[0].argsPreview, "DEPTH_BUFFER_BIT | STENCIL_BUFFER_BIT | COLOR_BUFFER_BIT", "The bits passed into `gl.clear` have been cast to their enum values."); + is(functionCalls[1].name, "bindTexture", + "The function's name is correct."); + is(functionCalls[1].argsPreview, "TEXTURE_2D, null", + "The bits passed into `gl.bindTexture` have been cast to their enum values."); + yield removeTab(target.tab); finish(); } diff --git a/browser/devtools/canvasdebugger/test/doc_webgl-enum.html b/browser/devtools/canvasdebugger/test/doc_webgl-enum.html index 3c639363d508..f7f4d6d1e433 100644 --- a/browser/devtools/canvasdebugger/test/doc_webgl-enum.html +++ b/browser/devtools/canvasdebugger/test/doc_webgl-enum.html @@ -25,6 +25,7 @@ function drawScene() { gl.clear(gl.COLOR_BUFFER_BIT | gl.DEPTH_BUFFER_BIT | gl.STENCIL_BUFFER_BIT); + gl.bindTexture(gl.TEXTURE_2D, null); window.requestAnimationFrame(drawScene); } diff --git a/toolkit/devtools/server/actors/call-watcher.js b/toolkit/devtools/server/actors/call-watcher.js index e9c902400f28..fb07b6368b29 100644 --- a/toolkit/devtools/server/actors/call-watcher.js +++ b/toolkit/devtools/server/actors/call-watcher.js @@ -210,9 +210,12 @@ let FunctionCallActor = protocol.ActorClass({ // XXX: All of this sucks. Make this smarter, so that the frontend // can inspect each argument, be it object or primitive. Bug 978960. let serializeArgs = () => args.map((arg, i) => { - if (typeof arg == "undefined") { + if (arg === undefined) { return "undefined"; } + if (arg === null) { + return "null"; + } if (typeof arg == "function") { return "Function"; } @@ -643,7 +646,7 @@ CallWatcherFront.ENUM_METHODS[CallWatcherFront.CANVAS_WEBGL_CONTEXT] = { * For example, when gl.clear(gl.COLOR_BUFFER_BIT) is called, the actual passed * argument's value is 16384, which we want identified as "COLOR_BUFFER_BIT". */ -var gEnumRegex = /^[A-Z_]+$/; +var gEnumRegex = /^[A-Z][A-Z0-9_]+$/; var gEnumsLookupTable = {}; // These values are returned from errors, or empty values, From 545b119ad1e16c1c590cc7997afe921ee5a9714f Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Tue, 22 Jul 2014 12:43:23 -0400 Subject: [PATCH 07/11] Bug 1041250 - Make texParameterf and texParameteri functions take enums for all params, r=jsantell --- toolkit/devtools/server/actors/call-watcher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/toolkit/devtools/server/actors/call-watcher.js b/toolkit/devtools/server/actors/call-watcher.js index fb07b6368b29..ab743bcf9d73 100644 --- a/toolkit/devtools/server/actors/call-watcher.js +++ b/toolkit/devtools/server/actors/call-watcher.js @@ -634,7 +634,7 @@ CallWatcherFront.ENUM_METHODS[CallWatcherFront.CANVAS_WEBGL_CONTEXT] = { stencilOpSeparate: [0, 1, 2, 3], texImage2D: (args) => args.length > 6 ? [0, 2, 6, 7] : [0, 2, 3, 4], texParameterf: [0, 1], - texParameteri: [0, 1], + texParameteri: [0, 1, 2], texSubImage2D: (args) => args.length === 9 ? [0, 6, 7] : [0, 4, 5], vertexAttribPointer: [2] }; From 85551493a0449e6a63eaa995dca8f00aff0d3b6d Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Tue, 22 Jul 2014 12:43:24 -0400 Subject: [PATCH 08/11] Bug 1041158 - Properly cleanup the framebuffer binding after generating a screenshot from a webgl context, r=rcampbell --- .../devtools/canvasdebugger/test/browser.ini | 2 + .../test/browser_canvas-actor-test-08.js | 2 - .../test/browser_canvas-actor-test-10.js | 74 +++++++++++++++++ .../test/doc_webgl-bindings.html | 60 ++++++++++++++ browser/devtools/canvasdebugger/test/head.js | 1 + toolkit/devtools/server/actors/canvas.js | 81 +++++++++++-------- 6 files changed, 184 insertions(+), 36 deletions(-) create mode 100644 browser/devtools/canvasdebugger/test/browser_canvas-actor-test-10.js create mode 100644 browser/devtools/canvasdebugger/test/doc_webgl-bindings.html diff --git a/browser/devtools/canvasdebugger/test/browser.ini b/browser/devtools/canvasdebugger/test/browser.ini index d1fe81961b1a..2430397707b2 100644 --- a/browser/devtools/canvasdebugger/test/browser.ini +++ b/browser/devtools/canvasdebugger/test/browser.ini @@ -5,6 +5,7 @@ support-files = doc_simple-canvas-bitmasks.html doc_simple-canvas-deep-stack.html doc_simple-canvas-transparent.html + doc_webgl-bindings.html doc_webgl-enum.html head.js @@ -17,6 +18,7 @@ support-files = [browser_canvas-actor-test-07.js] [browser_canvas-actor-test-08.js] [browser_canvas-actor-test-09.js] +[browser_canvas-actor-test-10.js] [browser_canvas-frontend-call-highlight.js] [browser_canvas-frontend-call-list.js] [browser_canvas-frontend-call-search.js] diff --git a/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-08.js b/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-08.js index ab31f11255a8..618a3ab2eaa4 100644 --- a/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-08.js +++ b/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-08.js @@ -18,9 +18,7 @@ function ifTestingSupported() { ok(true, "Target automatically navigated when the front was set up."); let snapshotActor = yield front.recordAnimationFrame(); - let animationOverview = yield snapshotActor.getOverview(); - let functionCalls = animationOverview.calls; is(functionCalls[0].name, "clearRect", diff --git a/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-10.js b/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-10.js new file mode 100644 index 000000000000..07c518b830bb --- /dev/null +++ b/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-10.js @@ -0,0 +1,74 @@ +/* Any copyright is dedicated to the Public Domain. + http://creativecommons.org/publicdomain/zero/1.0/ */ + +/** + * Tests that the correct framebuffer, renderbuffer and textures are re-bound + * after generating screenshots using the actor. + */ + +function ifTestingSupported() { + let [target, debuggee, front] = yield initCanavsDebuggerBackend(WEBGL_BINDINGS_URL); + + let navigated = once(target, "navigate"); + + yield front.setup({ reload: true }); + ok(true, "The front was setup up successfully."); + + yield navigated; + ok(true, "Target automatically navigated when the front was set up."); + + let snapshotActor = yield front.recordAnimationFrame(); + let animationOverview = yield snapshotActor.getOverview(); + let functionCalls = animationOverview.calls; + + let firstScreenshot = yield snapshotActor.generateScreenshotFor(functionCalls[0]); + is(firstScreenshot.index, -1, + "The first screenshot didn't encounter any draw call."); + is(firstScreenshot.width, 128, + "The first screenshot has the correct width."); + is(firstScreenshot.height, 128, + "The first screenshot has the correct height."); + is(firstScreenshot.flipped, true, + "The first screenshot has the correct 'flipped' flag."); + is(firstScreenshot.pixels.length, 0, + "The first screenshot should be empty."); + + let gl = debuggee.gl; + is(gl.getParameter(gl.FRAMEBUFFER_BINDING), debuggee.customFramebuffer, + "The debuggee's gl context framebuffer wasn't changed."); + is(gl.getParameter(gl.RENDERBUFFER_BINDING), debuggee.customRenderbuffer, + "The debuggee's gl context renderbuffer wasn't changed."); + is(gl.getParameter(gl.TEXTURE_BINDING_2D), debuggee.customTexture, + "The debuggee's gl context texture binding wasn't changed."); + + let secondScreenshot = yield snapshotActor.generateScreenshotFor(functionCalls[1]); + is(secondScreenshot.index, 1, + "The second screenshot has the correct index."); + is(secondScreenshot.width, 128, + "The second screenshot has the correct width."); + is(secondScreenshot.height, 128, + "The second screenshot has the correct height."); + is(secondScreenshot.flipped, true, + "The second screenshot has the correct 'flipped' flag."); + is(secondScreenshot.pixels.length, 128 * 128, + "The second screenshot should not be empty."); + is(new Uint8Array(secondScreenshot.pixels.buffer)[0], 0, + "The second screenshot has the correct red component."); + is(new Uint8Array(secondScreenshot.pixels.buffer)[1], 0, + "The second screenshot has the correct green component."); + is(new Uint8Array(secondScreenshot.pixels.buffer)[2], 255, + "The second screenshot has the correct blue component."); + is(new Uint8Array(secondScreenshot.pixels.buffer)[3], 255, + "The second screenshot has the correct alpha component."); + + let gl = debuggee.gl; + is(gl.getParameter(gl.FRAMEBUFFER_BINDING), debuggee.customFramebuffer, + "The debuggee's gl context framebuffer still wasn't changed."); + is(gl.getParameter(gl.RENDERBUFFER_BINDING), debuggee.customRenderbuffer, + "The debuggee's gl context renderbuffer still wasn't changed."); + is(gl.getParameter(gl.TEXTURE_BINDING_2D), debuggee.customTexture, + "The debuggee's gl context texture binding still wasn't changed."); + + yield removeTab(target.tab); + finish(); +} diff --git a/browser/devtools/canvasdebugger/test/doc_webgl-bindings.html b/browser/devtools/canvasdebugger/test/doc_webgl-bindings.html new file mode 100644 index 000000000000..562aba69d79f --- /dev/null +++ b/browser/devtools/canvasdebugger/test/doc_webgl-bindings.html @@ -0,0 +1,60 @@ + + + + + + + WebGL editor test page + + + + + + + + + diff --git a/browser/devtools/canvasdebugger/test/head.js b/browser/devtools/canvasdebugger/test/head.js index e922c31daac5..88e2aa6abc55 100644 --- a/browser/devtools/canvasdebugger/test/head.js +++ b/browser/devtools/canvasdebugger/test/head.js @@ -30,6 +30,7 @@ const SIMPLE_BITMASKS_URL = EXAMPLE_URL + "doc_simple-canvas-bitmasks.html"; const SIMPLE_CANVAS_TRANSPARENT_URL = EXAMPLE_URL + "doc_simple-canvas-transparent.html"; const SIMPLE_CANVAS_DEEP_STACK_URL = EXAMPLE_URL + "doc_simple-canvas-deep-stack.html"; const WEBGL_ENUM_URL = EXAMPLE_URL + "doc_webgl-enum.html"; +const WEBGL_BINDINGS_URL = EXAMPLE_URL + "doc_webgl-bindings.html"; // All tests are asynchronous. waitForExplicitFinish(); diff --git a/toolkit/devtools/server/actors/canvas.js b/toolkit/devtools/server/actors/canvas.js index 5aa2556cd6e6..34147a36b6aa 100644 --- a/toolkit/devtools/server/actors/canvas.js +++ b/toolkit/devtools/server/actors/canvas.js @@ -148,7 +148,7 @@ let FrameSnapshotActor = protocol.ActorClass({ // To get a screenshot, replay all the steps necessary to render the frame, // by invoking the context calls up to and including the specified one. // This will be done in a custom framebuffer in case of a WebGL context. - let { replayContext, lastDrawCallIndex } = ContextUtils.replayAnimationFrame({ + let replayData = ContextUtils.replayAnimationFrame({ contextType: global, canvas: canvas, calls: calls, @@ -156,22 +156,23 @@ let FrameSnapshotActor = protocol.ActorClass({ last: index }); + let { replayContext, lastDrawCallIndex, doCleanup } = replayData; let screenshot; // Depending on the canvas' context, generating a screenshot is done - // in different ways. In case of the WebGL context, we also need to reset - // the framebuffer binding to the default value. + // in different ways. if (global == CallWatcherFront.CANVAS_WEBGL_CONTEXT) { screenshot = ContextUtils.getPixelsForWebGL(replayContext); - replayContext.bindFramebuffer(replayContext.FRAMEBUFFER, null); screenshot.flipped = true; - } - // In case of 2D contexts, no additional special treatment is necessary. - else if (global == CallWatcherFront.CANVAS_2D_CONTEXT) { + } else if (global == CallWatcherFront.CANVAS_2D_CONTEXT) { screenshot = ContextUtils.getPixelsFor2D(replayContext); screenshot.flipped = false; } + // In case of the WebGL context, we also need to reset the framebuffer + // binding to the original value, after generating the screenshot. + doCleanup(); + screenshot.index = lastDrawCallIndex; return screenshot; }, { @@ -368,7 +369,7 @@ let CanvasActor = exports.CanvasActor = protocol.ActorClass({ let index = this._lastDrawCallIndex; let width = this._lastContentCanvasWidth; let height = this._lastContentCanvasHeight; - let flipped = this._lastThumbnailFlipped; + let flipped = !!this._lastThumbnailFlipped; // undefined -> false let pixels = ContextUtils.getPixelStorage()["32bit"]; let lastDrawCallScreenshot = { index: index, @@ -584,29 +585,35 @@ let ContextUtils = { * @param number last * The last (inclusive) function call to end at. * @return object - * The context on which the specified calls were invoked and the - * last registered draw call's index. + * The context on which the specified calls were invoked, the + * last registered draw call's index and a cleanup function, which + * needs to be called whenever any potential followup work is finished. */ replayAnimationFrame: function({ contextType, canvas, calls, first, last }) { let w = canvas.width; let h = canvas.height; - let replayCanvas; let replayContext; let customFramebuffer; let lastDrawCallIndex = -1; + let doCleanup = () => {}; // In case of WebGL contexts, rendering will be done offscreen, in a - // custom framebuffer, but on the provided canvas context. + // custom framebuffer, but using the same provided context. This is + // necessary because it's very memory-unfriendly to rebuild all the + // required GL state (like recompiling shaders, setting global flags, etc.) + // in an entirely new canvas. However, special care is needed to not + // permanently affect the existing GL state in the process. if (contextType == CallWatcherFront.CANVAS_WEBGL_CONTEXT) { - replayCanvas = canvas; - replayContext = this.getWebGLContext(replayCanvas); - customFramebuffer = this.createBoundFramebuffer(replayContext, w, h); + let gl = replayContext = this.getWebGLContext(canvas); + let { newFramebuffer, oldFramebuffer } = this.createBoundFramebuffer(gl, w, h); + customFramebuffer = newFramebuffer; + doCleanup = () => gl.bindFramebuffer(gl.FRAMEBUFFER, oldFramebuffer); } // In case of 2D contexts, draw everything on a separate canvas context. else if (contextType == CallWatcherFront.CANVAS_2D_CONTEXT) { let contentDocument = canvas.ownerDocument; - replayCanvas = contentDocument.createElement("canvas"); + let replayCanvas = contentDocument.createElement("canvas"); replayCanvas.width = w; replayCanvas.height = h; replayContext = replayCanvas.getContext("2d"); @@ -621,23 +628,24 @@ let ContextUtils = { // to the default value, since we want to perform the rendering offscreen. if (name == "bindFramebuffer" && args[1] == null) { replayContext.bindFramebuffer(replayContext.FRAMEBUFFER, customFramebuffer); + continue; + } + if (type == CallWatcherFront.METHOD_FUNCTION) { + replayContext[name].apply(replayContext, args); + } else if (type == CallWatcherFront.SETTER_FUNCTION) { + replayContext[name] = args; } else { - if (type == CallWatcherFront.METHOD_FUNCTION) { - replayContext[name].apply(replayContext, args); - } else if (type == CallWatcherFront.SETTER_FUNCTION) { - replayContext[name] = args; - } else { - // Ignore getter calls. - } - if (CanvasFront.DRAW_CALLS.has(name)) { - lastDrawCallIndex = i; - } + // Ignore getter calls. + } + if (CanvasFront.DRAW_CALLS.has(name)) { + lastDrawCallIndex = i; } } return { replayContext: replayContext, - lastDrawCallIndex: lastDrawCallIndex + lastDrawCallIndex: lastDrawCallIndex, + doCleanup: doCleanup }; }, @@ -692,16 +700,21 @@ let ContextUtils = { * The generated framebuffer object. */ createBoundFramebuffer: function(gl, width, height) { - let framebuffer = gl.createFramebuffer(); - gl.bindFramebuffer(gl.FRAMEBUFFER, framebuffer); + let oldFramebuffer = gl.getParameter(gl.FRAMEBUFFER_BINDING); + let oldRenderbufferBinding = gl.getParameter(gl.RENDERBUFFER_BINDING); + let oldTextureBinding = gl.getParameter(gl.TEXTURE_BINDING_2D); - // Use a texture as the color rendebuffer attachment, since consumenrs of + let newFramebuffer = gl.createFramebuffer(); + gl.bindFramebuffer(gl.FRAMEBUFFER, newFramebuffer); + + // Use a texture as the color rendebuffer attachment, since consumers of // this function will most likely want to read the rendered pixels back. let colorBuffer = gl.createTexture(); gl.bindTexture(gl.TEXTURE_2D, colorBuffer); gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MAG_FILTER, gl.NEAREST); gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_MIN_FILTER, gl.NEAREST); - gl.generateMipmap(gl.TEXTURE_2D); + gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_WRAP_S, gl.CLAMP_TO_EDGE); + gl.texParameteri(gl.TEXTURE_2D, gl.TEXTURE_WRAP_T, gl.CLAMP_TO_EDGE); gl.texImage2D(gl.TEXTURE_2D, 0, gl.RGBA, width, height, 0, gl.RGBA, gl.UNSIGNED_BYTE, null); let depthBuffer = gl.createRenderbuffer(); @@ -711,10 +724,10 @@ let ContextUtils = { gl.framebufferTexture2D(gl.FRAMEBUFFER, gl.COLOR_ATTACHMENT0, gl.TEXTURE_2D, colorBuffer, 0); gl.framebufferRenderbuffer(gl.FRAMEBUFFER, gl.DEPTH_ATTACHMENT, gl.RENDERBUFFER, depthBuffer); - gl.bindTexture(gl.TEXTURE_2D, null); - gl.bindRenderbuffer(gl.RENDERBUFFER, null); + gl.bindTexture(gl.TEXTURE_2D, oldTextureBinding); + gl.bindRenderbuffer(gl.RENDERBUFFER, oldRenderbufferBinding); - return framebuffer; + return { oldFramebuffer, newFramebuffer }; } }; From 80412ceb9d2e8c302fb8eb865adb02ae7ccdd3f6 Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Tue, 22 Jul 2014 12:43:24 -0400 Subject: [PATCH 09/11] Bug 1041166 - Rename `lastDrawCallScreenshot` to `animationFrameEndScreenshot` in canvas.js, r=past --- toolkit/devtools/server/actors/canvas.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/toolkit/devtools/server/actors/canvas.js b/toolkit/devtools/server/actors/canvas.js index 34147a36b6aa..c34d53077a20 100644 --- a/toolkit/devtools/server/actors/canvas.js +++ b/toolkit/devtools/server/actors/canvas.js @@ -117,7 +117,7 @@ let FrameSnapshotActor = protocol.ActorClass({ protocol.Actor.prototype.initialize.call(this, conn); this._contentCanvas = canvas; this._functionCalls = calls; - this._lastDrawCallScreenshot = screenshot; + this._animationFrameEndScreenshot = screenshot; }, /** @@ -127,7 +127,7 @@ let FrameSnapshotActor = protocol.ActorClass({ return { calls: this._functionCalls, thumbnails: this._functionCalls.map(e => e._thumbnail).filter(e => !!e), - screenshot: this._lastDrawCallScreenshot + screenshot: this._animationFrameEndScreenshot }; }, { response: { overview: RetVal("snapshot-overview") } @@ -187,17 +187,17 @@ let FrameSnapshotActor = protocol.ActorClass({ let FrameSnapshotFront = protocol.FrontClass(FrameSnapshotActor, { initialize: function(client, form) { protocol.Front.prototype.initialize.call(this, client, form); - this._lastDrawCallScreenshot = null; + this._animationFrameEndScreenshot = null; this._cachedScreenshots = new WeakMap(); }, /** - * This implementation caches the last draw call screenshot to optimize + * This implementation caches the animation frame end screenshot to optimize * frontend requests to `generateScreenshotFor`. */ getOverview: custom(function() { return this._getOverview().then(data => { - this._lastDrawCallScreenshot = data.screenshot; + this._animationFrameEndScreenshot = data.screenshot; return data; }); }, { @@ -210,7 +210,7 @@ let FrameSnapshotFront = protocol.FrontClass(FrameSnapshotActor, { */ generateScreenshotFor: custom(function(functionCall) { if (CanvasFront.ANIMATION_GENERATORS.has(functionCall.name)) { - return promise.resolve(this._lastDrawCallScreenshot); + return promise.resolve(this._animationFrameEndScreenshot); } let cachedScreenshot = this._cachedScreenshots.get(functionCall); if (cachedScreenshot) { @@ -371,7 +371,7 @@ let CanvasActor = exports.CanvasActor = protocol.ActorClass({ let height = this._lastContentCanvasHeight; let flipped = !!this._lastThumbnailFlipped; // undefined -> false let pixels = ContextUtils.getPixelStorage()["32bit"]; - let lastDrawCallScreenshot = { + let animationFrameEndScreenshot = { index: index, width: width, height: height, @@ -384,7 +384,7 @@ let CanvasActor = exports.CanvasActor = protocol.ActorClass({ let frameSnapshot = new FrameSnapshotActor(this.conn, { canvas: this._lastDrawCallCanvas, calls: functionCalls, - screenshot: lastDrawCallScreenshot + screenshot: animationFrameEndScreenshot }); this._currentAnimationFrameSnapshot.resolve(frameSnapshot); From a8901b62ce208e182f9fe3769d68e2a52eb974ca Mon Sep 17 00:00:00 2001 From: Victor Porof Date: Tue, 22 Jul 2014 12:43:24 -0400 Subject: [PATCH 10/11] Bug 1041225 - Generating a screenshot is very slow when the content canvas has obnoxious dimensions, r=rcampbell --- .../devtools/canvasdebugger/canvasdebugger.js | 14 ++-- .../test/browser_canvas-actor-test-10.js | 30 ++++++-- .../test/doc_webgl-bindings.html | 7 +- toolkit/devtools/server/actors/canvas.js | 71 ++++++++++++++++--- 4 files changed, 97 insertions(+), 25 deletions(-) diff --git a/browser/devtools/canvasdebugger/canvasdebugger.js b/browser/devtools/canvasdebugger/canvasdebugger.js index b8131542f06f..cac1d2e7e820 100644 --- a/browser/devtools/canvasdebugger/canvasdebugger.js +++ b/browser/devtools/canvasdebugger/canvasdebugger.js @@ -207,8 +207,8 @@ let SnapshotsListView = Heritage.extend(WidgetMethods, { let thumbnail = document.createElementNS(HTML_NS, "canvas"); thumbnail.className = "snapshot-item-thumbnail"; - thumbnail.width = CanvasFront.THUMBNAIL_HEIGHT; - thumbnail.height = CanvasFront.THUMBNAIL_HEIGHT; + thumbnail.width = CanvasFront.THUMBNAIL_SIZE; + thumbnail.height = CanvasFront.THUMBNAIL_SIZE; let title = document.createElement("label"); title.className = "plain snapshot-item-title"; @@ -712,14 +712,16 @@ let CallsListView = Heritage.extend(WidgetMethods, { * A single "snapshot-image" instance received from the backend. */ showScreenshot: function(screenshot) { - let { index, width, height, flipped, pixels } = screenshot; + let { index, width, height, scaling, flipped, pixels } = screenshot; let screenshotNode = $("#screenshot-image"); screenshotNode.setAttribute("flipped", flipped); drawBackground("screenshot-rendering", width, height, pixels); let dimensionsNode = $("#screenshot-dimensions"); - dimensionsNode.setAttribute("value", ~~width + " x " + ~~height); + let actualWidth = (width / scaling) | 0; + let actualHeight = (height / scaling) | 0; + dimensionsNode.setAttribute("value", actualWidth + " x " + actualHeight); window.emit(EVENTS.CALL_SCREENSHOT_DISPLAYED); }, @@ -754,8 +756,8 @@ let CallsListView = Heritage.extend(WidgetMethods, { let thumbnailNode = document.createElementNS(HTML_NS, "canvas"); thumbnailNode.setAttribute("flipped", flipped); - thumbnailNode.width = Math.max(CanvasFront.THUMBNAIL_HEIGHT, width); - thumbnailNode.height = Math.max(CanvasFront.THUMBNAIL_HEIGHT, height); + thumbnailNode.width = Math.max(CanvasFront.THUMBNAIL_SIZE, width); + thumbnailNode.height = Math.max(CanvasFront.THUMBNAIL_SIZE, height); drawImage(thumbnailNode, width, height, pixels, { centered: true }); thumbnailNode.className = "filmstrip-thumbnail"; diff --git a/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-10.js b/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-10.js index 07c518b830bb..bf29eb65960f 100644 --- a/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-10.js +++ b/browser/devtools/canvasdebugger/test/browser_canvas-actor-test-10.js @@ -24,9 +24,11 @@ function ifTestingSupported() { let firstScreenshot = yield snapshotActor.generateScreenshotFor(functionCalls[0]); is(firstScreenshot.index, -1, "The first screenshot didn't encounter any draw call."); - is(firstScreenshot.width, 128, + is(firstScreenshot.scaling, 0.25, + "The first screenshot has the correct scaling."); + is(firstScreenshot.width, CanvasFront.WEBGL_SCREENSHOT_MAX_HEIGHT, "The first screenshot has the correct width."); - is(firstScreenshot.height, 128, + is(firstScreenshot.height, CanvasFront.WEBGL_SCREENSHOT_MAX_HEIGHT, "The first screenshot has the correct height."); is(firstScreenshot.flipped, true, "The first screenshot has the correct 'flipped' flag."); @@ -40,17 +42,27 @@ function ifTestingSupported() { "The debuggee's gl context renderbuffer wasn't changed."); is(gl.getParameter(gl.TEXTURE_BINDING_2D), debuggee.customTexture, "The debuggee's gl context texture binding wasn't changed."); + is(gl.getParameter(gl.VIEWPORT)[0], 128, + "The debuggee's gl context viewport's left coord. wasn't changed."); + is(gl.getParameter(gl.VIEWPORT)[1], 256, + "The debuggee's gl context viewport's left coord. wasn't changed."); + is(gl.getParameter(gl.VIEWPORT)[2], 384, + "The debuggee's gl context viewport's left coord. wasn't changed."); + is(gl.getParameter(gl.VIEWPORT)[3], 512, + "The debuggee's gl context viewport's left coord. wasn't changed."); let secondScreenshot = yield snapshotActor.generateScreenshotFor(functionCalls[1]); is(secondScreenshot.index, 1, "The second screenshot has the correct index."); - is(secondScreenshot.width, 128, + is(secondScreenshot.width, CanvasFront.WEBGL_SCREENSHOT_MAX_HEIGHT, "The second screenshot has the correct width."); - is(secondScreenshot.height, 128, + is(secondScreenshot.height, CanvasFront.WEBGL_SCREENSHOT_MAX_HEIGHT, "The second screenshot has the correct height."); + is(secondScreenshot.scaling, 0.25, + "The second screenshot has the correct scaling."); is(secondScreenshot.flipped, true, "The second screenshot has the correct 'flipped' flag."); - is(secondScreenshot.pixels.length, 128 * 128, + is(secondScreenshot.pixels.length, Math.pow(CanvasFront.WEBGL_SCREENSHOT_MAX_HEIGHT, 2), "The second screenshot should not be empty."); is(new Uint8Array(secondScreenshot.pixels.buffer)[0], 0, "The second screenshot has the correct red component."); @@ -68,6 +80,14 @@ function ifTestingSupported() { "The debuggee's gl context renderbuffer still wasn't changed."); is(gl.getParameter(gl.TEXTURE_BINDING_2D), debuggee.customTexture, "The debuggee's gl context texture binding still wasn't changed."); + is(gl.getParameter(gl.VIEWPORT)[0], 128, + "The debuggee's gl context viewport's left coord. still wasn't changed."); + is(gl.getParameter(gl.VIEWPORT)[1], 256, + "The debuggee's gl context viewport's left coord. still wasn't changed."); + is(gl.getParameter(gl.VIEWPORT)[2], 384, + "The debuggee's gl context viewport's left coord. still wasn't changed."); + is(gl.getParameter(gl.VIEWPORT)[3], 512, + "The debuggee's gl context viewport's left coord. still wasn't changed."); yield removeTab(target.tab); finish(); diff --git a/browser/devtools/canvasdebugger/test/doc_webgl-bindings.html b/browser/devtools/canvasdebugger/test/doc_webgl-bindings.html index 562aba69d79f..eb1405359f65 100644 --- a/browser/devtools/canvasdebugger/test/doc_webgl-bindings.html +++ b/browser/devtools/canvasdebugger/test/doc_webgl-bindings.html @@ -9,7 +9,7 @@ - +