RESOLVED FIXED 24355
Refactor InspectorController to use IDL
https://bugs.webkit.org/show_bug.cgi?id=24355
Summary Refactor InspectorController to use IDL
Dimitri Glazkov (Google)
Reported 2009-03-04 08:33:55 PST
Creating an IDL for the InspectorController and utilizing generated bindings would greatly improve readability and reduce jamalayainess of InspectorController.cpp. I think I am going to take a stab at this.
Attachments
first stab at idl (4.49 KB, patch)
2009-03-04 16:35 PST, Dimitri Glazkov (Google)
no flags
first pass at writing custom bindings. (98.54 KB, patch)
2009-03-04 16:36 PST, Dimitri Glazkov (Google)
no flags
Add InspectorController.idl, custom bindings, v1. (70.10 KB, patch)
2009-03-05 14:32 PST, Dimitri Glazkov (Google)
timothy: review+
Dimitri Glazkov (Google)
Comment 1 2009-03-04 08:34:51 PST
I mean to say "Jambalayainess", not that other made-up word.
Dimitri Glazkov (Google)
Comment 2 2009-03-04 16:35:13 PST
Created attachment 28293 [details] first stab at idl WebCore/inspector/InspectorController.idl | 90 +++++++++++++++++++++++++++++ 1 files changed, 90 insertions(+), 0 deletions(-)
Dimitri Glazkov (Google)
Comment 3 2009-03-04 16:36:42 PST
Created attachment 28294 [details] first pass at writing custom bindings. WebCore/DerivedSources.make | 1 + WebCore/WebCore.xcodeproj/project.pbxproj | 46 + .../bindings/js/JSInspectorControllerCustom.cpp | 336 +++++++ WebCore/inspector/ConsoleMessage.cpp | 117 +++ WebCore/inspector/ConsoleMessage.h | 70 ++ WebCore/inspector/InspectorController.cpp | 1024 +------------------- WebCore/inspector/InspectorController.h | 16 +- WebCore/inspector/InspectorController.idl | 91 ++ WebCore/inspector/InspectorDOMStorageResource.cpp | 69 ++ WebCore/inspector/InspectorDOMStorageResource.h | 72 ++ WebCore/inspector/InspectorDatabaseResource.cpp | 67 ++ WebCore/inspector/InspectorDatabaseResource.h | 71 ++ WebCore/inspector/InspectorResource.cpp | 178 ++++ WebCore/inspector/InspectorResource.h | 110 +++ WebCore/page/Page.h | 2 +- 15 files changed, 1255 insertions(+), 1015 deletions(-)
Dimitri Glazkov (Google)
Comment 4 2009-03-04 16:41:14 PST
Timothy, here's the first pass. This already works (Mac-only, sorry), but still needs a lot of cleaning up and straightening-out. Oh, and testing. Let me know what you think. Any comments are appreciated. It may kinda-sorta look like I rewrote the world, but it's only because I couldn't stop. Just look how much prettier the bindings look compared to the old code, and I am sure you'll feel the same way :)
Dimitri Glazkov (Google)
Comment 5 2009-03-04 16:42:35 PST
Comment on attachment 28294 [details] first pass at writing custom bindings. As I mentioned, not requesting a formal review, just comments/feedback.
Timothy Hatcher
Comment 6 2009-03-04 17:03:51 PST
I think this looks great so far! Some clean up and I think it is ready. I would be good to split out the classes into files as a first pass.
Dimitri Glazkov (Google)
Comment 7 2009-03-04 17:17:54 PST
Thanks, I'll split this up into smaller patches, launder, then submit for review.
Dimitri Glazkov (Google)
Comment 8 2009-03-05 14:32:17 PST
Created attachment 28326 [details] Add InspectorController.idl, custom bindings, v1. WebCore/ChangeLog | 32 + WebCore/DerivedSources.make | 1 + WebCore/GNUmakefile.am | 2 + WebCore/WebCore.pro | 2 + WebCore/WebCore.vcproj/WebCore.vcproj | 12 + WebCore/WebCore.xcodeproj/project.pbxproj | 78 ++- WebCore/WebCoreSources.bkl | 2 + .../bindings/js/JSInspectorControllerCustom.cpp | 333 ++++++++++ WebCore/bindings/scripts/CodeGeneratorJS.pm | 2 +- WebCore/inspector/InspectorController.cpp | 681 +------------------- WebCore/inspector/InspectorController.h | 16 +- WebCore/inspector/InspectorController.idl | 91 +++ WebCore/page/Page.h | 2 +- 13 files changed, 544 insertions(+), 710 deletions(-)
Timothy Hatcher
Comment 9 2009-03-05 14:43:57 PST
Comment on attachment 28326 [details] Add InspectorController.idl, custom bindings, v1. > History \ > ImageData \ > + InspectorController \ > KeyboardEvent \ > Location \ That should be spaces instead of a tab to match the other items in the list.
Dimitri Glazkov (Google)
Comment 10 2009-03-05 14:59:40 PST
Note You need to log in before you can comment on or make changes to this bug.