[Webkit-unassigned] [Bug 37930] [Win] Implement LayoutTestController::markerTextForListItem()

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=37930


Adam Roben (aroben) <aroben at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #61605|review?                     |review-
               Flag|                            |




--- Comment #4 from Adam Roben (aroben) <aroben at apple.com>  2010-07-15 12:13:13 PST ---
(From update of attachment 61605)
> +HRESULT WebFrame::elementFromJS(JSContextRef context, JSValueRef nodeObject, IDOMElement **element)
> +{
> +    if (!element)
> +        return E_POINTER;
> +
> +    *element = 0;
> +
> +    if (!context)
> +        return E_FAIL;
> +
> +    JSLock lock(SilenceAssertionsOnly);
> +    ExecState* execState = toJS(context);
> +    if (!nodeObject)
> +        return E_FAIL;

I'd null-check nodeObject at the same time as context.

> +    JSValue jsValue = toJS(execState, nodeObject);

I don't think the execState or jsValue local variables are useful.

> @@ -114,4 +114,6 @@ interface IWebFramePrivate : IUnknown
>      HRESULT paintScrollViewRectToContextAtPoint([in] RECT rect, [in] POINT pt, [in] OLE_HANDLE deviceContext);
>  
>      HRESULT renderTreeAsExternalRepresentation([in] BOOL forPrinting, [out, retval] BSTR* result);
> +
> +    [local] HRESULT elementFromJS([in] JSContextRef context, [in] JSValueRef nodeObject, [out, retval] IDOMElement** element);

Putting this on IWebViewPrivate would be a little more conventional, I think.

> +    if (!framePrivate)
> +        return JSRetainPtr<JSStringRef>();

You can say "return 0" instead of "return JSRetainPtr<JSStringRef>()".

> +    wstring text(textBSTR, SysStringLen(textBSTR));
> +    SysFreeString(textBSTR);
> +    JSRetainPtr<JSStringRef> markerText(Adopt, JSStringCreateWithCharacters(text.data(), text.length()));
> +    return markerText;
>  }

Again, you can use JSStringCreateWithBSTR to make this easier and cleaner.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.