[webkit-reviews] review granted: [Bug 201201] REGRESSION (r249078): Flaky crash in com.apple.JavaScriptCore: Inspector::InjectedScriptModule::ensureInjected : [Attachment 377758] Patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Devin Rousso
<drousso at apple.com>'s request for review:
Bug 201201: REGRESSION (r249078): Flaky crash in com.apple.JavaScriptCore:
Inspector::InjectedScriptModule::ensureInjected
https://bugs.webkit.org/show_bug.cgi?id=201201

Attachment 377758: Patch

https://bugs.webkit.org/attachment.cgi?id=377758&action=review




--- Comment #5 from Joseph Pecoraro <joepeck at webkit.org> ---
Comment on attachment 377758
  --> https://bugs.webkit.org/attachment.cgi?id=377758
Patch

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

Seems fine. I actually liked the old methods better, but I don't have a strong
preference.

> Source/JavaScriptCore/inspector/InjectedScriptModule.cpp:75
> +	   hadException = false;

This shouldn't be necessary.

> Source/JavaScriptCore/inspector/InjectedScriptSource.js:-436
> -    setInspectObject(callback)
> -    {
> -	   this._inspectObject = callback;
> -    }
> -
> -    addCommandLineAPIGetter(name, func)
> -    {
> -	   InjectedScript.CommandLineAPI._getters.push({name, func});
> -    }
> -
> -    addCommandLineAPIMethod(name, func)
> -    {
> -	   func.toString = function() { return "function " + name + "() {
[Command Line API] }" };
> -	   InjectedScript.CommandLineAPI._methods.push({name, func});
> -    }

I found this approach really clean!