1. Open Web Inspector 2. Go to Scripts tab and choose 'Always enabled' 3. Restart browser 4. Open Web Inspector Expected: window opens Actual: window opens and gets into non-responsive mode. Browser halts.
Created attachment 30909 [details] fix
Comment on attachment 30909 [details] fix > - enableDebugger(); > + m_frontend->attachDebuggerWhenShown(); Can't you just leave this calling enableDebugger() since you fixed that function? > disableDebugger(); > + m_attachDebuggerWhenShown = true; Why is this needed? Shoudln't this be m_attachDebuggerWhenShown = false? If it is supose to be false, should disableDebugger() set it to false? If it isn't those things, maybe a comment is needed to explain. > - enableDebugger(); > + m_attachDebuggerWhenShown = true; Same as before. Can't you just leave this calling enableDebugger() since you fixed that function? > + this._attachDebuggerWhenShown = false; I like to use delete this._attachDebuggerWhenShown for cases like this, since there is no benifit to keeping this property around. r+, but maybe this can be a smaller change based on the comments above.
Created attachment 30911 [details] fix (In reply to comment #2) > (From update of attachment 30909 [details] [review]) > > - enableDebugger(); > > + m_frontend->attachDebuggerWhenShown(); > > Can't you just leave this calling enableDebugger() since you fixed that > function? > Done > > disableDebugger(); > > + m_attachDebuggerWhenShown = true; > > Why is this needed? Shoudln't this be m_attachDebuggerWhenShown = false? If it > is supose to be false, should disableDebugger() set it to false? If it isn't > those things, maybe a comment is needed to explain. > > > - enableDebugger(); > > + m_attachDebuggerWhenShown = true; > I added a comment for this. The scenario is: close window when debugger is enabled, expect it to be enabled when the window is being reopened. The hidden thing is that debugger is being disabled on window close unconditionally. > Same as before. Can't you just leave this calling enableDebugger() since you > fixed that function? > Done. It is just that I fixed the function after I changed these. > > + this._attachDebuggerWhenShown = false; > > I like to use delete this._attachDebuggerWhenShown for cases like this, since > there is no benifit to keeping this property around. > Done. > r+, but maybe this can be a smaller change based on the comments above. > Thanks!
Assign for landing.
Landed in @r44398.