[PATCH 5/5] qedit: Implement IAMTimelineObj_GetTimelineNoRef and add tests.

[Andrew Eikum]

On Mon, Apr 25, 2016 at 11:46:09PM -0600, Alex Henrie wrote:
> 2016-04-25 23:10 GMT-06:00 Nikolay Sivov <bunglehead at gmail.com>:
> > On 26.04.2016 7:57, Alex Henrie wrote:
> >> 2016-04-25 22:21 GMT-06:00 Nikolay Sivov <bunglehead at gmail.com>:
> >>> On 26.04.2016 6:10, Alex Henrie wrote:
> >>>>  static HRESULT WINAPI TimelineObj_GetTimelineNoRef(IAMTimelineObj *iface, IAMTimeline **timeline)
> >>>>  {
> >>>> +    /* MSDN says that this function is "not supported" */
> >>>>      TimelineObjImpl *This = impl_from_IAMTimelineObj(iface);
> >>>> -    FIXME("(%p)->(%p): not implemented!\n", This, timeline);
> >>>> -    return E_NOTIMPL;
> >>>> +    TRACE("(%p)->(%p)\n", This, timeline);
> >>>> +    if (!timeline) return E_POINTER;
> >>>> +    return E_NOINTERFACE;
> >>>>  }
> >>>
> >>> This looks like a call to QI, according to your tests.
> >>
> >> It looks that way, but which interface would it be querying? The tests
> >> show that an IAMTimelineObj cannot double as an IAMTimeline, nor is
> >> the function supposed to return the IAMTimeline that was used to
> >> create the IAMTimelineObj. My best guess is that Microsoft never
> >> bothered implementing this function properly and so whatever
> >> implementation they have always winds up returning E_NOINTERFACE.
> >
> > Yes, return type is IAMTimeline, so I was suggesting a shortcut doing
> > same thing in one line IAMTimelineObj_QueryInterface(iface,
> > &IID_IAMTimeline, ...). This should cover for setting out pointer to
> > NULL, returning E_NOINTERFACE, and probably handling NULL case too,
> > though it's not a standard QI behavior. None of that is actually
> > important, what you did is fine, take it as a suggestion, or don't.
> 
> OK, I see what you mean now, and I'll make that change in version 2 of
> this patchset.
> 

Eh, I'd rather keep it like it was. It's not clear whether it should
be QIing on this same object or on the parent Timeline object, and the
"NoRef" is weird behavior for a QI.  Implementing it on top of QI
seems to imply confidence we don't have, and would confuse me if I was
reading this code blind.

Andrew