COMPropertyBag template class needs to implement IPropertyBag2 methods

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Steve Falkenburg]

COMPropertyBag in WebKit/win doesn't implement IPropertyBag2::Read.
It also doesn't fill in the dwType for the PROPBAG2 structure.

[Steve Falkenburg]

Created attachment 25751 [details]
Implemented method, filled in type field.

[Alice Liu]

Comment on attachment 25751 [details]
Implemented method, filled in type field.

> Index: ChangeLog
> ===================================================================
> --- ChangeLog	(revision 39005)
> +++ ChangeLog	(working copy)
> @@ -1,3 +1,17 @@
> [phone]  Steve Falkenburg  <[e-mail]>
> +
> +        Implement IPropertyBag2::Read.
> +        Fill in dwType for IPropertyBag2::GetPropertyInfo (we always use PROPBAG2_TYPE_DATA).
> +        https://bugs.webkit.org/show_bug.cgi?id=22659
> +        
> +        This fixes <rdar://problem/6419127> REGRESSION: Forms autocomplete is broken for "other forms"
> +        
> +        Reviewed by NOBODY (OOPS!).
> +
> +        * COMPropertyBag.h:
> +        (::Read): Implemented.
> +        (::GetPropertyInfo): Set dwType to PROPBAG2_TYPE_DATA.
> +
>  2008-12-02  Adam Roben  <[e-mail]>
>  
>          Export new-ish JSCore convenience constructor functions
> Index: COMPropertyBag.h
> ===================================================================
> --- COMPropertyBag.h	(revision 38999)
> +++ COMPropertyBag.h	(working copy)
> @@ -158,9 +158,22 @@ HRESULT STDMETHODCALLTYPE COMPropertyBag
>  }
>  
>  template<typename ValueType, typename HashType>
> -HRESULT STDMETHODCALLTYPE COMPropertyBag<ValueType, HashType>::Read(ULONG cProperties, PROPBAG2*, IErrorLog*, VARIANT* pvarValue, HRESULT* phrError)
> +HRESULT STDMETHODCALLTYPE COMPropertyBag<ValueType, HashType>::Read(ULONG cProperties, PROPBAG2* pPropBag, IErrorLog* pErrorLog, VARIANT* pvarValue, HRESULT* phrError)
>  {
> -    return E_NOTIMPL;
> +    if (!pPropBag || !pvarValue || !phrError)

minor pick: If we don't also check pErrorLog here, isn't it possible that we'll erroneously return E_FAIL below, if the Read() fails due to pErrorLog being null?

> +        return E_POINTER;
> +
> +    HRESULT hr = S_OK;
> +
> +    for (ULONG i = 0; i < cProperties; ++i) {
> +        VariantInit(&pvarValue[i]);
> +        pvarValue[i].vt = pPropBag[i].vt;
> +        phrError[i] = Read(pPropBag[i].pstrName, &pvarValue[i], pErrorLog);
> +        if (FAILED(phrError[i]))
> +            hr = E_FAIL;
> +    }
> +
> +    return hr;
>  }
>  
>  template<typename ValueType, typename HashType>
> @@ -196,10 +209,10 @@ HRESULT STDMETHODCALLTYPE COMPropertyBag
>          ;
>      for (ULONG j = 0; j < cProperties && current != end; ++j, ++current) {
>          // FIXME: the following fields aren't filled in
> -        //pPropBag[j].dwType;   // (DWORD) Type of property. This will be one of the PROPBAG2_TYPE values.
> -        //pPropBag[j].cfType;   // (CLIPFORMAT) Clipboard format or MIME type of the property.
> -        //pPropBag[j].clsid;    // (CLSID) CLSID of the object. This member is valid only if dwType is PROPBAG2_TYPE_OBJECT.
> +        //pPropBag[j].cfType;   // (CLIPFORMAT) Clipboard format or MIME type of the property.
> +        //pPropBag[j].clsid;    // (CLSID) CLSID of the object. This member is valid only if dwType is PROPBAG2_TYPE_OBJECT.
>  
> +        pPropBag[j].dwType = PROPBAG2_TYPE_DATA;
>          pPropBag[j].vt = COMVariantSetter<ValueType>::VariantType;
>          pPropBag[j].dwHint = iProperty + j;
>          pPropBag[j].pstrName = (LPOLESTR)CoTaskMemAlloc(sizeof(wchar_t)*(current->first.length()+1));

[Steve Falkenburg]

pErrorLog is allowed to be 0 according to:
http://msdn.microsoft.com/en-us/library/aa768194(VS.85).aspx

pErrLog
[in] The address of an IErrorlog interface in which the property bag stores any errors that occur during the reads. This argument can be NULL; in which case, the caller receives no logging errors.

[Steve Falkenburg]

Fixed in r39006.