Bug 26481 – Implement layoutTestController.dispatchPendingLoadRequests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Adam Roben (:aroben)]

r44519 added layoutTestController.dispatchPendingLoadRequests. We need to implement this on Windows so we can run the tests that use it.

[Adam Roben (:aroben)]

<rdar://problem/6980493>

[Hyungwook Lee]

Created attachment 255012 [details]
Patch

[Brady Eidson]

Comment on attachment 255012 [details]
Patch

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

> Source/WebKit/win/Interfaces/IWebViewPrivate.idl:300
>      // SPI for DumpRenderTree
>      HRESULT setLoadResourcesSerially([in] BOOL serialize);
>  
> +    HRESULT dispatchPendingLoadRequests();
> +
>      HRESULT scaleWebView([in] double scale, [in] POINT origin);
>  }

I might be wrong about this because I haven't worked on the Windows bindings in forever, but I seem to recall a requirement that new methods be added to the end of an interface as to not break shipping software?

I might be conflating this with something else altogether, but I'm weary of this.

Need a Windows expert to look at this.

[Brent Fulgham]

Comment on attachment 255012 [details]
Patch

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

r- because the ordering in the IDL file needs to be adjusted to avoid breaking shipping software. Otherwise this looks fine.

> Source/WebKit/win/WebView.h:1024
> +    HRESULT STDMETHODCALLTYPE dispatchPendingLoadRequests();

Just to be safe, I'd move this below "scaleWebView" as well.

>> Source/WebKit/win/Interfaces/IWebViewPrivate.idl:300
>>  }
> 
> I might be wrong about this because I haven't worked on the Windows bindings in forever, but I seem to recall a requirement that new methods be added to the end of an interface as to not break shipping software?
> 
> I might be conflating this with something else altogether, but I'm weary of this.
> 
> Need a Windows expert to look at this.

You're right. We want the link offsets to be consistent across builds, so we need to move this new method below "scaleWebView".

[Hyungwook Lee]

Comment on attachment 255012 [details]
Patch

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

>>> Source/WebKit/win/Interfaces/IWebViewPrivate.idl:300
>>>  }
>> 
>> I might be wrong about this because I haven't worked on the Windows bindings in forever, but I seem to recall a requirement that new methods be added to the end of an interface as to not break shipping software?
>> 
>> I might be conflating this with something else altogether, but I'm weary of this.
>> 
>> Need a Windows expert to look at this.
> 
> You're right. We want the link offsets to be consistent across builds, so we need to move this new method below "scaleWebView".

Thanks for your review.
I will update it as your guide.

[Hyungwook Lee]

Created attachment 255047 [details]
Patch

[Gyuyoung Kim]

Comment on attachment 255047 [details]
Patch

Hyungwook, please check if this patch breaks buildbots after landing.

[Hyungwook Lee]

I will check it.

[WebKit Commit Bot]

Comment on attachment 255047 [details]
Patch

Clearing flags on attachment: 255047

Committed r185691: <http://trac.webkit.org/changeset/185691>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.