Summary: | Implement layoutTestController.dispatchPendingLoadRequests | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Adam Roben (:aroben) <aroben> | ||||||
Component: | Tools / Tests | Assignee: | Hyungwook Lee <hyungwook.lee> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | beidson, bfulgham, commit-queue, hyungwook.lee | ||||||
Priority: | P2 | Keywords: | InRadar, PlatformOnly | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | PC | ||||||||
OS: | Windows XP | ||||||||
URL: | http://trac.webkit.org/changeset/44519 | ||||||||
Attachments: |
|
Description
Adam Roben (:aroben)
2009-06-17 08:52:44 PDT
Created attachment 255012 [details]
Patch
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. 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". 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. Created attachment 255047 [details]
Patch
Comment on attachment 255047 [details]
Patch
Hyungwook, please check if this patch breaks buildbots after landing.
I will check it. Comment on attachment 255047 [details] Patch Clearing flags on attachment: 255047 Committed r185691: <http://trac.webkit.org/changeset/185691> All reviewed patches have been landed. Closing bug. |