Changeset 161850 in webkit

Timestamp:

Jan 12, 2014 6:27:42 PM (10 years ago)

Author:

Darin Adler

Message:

Add type checking to isEqual methods
​https://bugs.webkit.org/show_bug.cgi?id=126862

Reviewed by Anders Carlsson.

Source/WebCore:

• page/ios/WebEventRegion.mm:

(-[WebEventRegion isEqual:]): Add type checking on the argument.
Add a FIXME about the lack of a hash method override. Formatted to match
the usual WebKit coding style.

Source/WebKit/ios:

• WebCoreSupport/WebVisiblePosition.mm:

(-[WebVisiblePosition isEqual:]): Add type checking on the argument.
Add a FIXME about the lack of a hash method override. Simplified by
removing the unneeded local variables.

Source/WebKit/mac:

• WebCoreSupport/WebSecurityOrigin.mm:

(-[WebSecurityOrigin isEqual:]): Added a FIXME about the lack of a hash method
override. Tweaked formatting.

• WebView/WebDashboardRegion.mm:

(-[WebDashboardRegion isEqual:]): Added type checking on the argument. Added a
FIXME about the lack of a hash method override.

Source/WebKit2:

• UIProcess/API/ios/WKInteractionView.mm:

(-[WKTextRange isEqual:]): Added type checking for the argument. The old
code asserted instead, and it's not clear what guarantees that assertion is
true. Added a FIXME about the lack of a hash method. Added another FIXME
about the fact that this method ignores much of the object state. Removed
an unneeded extra fetch of the isRange property. Deleted some dead code.
(-[WKTextPosition isEqual:]): Ditto.

Location:

trunk/Source

Files:

• WebCore/ChangeLog (modified) (1 diff)
• WebCore/page/ios/WebEventRegion.mm (modified) (1 diff)
• WebKit/ios/ChangeLog (modified) (1 diff)
• WebKit/ios/WebCoreSupport/WebVisiblePosition.mm (modified) (1 diff)
• WebKit/mac/ChangeLog (modified) (1 diff)
• WebKit/mac/WebCoreSupport/WebSecurityOrigin.mm (modified) (1 diff)
• WebKit/mac/WebView/WebDashboardRegion.mm (modified) (1 diff)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/UIProcess/API/ios/WKInteractionView.mm (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2014-01-12  Darin Adler  <darin@apple.com>
+
+        Add type checking to isEqual methods
+        https://bugs.webkit.org/show_bug.cgi?id=126862
+
+        Reviewed by Anders Carlsson.
+
+        * page/ios/WebEventRegion.mm:
+        (-[WebEventRegion isEqual:]): Add type checking on the argument.
+        Add a FIXME about the lack of a hash method override. Formatted to match
+        the usual WebKit coding style.
+
 2014-01-12  Anders Carlsson  <andersca@apple.com>
 

trunk/Source/WebCore/page/ios/WebEventRegion.mm

@@ -67 +67 @@
 }
 
+// FIXME: Overriding isEqual: without overriding hash will cause trouble if this ever goes into an NSSet or is the key in an NSDictionary,
+// since two equal objects could have different hashes.
 - (BOOL)isEqual:(id)other
 {
-    return  CGPointEqualToPoint(p1, ((WebEventRegion *)other)->p1) &&
-            CGPointEqualToPoint(p2, ((WebEventRegion *)other)->p2) &&
-            CGPointEqualToPoint(p3, ((WebEventRegion *)other)->p3) &&
-            CGPointEqualToPoint(p4, ((WebEventRegion *)other)->p4);
+    if (![other isKindOfClass:[WebEventRegion class]])
+        return NO;
+    return CGPointEqualToPoint(p1, ((WebEventRegion *)other)->p1)
+        && CGPointEqualToPoint(p2, ((WebEventRegion *)other)->p2)
+        && CGPointEqualToPoint(p3, ((WebEventRegion *)other)->p3)
+        && CGPointEqualToPoint(p4, ((WebEventRegion *)other)->p4);
 }
 

trunk/Source/WebKit/ios/ChangeLog

@@ +1 @@
+2014-01-12  Darin Adler  <darin@apple.com>
+
+        Add type checking to isEqual methods
+        https://bugs.webkit.org/show_bug.cgi?id=126862
+
+        Reviewed by Anders Carlsson.
+
+        * WebCoreSupport/WebVisiblePosition.mm:
+        (-[WebVisiblePosition isEqual:]): Add type checking on the argument.
+        Add a FIXME about the lack of a hash method override. Simplified by
+        removing the unneeded local variables.
+
 2014-01-02  Andy Estes  <aestes@apple.com>
 

trunk/Source/WebKit/ios/WebCoreSupport/WebVisiblePosition.mm

@@ -88 +88 @@
 }
 
