[wasm][debugger] Removing console.debug message helper to make debugger work. by thaystg · Pull Request #72812 · dotnet/runtime

thaystg · 2022-07-25T20:08:14Z

For debugging on chrome it was already unused, if we just remove the message and don't change anything else the debugging continues working, using chrome as ide, using visual studio and also visual studio code.
On firefox it was using the message, but I changed to have the same behavior that has for chrome.
For the debugger tests I created a new message named DotnetDebugger.runTests which will be called when the page is ready to start running the tests.

also:

renaming wrong file name for firefox
fix pause in a breakpoint on firefox

Fixes#63703

ghost · 2022-07-25T20:08:28Z

Tagging subscribers to this area: @thaystg
See info in area-owners.md if you want to be subscribed.

Issue Details

For debugging on chrome it was already unused, if we just remove the message and don't change anything else the debugging continues working, using chrome as ide, using visual studio and also visual studio code.
On firefox it was using the message, but I changed to have the same behavior that has for chrome.
For the debugger tests I printed the expected message in the html files as we don't really have an IDE, to receive the equivalent messages and check if runtime is ready.

also:

renaming wrong file name for firefox
fix pause in a breakpoint on firefox

Author:	thaystg
Assignees:	-
Labels:	`area-Debugger-mono`
Milestone:	-

Co-authored-by: Larry Ewing <lewing@microsoft.com>

pavelsavara · 2022-07-27T17:55:52Z

+ ctx.GlobalName = args["target"]["actor"].Value<string>();
+ ctx.ThreadName = args["target"]["threadActor"].Value<string>();
+ ResetCmdId();
+ if (await IsRuntimeAlreadyReadyAlready(sessionId, token))


funny name IsRuntimeAlreadyReadyAlready :-D

pavelsavara · 2022-07-27T17:56:51Z

/azp run runtime-wasm

azure-pipelines · 2022-07-27T17:57:06Z

Azure Pipelines successfully started running 1 pipeline(s).

ghost · 2022-07-28T20:32:45Z

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

For debugging on chrome it was already unused, if we just remove the message and don't change anything else the debugging continues working, using chrome as ide, using visual studio and also visual studio code.
On firefox it was using the message, but I changed to have the same behavior that has for chrome.
For the debugger tests I printed the expected message in the html files as we don't really have an IDE, to receive the equivalent messages and check if runtime is ready.

also:

renaming wrong file name for firefox
fix pause in a breakpoint on firefox

Fixes #63703

Author:	thaystg
Assignees:	thaystg
Labels:	`arch-wasm`, `area-Debugger-mono`
Milestone:	-

radical · 2022-07-28T20:46:14Z

just fyi, runtime-wasm isn't needed for debugger tests specifically. This is because the debugger tests job is triggered by any changes in src/mono/wasm/debugger. But if you have changes anywhere else for which you want to run debugger-tests, then runtime-wasm could be useful.

