1058879 - Enable canvasdebugger tests for e10s

Status: RESOLVED FIXED

(DevTools Graveyard :: Canvas Debugger, defect)

Description

[Brian Grinstead [:bgrins]]

These are currently failing on Holley: https://tbpl.mozilla.org/?tree=Holly.  Here is an example: https://tbpl.mozilla.org/php/getParsedLog.php?id=46482670.

Seems the call to target.window.wrappedJSObject is failing: http://dxr.mozilla.org/mozilla-central/source/browser/devtools/canvasdebugger/test/head.js#188.

Comment 1

[Brian Grinstead [:bgrins]]

Attachment: canvasdebugger-e10s-skip.patch

First step: skip the failing tests so we can get the tree green

Comment 2

[Brian Grinstead [:bgrins]]

https://tbpl.mozilla.org/?tree=Try&rev=4d3de8a8f07d

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/21496f6351e4

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/21496f6351e4

Comment 5

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0002-Bug-1058879-enable-most-canvasdebugger-tests-in-e10s.patch

This patch is very similar to the one in bug 1072141 etc.  A key difference here is that there is a single test that relies on the "debuggee" - so this patch just disables that single test in e10s (and thus, I'll keep the "leave-open" annotation to track that one final test)

Comment 6

[Patrick Brosset <:pbro>]

Comment on attachment 8494902 [details] [diff] [review]
0002-Bug-1058879-enable-most-canvasdebugger-tests-in-e10s.patch

Review of attachment 8494902 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM.

Comment 7

[Mark Hammond [:markh] [:mhammond]]

https://hg.mozilla.org/integration/fx-team/rev/f26dcd7a38ac

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f26dcd7a38ac

Comment 9

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1058879-enable-canvas-test-e10s.patch

This enables browser_canvas-actor-test-10.js

Comment 10

[Jordan Santell [:jsantell] [@jsantell]]

try build for none10s, passes locally with e10s
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=573756bd0fe7

Comment 11

[Victor Porof [:vporof][:vp]]

Comment on attachment 8505109 [details] [diff] [review]
1058879-enable-canvas-test-e10s.patch

Review of attachment 8505109 [details] [diff] [review]:
-----------------------------------------------------------------

Nice.

Comment 12

[Jordan Santell [:jsantell] [@jsantell]]

I believe this is the last patch needed for this tool to be enabled in e10s.

Comment 13

[Carsten Book [:Tomcat]]

Hi Jordan, the patch didn't apply cleanly:

patching file browser/devtools/shared/frame-script-utils.js
Hunk #1 FAILED at 11
1 out of 1 hunks FAILED -- saving rejects to file browser/devtools/shared/frame-script-utils.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh 1058879-enable-canvas-test-e10s.patch

could you take a look, thanks!

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8505109 [details] [diff] [review]
1058879-enable-canvas-test-e10s.patch

Presumably you want to remove leave-open too?

Comment 15

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: 1058879-enable-canvas-test-e10s.patch

Rebased and good to go!

Comment 16

[Jordan Santell [:jsantell] [@jsantell]]

Removing the [leave-open]

Comment 17

[Carsten Book [:Tomcat]]

Comment on attachment 8513753 [details] [diff] [review]
1058879-enable-canvas-test-e10s.patch

remote:   https://hg.mozilla.org/integration/fx-team/rev/aa97afb0a949

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/aa97afb0a949