Bug 108776 – Web Inspector: prevent crash, add required error string value

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Peter Rybin]

Make inspector more fair to either-result-or-error-string convention.

[Peter Rybin]

Created attachment 186275 [details]
Patch

[Build Bot]

Comment on attachment 186275 [details]
Patch

Attachment 186275 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://queues.webkit.org/results/16359804

New failing tests:
inspector/debugger/watch-expressions-preserve-expansion.html

[Build Bot]

Comment on attachment 186275 [details]
Patch

Attachment 186275 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16354870

[Build Bot]

Comment on attachment 186275 [details]
Patch

Attachment 186275 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16355911

New failing tests:
inspector/debugger/watch-expressions-preserve-expansion.html

[Build Bot]

Comment on attachment 186275 [details]
Patch

Attachment 186275 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/16352918

New failing tests:
inspector/debugger/watch-expressions-preserve-expansion.html

[WebKit Review Bot]

Comment on attachment 186275 [details]
Patch

Attachment 186275 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/16359826

New failing tests:
inspector/debugger/watch-expressions-preserve-expansion.html

[Pavel Feldman]

Comment on attachment 186275 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=186275&action=review

> Source/WebCore/inspector/InspectorRuntimeAgent.cpp:84
> +        *errorString = "Capybara, capybara capybara capybara: capybara!";

Please use proper wording.

[Peter Rybin]

(In reply to comment #7)
> (From update of attachment 186275 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=186275&action=review
> 
> > Source/WebCore/inspector/InspectorRuntimeAgent.cpp:84
> > +        *errorString = "Capybara, capybara capybara capybara: capybara!";
> 
> Please use proper wording.

I meant I didn't know what the proper wording was here. Could you please advise?

[Peter Rybin]

> I meant I didn't know what the proper wording was here. Could you please advise?

Oh, wait. That's something stupid here, error string must be already provided. Let me watch again.

[Peter Rybin]

Created attachment 186497 [details]
Patch

[Peter Rybin]

(In reply to comment #10)
> Patch

Please advise what a proper wording should be instead of bogus message currently in patch. This is a protocol-visible message.

[Pavel Feldman]

Comment on attachment 186497 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=186497&action=review

> Source/WebCore/inspector/PageRuntimeAgent.cpp:141
> +            *errorString = "Main world execution context not found. Capybara! Capybara!";

Is this some kind of joke?

[Peter Rybin]

Created attachment 186586 [details]
Patch

[Peter Rybin]

> > Source/WebCore/inspector/PageRuntimeAgent.cpp:141
> > +            *errorString = "Main world execution context not found. Capybara! Capybara!";
> 
> Is this some kind of joke?

Fixed.

[Pavel Feldman]

Comment on attachment 186586 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=186586&action=review

> Source/WebCore/inspector/PageRuntimeAgent.cpp:140
> +            // FIXME(code.review): correct or confirm the message.

Please remove this fixme.

> Source/WebCore/inspector/PageRuntimeAgent.cpp:141
> +            *errorString = "Main world execution context not found.";

"Internal error: main world execution context not found." ?

[Peter Rybin]

Created attachment 186610 [details]
Patch

[WebKit Review Bot]

Comment on attachment 186610 [details]
Patch

Clearing flags on attachment: 186610

Committed r141891: <http://trac.webkit.org/changeset/141891>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.