lewing · 2022-08-15T04:21:23Z

 	<script type='text/javascript'>
 var App = {
 init: function () {
+ console.debug ("mono_wasm_runtime_ready", "fe00e07a-5519-4dfe-b35a-f867dbaf2e28");


Is something depending on this behavior? if so we should fix it?

The firefox debugger tests.
But @radical changed other things and now this PR is old, I need to merge main and also I'm planning to fix this:
#73651 in the same PR, then we can totally remove this message probably.

thaystg · 2023-01-04T17:31:51Z

/azp run runtime-wasm-non-libtests

azure-pipelines · 2023-01-04T17:32:01Z

Azure Pipelines successfully started running 1 pipeline(s).

pavelsavara · 2023-01-05T09:48:58Z

Could you also please delete

runtime/src/mono/sample/wasm/browser-bench/README.md

Line 28 in 83ce125

console.debug: mono_wasm_runtime_ready fe00e07a-5519-4dfe-b35a-f867dbaf2e28

and

runtime/src/mono/wasm/README.md

Line 197 in 83ce125

mono_wasm_runtime_ready fe00e07a-5519-4dfe-b35a-f867dbaf2e28

pavelsavara · 2023-01-05T09:58:23Z

If we are no longer sending that message, could we also delete

runtime/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs

Lines 334 to 335 in 83ce125

publicconststringRUNTIME_IS_READY="mono_wasm_runtime_ready";
publicconststringRUNTIME_IS_READY_ID="fe00e07a-5519-4dfe-b35a-f867dbaf2e28";

And

runtime/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs

Lines 107 to 127 in 83ce125

if(aCount>=2&&
a[0]?["value"]?.ToString()==MonoConstants.RUNTIME_IS_READY&&
a[1]?["value"]?.ToString()==MonoConstants.RUNTIME_IS_READY_ID)
{
if(aCount>2)
{
try
{
// The optional 3rd argument is the stringified assembly
// list so that we don't have to make more round trips
stringloaded=a[2]?["value"]?.ToString();
if(loaded!=null)
context.LoadedFiles=JToken.Parse(loaded).ToObject<string[]>();
}
catch(InvalidCastExceptionice)
{
Log("verbose",ice.ToString());
}
}
awaitRuntimeReady(sessionId,token);
}

I'm bit confused about if it's possible.

thaystg · 2023-01-06T18:01:33Z

If we are no longer sending that message, could we also delete
runtime/src/mono/wasm/debugger/BrowserDebugProxy/DevToolsHelper.cs
Lines 334 to 335 in 83ce125
publicconststringRUNTIME_IS_READY="mono_wasm_runtime_ready";
publicconststringRUNTIME_IS_READY_ID="fe00e07a-5519-4dfe-b35a-f867dbaf2e28";
And
runtime/src/mono/wasm/debugger/BrowserDebugProxy/MonoProxy.cs
Lines 107 to 127 in 83ce125
if(aCount>=2&&
a[0]?["value"]?.ToString()==MonoConstants.RUNTIME_IS_READY&&
a[1]?["value"]?.ToString()==MonoConstants.RUNTIME_IS_READY_ID)
{
if(aCount>2)
{
try
{
// The optional 3rd argument is the stringified assembly
// list so that we don't have to make more round trips
stringloaded=a[2]?["value"]?.ToString();
if(loaded!=null)
context.LoadedFiles=JToken.Parse(loaded).ToObject<string[]>();
}
catch(InvalidCastExceptionice)
{
Log("verbose",ice.ToString());
}
}
awaitRuntimeReady(sessionId,token);
}
I'm bit confused about if it's possible.

After your comment I tried another approach which is completely removing the message. :)

thaystg · 2023-01-06T18:01:47Z

/azp run runtime-wasm-non-libtests

azure-pipelines · 2023-01-06T18:01:56Z

Azure Pipelines successfully started running 1 pipeline(s).

pavelsavara

I guess you tested it manually too?

thaystg · 2023-01-09T12:18:46Z

I guess you tested it manually too?

Yes, I did :)

thaystg · 2023-01-09T12:19:48Z

@ilonatommy can you double check if I didn't break anything, please? Test it on blazor using VS, using VSCode and using ctrl-shift-d from chrome?

ilonatommy · 2023-01-09T14:34:01Z

@ilonatommy can you double check if I didn't break anything, please? Test it on blazor using VS, using VSCode and using ctrl-shift-d from chrome?

Working. Checked:

VS debugger
VS Code
Chrome Dev tools

ilonatommy

Looks cool!

thaystg · 2023-01-09T14:47:44Z

/azp run runtime-wasm-non-libtests

azure-pipelines · 2023-01-09T14:47:53Z

Azure Pipelines successfully started running 1 pipeline(s).

Removing console.debug message helper to make debugger work.
c83255c

ghost added the area-Debugger-mono label Jul 25, 2022

ghost assigned thaystg Jul 25, 2022

pavelsavara reviewed Jul 26, 2022
View reviewed changes

