[wasm][debugger] Removing console.debug message helper to make debugger work.#72812

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:	-

src/mono/wasm/debugger/tests/debugger-test/debugger-driver.html

src/mono/wasm/runtime/debug.ts

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

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

src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs

+                    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

src/mono/wasm/debugger/tests/debugger-test/wasm-page-without-assets.html

 	<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

    
           public const string RUNTIME_IS_READY = "mono_wasm_runtime_ready"; 
        
           public const string RUNTIME_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 
        
                       string loaded = a[2]?["value"]?.ToString(); 
        
                       if (loaded != null) 
        
                           context.LoadedFiles = JToken.Parse(loaded).ToObject<string[]>(); 
        
                   } 
        
                   catch (InvalidCastException ice) 
        
                   { 
        
                       Log("verbose", ice.ToString()); 
        
                   } 
        
               } 
        
               await RuntimeReady(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

public const string RUNTIME_IS_READY = "mono_wasm_runtime_ready";

public const string RUNTIME_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

string loaded = a[2]?["value"]?.ToString();

if (loaded != null)

context.LoadedFiles = JToken.Parse(loaded).ToObject<string[]>();

}

catch (InvalidCastException ice)

{

Log("verbose", ice.ToString());

}

}

await RuntimeReady(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

src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs

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

src/mono/wasm/debugger/tests/debugger-test/debugger-driver.html Outdated Show resolved Hide resolved

pavelsavara mentioned this pull request Jul 26, 2022

[wasm] make debugger less noisy #72675

Closed

lewing reviewed Jul 26, 2022

View reviewed changes

src/mono/wasm/runtime/debug.ts Outdated Show resolved Hide resolved

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

runfoapp bot 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 @radical comments 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

ilonatommy and 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

src/mono/wasm/debugger/BrowserDebugProxy/Firefox/FirefoxMonoProxy.cs Outdated Show resolved Hide resolved

ilonatommy approved these changes Jan 9, 2023

View reviewed changes

Remove unused comment

50cfc85

thaystg merged commit 8a2b72e into dotnet:main Jan 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

Comments

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!

pavelsavara Jul 27, 2022

Choose a reason for hiding this comment

Uh oh!

pavelsavara commented Jul 27, 2022

Uh oh!

azure-pipelines bot commented Jul 27, 2022

Uh oh!

ghost commented Jul 28, 2022

Uh oh!

radical commented Jul 28, 2022

Uh oh!

lewing Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

thaystg Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

thaystg commented Jan 4, 2023

Uh oh!

azure-pipelines bot 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-pipelines bot 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-pipelines bot 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