[Webkit-unassigned] [Bug 8516] frame load delegate method webView:willCloseFrame: is called at the wrong times

[bugzilla-daemon at opendarwin.org]

http://bugzilla.opendarwin.org/show_bug.cgi?id=8516


darin at apple.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #8061|review?                     |review-
               Flag|                            |




------- Comment #10 from darin at apple.com  2006-05-02 09:01 PDT -------
(From update of attachment 8061)
This looks great. An excellent set of ideas.

I see a typo: "messaages"

Another thought for the future: if you're going to enhance the test tool like
this you should really use a separate bug. It's hard for us to keep track of
landing multiple distinct patches from the same bug report; we're going to want
this to stay open while we fix the real bug.

In general we try to use nouns for method names that return values. So "dump"
is a method name for something that has a side effect and no result (unless you
mean the "city dump" and I know you don't). So this patch would be even better
if the method was named something more like descriptionForTestResult or
descriptionForDumping or something else that's a noun phrase rather than a
verb. These standards are not really as important in a test tool.

The MethodSwizzle technique unfortunately uses API that's deprecated, but I
guess someone on the Apple side will figure out a way to make it work without
that. It's a little strange to use class_poseAs elsewhere and this technique
here.

+    // only do this once.. install the replacement KVO methods into NSObject

I'd suggest ":" rather than ".." in this message.

For the long term, I'm not sure we're going to be keeping the
WebDefaultXXXDelegate classes and _WebSafeForwarder -- instead we might just
call respondsToSelector in each caller. I suppose that will break this test.
Maybe there's a better technique that will accomplish the same thing.

I believe the -[NSObject dump] method has a bug. If description is empty, then
it will raise an exception because [parts count] will be 0.

The KEY() macro is not a great idea; it assumes that an unsigned int is the
same size as a pointer. It's easy enough to make a CFDictionary for a purpose
like this, or if you must use a hack like this one, -[NSValue
valueWithNonretainedObject:] is a better version.


-- 
Configure bugmail: http://bugzilla.opendarwin.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.