-- (BOOL)isEqual:(WebVisiblePosition *)other
-{
-    VisiblePosition myVP = [self _visiblePosition];
-    VisiblePosition otherVP = [other _visiblePosition];
-    return (myVP == otherVP);
+// FIXME: Overriding isEqual: without overriding hash will cause trouble if this ever goes into an NSSet or is the key in an NSDictionary,
+// since two equal objects could have different hashes.
+- (BOOL)isEqual:(id)other
+{
+    if (![other isKindOfClass:[WebVisiblePosition class]])
+        return NO;
+    return [self _visiblePosition] == [(WebVisiblePosition *)other _visiblePosition];
 }
 

trunk/Source/WebKit/mac/ChangeLog

@@ +1 @@
+2014-01-12  Darin Adler  <darin@apple.com>
+
+        Add type checking to isEqual methods
+        https://bugs.webkit.org/show_bug.cgi?id=126862
+
+        Reviewed by Anders Carlsson.
+
+        * WebCoreSupport/WebSecurityOrigin.mm:
+        (-[WebSecurityOrigin isEqual:]): Added a FIXME about the lack of a hash method
+        override. Tweaked formatting.
+
+        * WebView/WebDashboardRegion.mm:
+        (-[WebDashboardRegion isEqual:]): Added type checking on the argument. Added a
+        FIXME about the lack of a hash method override.
+
 2014-01-11  Alexey Proskuryakov  <ap@apple.com>
 

trunk/Source/WebKit/mac/WebCoreSupport/WebSecurityOrigin.mm

@@ -90 +90 @@
 }
 
+// FIXME: Overriding isEqual: without overriding hash will cause trouble if this ever goes into an NSSet or is the key in an NSDictionary,
+// since two equal objects could have different hashes.
 - (BOOL)isEqual:(id)anObject
 {
-    if (![anObject isMemberOfClass:[WebSecurityOrigin class]]) {
+    if (![anObject isMemberOfClass:[WebSecurityOrigin class]])
         return NO;
-    }
     
     return [self _core]->equal([anObject _core]);

trunk/Source/WebKit/mac/WebView/WebDashboardRegion.mm

@@ -81 +81 @@
 }
 
-// FIXME: Overriding isEqual: without overriding hash will cause trouble if this ever goes into a NSSet or is the key in an NSDictionary.
+// FIXME: Overriding isEqual: without overriding hash will cause trouble if this ever goes into an NSSet or is the key in an NSDictionary,
+// since two equal objects could have different hashes.
 - (BOOL)isEqual:(id)other
 {
+    if (![other isKindOfClass:[WebDashboardRegion class]])
+        return NO;
+
     return NSEqualRects(rect, [other dashboardRegionRect]) && NSEqualRects(clip, [other dashboardRegionClip]) && type == [other dashboardRegionType];
 }

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2014-01-12  Darin Adler  <darin@apple.com>
+
+        Add type checking to isEqual methods
+        https://bugs.webkit.org/show_bug.cgi?id=126862
+
+        Reviewed by Anders Carlsson.
+
+        * UIProcess/API/ios/WKInteractionView.mm:
+        (-[WKTextRange isEqual:]): Added type checking for the argument. The old
+        code asserted instead, and it's not clear what guarantees that assertion is
+        true. Added a FIXME about the lack of a hash method. Added another FIXME
+        about the fact that this method ignores much of the object state. Removed
+        an unneeded extra fetch of the isRange property. Deleted some dead code.
+        (-[WKTextPosition isEqual:]): Ditto.
+
 2014-01-12  Anders Carlsson  <andersca@apple.com>
 

trunk/Source/WebKit2/UIProcess/API/ios/WKInteractionView.mm

@@ -1516 +1516 @@
 }
 
+// FIXME: Overriding isEqual: without overriding hash will cause trouble if this ever goes into an NSSet or is the key in an NSDictionary,
+// since two equal items could have different hashes.
 - (BOOL)isEqual:(id)other
 {
-    assert([other isKindOfClass:[WKTextRange class]]);
+    if (![other isKindOfClass:[WKTextRange class]])
+        return NO;
+
     WKTextRange *otherRange = (WKTextRange *)other;
-    
+
     if (self == other)
         return YES;
-    
+
+    // FIXME: Probably incorrect for equality to ignore so much of the object state.
+    // It ignores isNone, isEditable, selectedTextLength, and selectionRects.
+
     if (self.isRange) {
         if (!otherRange.isRange)
             return NO;
         return CGRectEqualToRect(self.startRect, otherRange.startRect) && CGRectEqualToRect(self.endRect, otherRange.endRect);
-    } else if (!self.isRange) {
+    } else {
         if (otherRange.isRange)
             return NO;
+        // FIXME: Do we need to check isNone here?
         return CGRectEqualToRect(self.startRect, otherRange.startRect);
     }
-    return otherRange.isNone;
 }
 
@@ -1549 +1556 @@
 }
 
+// FIXME: Overriding isEqual: without overriding hash will cause trouble if this ever goes into a NSSet or is the key in an NSDictionary,
+// since two equal items could have different hashes.
 - (BOOL)isEqual:(id)other
 {
-    assert([other isKindOfClass:[self class]]);
+    if (![object isKindOfClass:[WKTextPosition class]])
+        return NO;
+
     return CGRectEqualToRect(self.positionRect, ((WKTextPosition *)other).positionRect);
 }