[webkit-reviews] review granted: [Bug 116191] Web Inspector: InspectorInstrumentation::willEvaluateScript should include column number : [Attachment 358715] 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 116191: Web Inspector: InspectorInstrumentation::willEvaluateScript should
include column number
https://bugs.webkit.org/show_bug.cgi?id=116191

Attachment 358715: Patch

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




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

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

Nice! r=me.

We don't have any tests on Timeline events right now, we should tackle that at
some point.

> Source/WebCore/ChangeLog:9
> +	   No new tests (OOPS!).

Seems we can have a test.

> Source/WebCore/inspector/TimelineRecordFactory.cpp:107
> -Ref<JSON::Object> TimelineRecordFactory::createEvaluateScriptData(const
String& url, double lineNumber)
> +Ref<JSON::Object> TimelineRecordFactory::createEvaluateScriptData(const
String& url, double lineNumber, double columnNumber)

Hmm, why are these doubles? Maybe we can convert them to ints up the chain.
JSON::Object::setInteger implicitly converts it to an int anyways.