Comment threadsrc/mono/wasm/debugger/tests/debugger-test/debugger-driver.html Outdated

pavelsavara mentioned this pull request Jul 26, 2022
[wasm] make debugger less noisy #72675
Closed

lewing reviewed Jul 26, 2022
View reviewed changes

Comment threadsrc/mono/wasm/runtime/debug.ts Outdated

Update src/mono/wasm/runtime/debug.ts
a3f853b
Co-authored-by: Larry Ewing <lewing@microsoft.com>

thaystg marked this pull request as ready for review July 26, 2022 19:56

thaystg requested a review from radical as a code owner July 26, 2022 19:56

runfoappBot mentioned this pull request Jul 26, 2022
System.Net.* cancellation tests fail in CI #72818
Closed

pavelsavara reviewed Jul 27, 2022
View reviewed changes

radical added the arch-wasm WebAssembly architecture label Jul 28, 2022

radical added this to the 7.0.0 milestone Jul 28, 2022

Addressing@radicalcomments offline.
70d49fb

thaystg mentioned this pull request Aug 12, 2022
[wasm] Do not print mono_wasm_runtime_ready to console #63703
Closed

thaystg removed this from the 7.0.0 milestone Aug 12, 2022

lewing reviewed Aug 15, 2022
View reviewed changes

lewing added this to the 8.0.0 milestone Aug 15, 2022

ilonatommyand others added 3 commits December 7, 2022 13:39

Merge branch 'main' into thays_remove_runtime_ready_message
cf25b33

Merge branch 'main' into thays_remove_runtime_ready_message
4edec00

Fixing merge.
361fd76

Fix line change error.
52f13dc

Totally removing the message.
ee1e211

thaystg requested review from lewing and pavelsavara January 6, 2023 18:32

pavelsavara approved these changes Jan 6, 2023
View reviewed changes

ilonatommy reviewed Jan 9, 2023
View reviewed changes

Comment threadsrc/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs Outdated

ilonatommy approved these changes Jan 9, 2023
View reviewed changes

Remove unused comment
50cfc85

thaystg merged commit 8a2b72e into dotnet:mainJan 10, 2023

thaystg mentioned this pull request Jan 30, 2023
[wasm][debugger] Fix firefox tests #81364
Merged

ghost locked as resolved and limited conversation to collaborators Feb 9, 2023

Uh oh!

Conversation

thaystg commented Jul 25, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ghost commented Jul 25, 2022

Uh oh!

Uh oh!

Uh oh!

pavelsavaraJul 27, 2022

Choose a reason for hiding this comment

Uh oh!

pavelsavara commented Jul 27, 2022

Uh oh!

azure-pipelinesBot commented Jul 27, 2022

Uh oh!

ghost commented Jul 28, 2022

Uh oh!

radical commented Jul 28, 2022

Uh oh!

lewingAug 15, 2022

Choose a reason for hiding this comment

Uh oh!

thaystgAug 15, 2022

Choose a reason for hiding this comment

Uh oh!

thaystg commented Jan 4, 2023

Uh oh!

azure-pipelinesBot commented Jan 4, 2023

Uh oh!

pavelsavara commented Jan 5, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pavelsavara commented Jan 5, 2023

Uh oh!

thaystg commented Jan 6, 2023

Uh oh!

thaystg commented Jan 6, 2023

Uh oh!

azure-pipelinesBot commented Jan 6, 2023

Uh oh!

pavelsavara left a comment

Choose a reason for hiding this comment

Uh oh!

thaystg commented Jan 9, 2023

Uh oh!

thaystg commented Jan 9, 2023

Uh oh!

ilonatommy commented Jan 9, 2023

Uh oh!

Uh oh!

ilonatommy left a comment

Choose a reason for hiding this comment

Uh oh!

thaystg commented Jan 9, 2023

Uh oh!

azure-pipelinesBot commented Jan 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

thaystg commented Jul 25, 2022•
edited
Loading

pavelsavara commented Jan 5, 2023•
edited
Loading