Closed Bug 917188 Opened 7 years ago Closed 7 years ago
Use a more deterministic way to write the browser
_webconsole _view _source .js test
This test makes sure that when we click on a source link in the web console that corresponds to a script that has been garbage-collected, view-source will open instead of the debugger. Since the behavior and the test are GC-dependent, bug 916995 added a Cu.forceGC() call to make it more explicit and less prone to oranges. As discussed with Jim on IRC, we should be writing the test in a way that would guarantee it will pass regardless of whether the garbage collector decides to kick at some particular time. Jim's idea was to make the ThreadActor configurable from tests so that it can lie about a script being present in the debuggee. Since this is a browser chrome test, we need a way to get to the thread actor from the client. Luckily we already have such a method via the _serverConnection property in the client transport object: http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/server/main.js#482 I propose we add an _ignoreScripts flag on the thread actor and have the test set it to true, so that the onSources() request handler can consult it and return an empty source list for this test. I'm filing this bug in the debugger component as most of the necessary changes need to be made to the script.js file.
I propose a simpler approach that doesn't involve changes in the webconsole/debugger code: the test can override the DebuggerView.Sources.containsValue() to lie. This is what I do with prompt() calls or openInTab() calls, and so on.
I suggested something similar yesterday, to have the console use a fabricated URL that was guaranteed not to be found, but I like your idea better than mine. Jim made the point that we should have the test actually exercise the code path we care about, but in this case, I believe it's just the web console's code path that we want to test.
(In reply to Panos Astithas [:past] from comment #2) > I suggested something similar yesterday, to have the console use a > fabricated URL that was guaranteed not to be found, but I like your idea > better than mine. Jim made the point that we should have the test actually > exercise the code path we care about, but in this case, I believe it's just > the web console's code path that we want to test. Sure --- if that's the intent of the test, then 'mocking' the absence of the script at that level could make sense.
Yes, that is the intent of the test - check that the web console opens view source, when the script is not available in the debugger.
This should work. Try push: https://tbpl.mozilla.org/?tree=Try&rev=c12d7ac5cc42 Should we ask bz to also test this patch with his DOM patches?
Assignee: nobody → mihai.sucan
Status: NEW → ASSIGNED
Attachment #806856 - Flags: review?(past)
Comment on attachment 806856 [details] [diff] [review] bug-917188-1-fx-team.diff Review of attachment 806856 [details] [diff] [review]: ----------------------------------------------------------------- (In reply to Mihai Sucan [:msucan] from comment #5) > Should we ask bz to also test this patch with his DOM patches? Yes, that's a good idea as his main patch seems not to have landed yet.
https://tbpl.mozilla.org/?tree=Try&rev=01f066669a62 says it looks good.
Attachment #806856 - Flags: feedback?(bzbarsky) → feedback+
Thank you Panos and Boris!
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
You need to log in before you can comment on or make changes to this bug.