This patch exposes the classes and their callbacks to JSC through Image.webkitGetImage. The API is functionally incomplete, so it only will ever call the error callback or throw exceptions.
Created attachment 59151 [details] Patch 2: JSC bindings
Created attachment 59155 [details] Patch 2: JSC bindings
Created attachment 59163 [details] Patch 2: JSC bindings
Created attachment 59165 [details] Patch 2: JSC bindings
Comment on attachment 59165 [details] Patch 2: JSC bindings I'm pretty sure you can create these bindings without any extra code. If you have trouble, you should ask dumi. WebCore/ChangeLog:59 + 2010-06-18 Sterling Swigart <sswigart@google.com> You have two ChangeLogs in this patch. WebCore/bindings/js/JSCustomImageResizerErrorCallback.cpp:47 + JSCustomImageResizerErrorCallback::JSCustomImageResizerErrorCallback(JSObject* callback, JSDOMGlobalObject* globalObject) We should be able to auto-generate these callbacks now. There are some examples in the database bindings. WebCore/html/HTMLImageElement.idl:56 + [Custom] void webkitGetImage(in DOMString mimeType, in ImageResizerSuccessCallback successCallback, [Optional] in ImageResizerErrorCallback errorCallback, [Optional] in ResizeOptions options) This shouldn't be custom.
Created attachment 59321 [details] Patch 2: JSC bindings
Comment on attachment 59321 [details] Patch 2: JSC bindings some drive-by comments: WebCore/bindings/scripts/CodeGeneratorJS.pm:2182 + push(@implContent, " args.append("); minor: it might be nicer to keep the code shorter by moving this line and the one after the if-block, inside the if-else block: if (...) push(@implContent, " args.append(jsString(exec, ${paramName}));\n"); else ... or maybe even add a separate function for this? getToJSString()? i'll let adam decide what's best. :) WebCore/bindings/scripts/CodeGeneratorJS.pm:2185 + } else { indentation is off by 1 space. do we need to change the V8 code generator too? or toV8() works correctly on Strings?
I'm concerned that the spec for this API hasn't received enough review. For example, it's unclear to to implement resizeOptions in other languages, such as Objective-C. If we want other browsers to adopt this API, we should make sure it follows the regular conventions of other DOM APIs, such as being language neutral.
(In reply to comment #5) > (From update of attachment 59165 [details]) > I'm pretty sure you can create these bindings without any extra code. If you have trouble, you should ask dumi. > Done. > WebCore/ChangeLog:59 > + 2010-06-18 Sterling Swigart <sswigart@google.com> > You have two ChangeLogs in this patch. > Fixed. > WebCore/bindings/js/JSCustomImageResizerErrorCallback.cpp:47 > + JSCustomImageResizerErrorCallback::JSCustomImageResizerErrorCallback(JSObject* callback, JSDOMGlobalObject* globalObject) > We should be able to auto-generate these callbacks now. There are some examples in the database bindings. > Done. > WebCore/html/HTMLImageElement.idl:56 > + [Custom] void webkitGetImage(in DOMString mimeType, in ImageResizerSuccessCallback successCallback, [Optional] in ImageResizerErrorCallback errorCallback, [Optional] in ResizeOptions options) > This shouldn't be custom. I asked WPL about this, but I am still looking for further feedback on the current state of this patch. @Dumitru: - I consolidated the new CodeGeneratorJS.pm code. - This patch is only for JSC bindings--when the V8 bindings are added, CodeGeneratorV8.pm will be changed to match.
Created attachment 59383 [details] Patch 2: JSC bindings
Created attachment 59387 [details] Patch 2: JSC bindings
Created attachment 59415 [details] Patch 2: JSC bindings
Created attachment 59451 [details] Patch 2: JSC bindings
Created attachment 59463 [details] Patch 2: JSC bindings
Created attachment 59465 [details] Patch 2: JSC bindings
Attachment 59465 [details] did not pass style-queue: Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1 WebCore/bindings/js/JSHTMLImageElementCustom.cpp:92: Tab found; better to use spaces [whitespace/tab] [1] WebCore/bindings/js/JSHTMLImageElementCustom.cpp:93: Tab found; better to use spaces [whitespace/tab] [1] WebCore/bindings/js/JSHTMLImageElementCustom.cpp:94: Tab found; better to use spaces [whitespace/tab] [1] WebCore/bindings/js/JSHTMLImageElementCustom.cpp:101: Tab found; better to use spaces [whitespace/tab] [1] WebCore/bindings/js/JSHTMLImageElementCustom.cpp:102: Tab found; better to use spaces [whitespace/tab] [1] WebCore/bindings/js/JSHTMLImageElementCustom.cpp:103: Tab found; better to use spaces [whitespace/tab] [1] Total errors found: 6 in 45 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 59467 [details] Patch 2: JSC bindings
Attachment 59467 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3281627
Created attachment 59474 [details] Patch 2: JSC bindings
Created attachment 59476 [details] Patch 2: JSC bindings
Created attachment 59478 [details] Patch 2: JSC bindings
Attachment 59478 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3337572
Created attachment 59527 [details] Patch 2: JSC bindings
Attachment 59527 [details] did not build on qt: Build output: http://webkit-commit-queue.appspot.com/results/3309603
Created attachment 59530 [details] Patch 2: JSC bindings
Attachment 59527 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3291659
Attachment 59530 [details] did not build on chromium: Build output: http://webkit-commit-queue.appspot.com/results/3273615
Just wanted to point out that the cr-linux bot (incorrectly) skips at least one step when building WebKit, so it goes red for any patch with IDL changes. If you're concerned about that bot, I'd recommend just patching the changes into a clean Chromium client and building it locally.
(In reply to comment #28) > Just wanted to point out that the cr-linux bot (incorrectly) skips at least one step when building WebKit, so it goes red for any patch with IDL changes. If you're concerned about that bot, I'd recommend just patching the changes into a clean Chromium client and building it locally. What step is the bot supposed to run? It just calls "build-webkit --chromium"
(In reply to comment #29) > (In reply to comment #28) > > Just wanted to point out that the cr-linux bot (incorrectly) skips at least one step when building WebKit, so it goes red for any patch with IDL changes. If you're concerned about that bot, I'd recommend just patching the changes into a clean Chromium client and building it locally. > > What step is the bot supposed to run? It just calls "build-webkit --chromium" In my case, I was developing this API under the ENABLE_IMAGE_RESIZER flag, but someone flipped it from 0 to 1 without me knowing. It shouldn't be building the stuff in the IDLs at all--which is being fixed with 41116.
> What step is the bot supposed to run? It just calls "build-webkit --chromium" It looks like it misses the "gclient sync" step (WebKitTools/Scripts/update-webkit-chromium). So whenever new files are added, the Makefile is not regenerated, which leads to build errors.