[webkit-reviews] review denied: [Bug 36723] Crash while uploading a PDF document on www.largefilesasap.com : [Attachment 52118] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Mar 30 19:14:46 PDT 2010


Brady Eidson <beidson at apple.com> has denied TAMURA, Kent <tkent at chromium.org>'s
request for review:
Bug 36723: Crash while uploading a PDF document on www.largefilesasap.com
https://bugs.webkit.org/show_bug.cgi?id=36723

Attachment 52118: Patch
https://bugs.webkit.org/attachment.cgi?id=52118&action=review

------- Additional Comments from Brady Eidson <beidson at apple.com>
Quite honestly, this is my least favorite of the three approaches I suggested.

IMHO, 2 stage construction is not a great pattern unless there's no
alternatives.

I added this comment to the original bug
(https://bugs.webkit.org/show_bug.cgi?id=35072):
---
Since this patch:
1 - Didn't change functionality
2 - The approach was generally not liked by the reviewers.
3 - In general, round tripping between WebCore and WebKit is something we try
to *reduce*

...I think it needs to be rolled out.
---

Can you convince me why round tripping between WebCore and WebKit for 5+
different platforms was better than the previous solution that kept things
entirely in WebCore except for the lone platform that needed the client
interface?

I know Darin expressed disdain for the duplication of the API call in
https://bugs.webkit.org/show_bug.cgi?id=32054 because it would make the code
more complicated and harder to maintain in the long run.  But is the current
state of things actually *better*?

I think it's crazy that WebCore always calls out to the client interfaces for
5+ different platforms, and most of them call straight back into WebCore.  This
makes the code harder to follow and maintain, not easier.


More information about the webkit-reviews mailing list