Skip to content

GitLab

T
tangled
Commit 05bc8d81 authored by Hiroshi Ichikawa's avatar Hiroshi Ichikawa Committed by Commit Bot
Browse files
Options

Simplify CRWWebViewScrollViewProxy implementation. 

Simplify based on the fact that we can now assume that
self.underlyingScrollView is never nil.

Especially, simplify the properties preservation by deleting
CRWPropertiesStore and simply assigning them from the old scroll view to the
new scroll view.

Bug: 1023250
Change-Id: I29e97ff154a92b1ec0ad9ea91d27bc7a393f4ecf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2132892
Auto-Submit: Hiroshi Ichikawa <ichikawa@chromium.org>
Reviewed-by: default avatarGauthier Ambard <gambard@chromium.org>
Commit-Queue: Hiroshi Ichikawa <ichikawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#756560}
parent c4c43642
Hide whitespace changes
Inline Side-by-side
Showing with 68 additions and 921 deletions
ios/web/BUILD.gn
View file @ 05bc8d81
...@@ -485,7 +485,6 @@ source_set("ios_web_web_state_ui_unittests") { ...@@ -485,7 +485,6 @@ source_set("ios_web_web_state_ui_unittests") {
] ]
sources = [ sources = [
"web_state/ui/crw_properties_store_unittest.mm",
"web_state/ui/crw_web_controller_unittest.mm", "web_state/ui/crw_web_controller_unittest.mm",
"web_state/ui/crw_web_view_content_view_unittest.mm", "web_state/ui/crw_web_view_content_view_unittest.mm",
"web_state/ui/crw_web_view_proxy_impl_unittest.mm", "web_state/ui/crw_web_view_proxy_impl_unittest.mm",
......
ios/web/web_state/ui/BUILD.gn
View file @ 05bc8d81
...@@ -47,8 +47,6 @@ source_set("ui") { ...@@ -47,8 +47,6 @@ source_set("ui") {
] ]
sources = [ sources = [
"crw_properties_store.h",
"crw_properties_store.mm",
"crw_swipe_recognizer_provider.h", "crw_swipe_recognizer_provider.h",
"crw_touch_tracking_recognizer.h", "crw_touch_tracking_recognizer.h",
"crw_touch_tracking_recognizer.mm", "crw_touch_tracking_recognizer.mm",
......
ios/web/web_state/ui/crw_properties_store.h deleted 100644 → 0
View file @ c4c43642
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef IOS_WEB_WEB_STATE_UI_CRW_PROPERTIES_STORE_H_
#define IOS_WEB_WEB_STATE_UI_CRW_PROPERTIES_STORE_H_
#import <Foundation/Foundation.h>
// An attribute of a property with an Objective-C object type to be preserved.
typedef NS_ENUM(NSInteger, CRWStoredPropertyAttribute) {
// "strong" attribute.
CRWStoredPropertyAttributeStrong,
// "weak" attribute.
CRWStoredPropertyAttributeWeak,
// "copy" attribute.
CRWStoredPropertyAttributeCopy,
};
// An object which preserves properties of an underlying object when the
// underlying object is reassigned.
//
// This is useful when:
// - A class is a proxy for an underlying object.
// - The underlying object can be nil or reassigned during the lifetime of the
// wrapper.
// - The proxy should preserve a subset of the properties of the underlying
// object when the underlying object is reassigned.
//
// A caller must call its "register" methods to register properties to be
// preserved before using it.
//
// TODO(crbug.com/1023250): Add a unit test for this class.
// TODO(crbug.com/1023250): Add more safety checks for this class.
@interface CRWPropertiesStore : NSObject
// Registers a property with an Objective-C object type to be preserved.
// |getter| and |setter| are selectors of the getter/setter of the underlying
// object.
- (void)registerObjectPropertyWithGetter:(nonnull SEL)getter
setter:(nonnull SEL)setter
attribute:(CRWStoredPropertyAttribute)attribute;
// Registers a property with a type not an Objective-C object to be preserved.
// |getter| and |setter| are selectors of the getter/setter of the underlying
// object. |type| is the type encoding of the property type e.g., @encode(BOOL).
//
// This should be used e.g., for scalar types (NSInteger, CGFloat, etc.) and C
// structures (CGRect, CGPoint, etc.).
- (void)registerNonObjectPropertyWithGetter:(nonnull SEL)getter
setter:(nonnull SEL)setter
type:(nonnull const char*)type;
// Saves the properties of |object| to the properties store. Should be called
// against the old underlying object when the underlying object is reassigned.
- (void)savePropertiesFromObject:(nonnull id)object;
// Loads the properties from the properties store to |object|. Should be called
// against the new underlying object when the underlying object is reassigned.
- (void)loadPropertiesToObject:(nonnull id)object;
// Clears values of all the properties in the properties store. This prevents
// from retaining property values no longer needed. It does not reset
// registration of properties.
- (void)clearValues;
// Forwards |invocation| to the properties store.
//
// If |invocation| is an invocation of a getter or setter of a registered
// property, it gets or sets the property in the property store, which is later
// restored in the underlying object, and returns YES. Otherwise does nothing
// and returns NO.
//
// This should be called in -forwardInvocation: of the wrapper when the
// underlying object is nil.
- (BOOL)forwardInvocationToPropertiesStore:(nonnull NSInvocation*)invocation;
@end
#endif // IOS_WEB_WEB_STATE_UI_CRW_PROPERTIES_STORE_H_
ios/web/web_state/ui/crw_properties_store.mm deleted 100644 → 0
View file @ c4c43642
// Copyright 2019 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#import "ios/web/web_state/ui/crw_properties_store.h"
#include "base/logging.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
// An abstract base class of storage of a single property in CRWPropertiesStore.
@interface CRWPropertyStore : NSObject
- (instancetype)initWithGetter:(SEL)getter
setter:(SEL)setter NS_DESIGNATED_INITIALIZER;
- (instancetype)init NS_UNAVAILABLE;
// The selector of the getter of the property.
@property(nonatomic, readonly) SEL getter;
// The selector of the setter of the property.
@property(nonatomic, readonly) SEL setter;
// The type encoding of the property type e.g., @encode(BOOL).
@property(nonatomic, readonly) const char* type;
// A pointer to the value with the byte size |size|.
@property(nonatomic, readonly) const void* valueBytes;
// The byte size of the value type.
@property(nonatomic, readonly) NSUInteger size;
// YES if the store contains a value.
@property(nonatomic) BOOL hasValue;
// Sets the value from |bytes|. |bytes| must be a buffer with the byte size
// |size|. This method copies the content of |bytes|. |bytes| must
// not be under control of ARC.
- (void)setValueFromBytes:(const void*)bytes;
// Clears the value. |hasValue| becomes NO after this call.
- (void)clearValue;
@end
// Storage of a single property with "strong" attribute in CRWPropertiesStore.
@interface CRWStrongPropertyStore : CRWPropertyStore
@end
// Storage of a single property with "weak" attribute in CRWPropertiesStore.
@interface CRWWeakPropertyStore : CRWPropertyStore
@end
// Storage of a single property with "copy" attribute in CRWPropertiesStore.
@interface CRWCopyPropertyStore : CRWPropertyStore
@end
// Storage of a single property with an non object type in CRWPropertiesStore.
@interface CRWNonObjectPropertyStore : CRWPropertyStore
- (instancetype)initWithGetter:(SEL)getter
setter:(SEL)setter
type:(const char*)type NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithGetter:(SEL)getter setter:(SEL)setter NS_UNAVAILABLE;
@end
@implementation CRWPropertyStore
@dynamic valueBytes;
@dynamic type;
@dynamic size;
- (instancetype)initWithGetter:(SEL)getter setter:(SEL)setter {
self = [super init];
if (self) {
_getter = getter;
_setter = setter;
_hasValue = NO;
}
return self;
}
- (void)setValueFromBytes:(const void*)bytes {
NOTIMPLEMENTED() << "A subclass must implement this.";
}
- (void)clearValue {
NOTIMPLEMENTED() << "A subclass must implement this.";
}
@end
@implementation CRWStrongPropertyStore {
id _value;
}
- (const void*)valueBytes {
return &_value;
}
- (const char*)type {
return @encode(id);
}
- (NSUInteger)size {
return sizeof(id);
}
- (void)setValueFromBytes:(const void*)bytes {
self.hasValue = YES;
// |bytes| is not under control of ARC, so it should be treated as
// __unsafe_unretained. ARC automatically retains the object by this
// assignment to a strong variable |_value|, so manual retention is not
// necessary.
_value = *reinterpret_cast<__unsafe_unretained const id*>(bytes);
}
- (void)clearValue {
self.hasValue = NO;
_value = nil;
}
@end
@implementation CRWCopyPropertyStore {
id _value;
}
- (const void*)valueBytes {
return &_value;
}
- (const char*)type {
return @encode(id);
}
- (NSUInteger)size {
return sizeof(id);
}
- (void)setValueFromBytes:(const void*)bytes {
self.hasValue = YES;
// |bytes| is not under control of ARC, so it should be treated as
// __unsafe_unretained.
_value =
[*reinterpret_cast<__unsafe_unretained NSObject* const*>(bytes) copy];
}
- (void)clearValue {
self.hasValue = NO;
_value = nil;
}
@end
@implementation CRWWeakPropertyStore {
__weak id _value;
}
- (const void*)valueBytes {
return &_value;
}
- (const char*)type {
return @encode(id);
}
- (NSUInteger)size {
return sizeof(id);
}
- (void)setValueFromBytes:(const void*)bytes {
self.hasValue = YES;
// |bytes| is not under control of ARC, so it should be treated as
// __unsafe_unretained.
_value = *reinterpret_cast<__unsafe_unretained const id*>(bytes);
}
- (void)clearValue {
self.hasValue = NO;
_value = nil;
}
@end
@implementation CRWNonObjectPropertyStore {
NSMutableData* _valueData;
}
@synthesize type = _type;
@synthesize size = _size;
- (instancetype)initWithGetter:(SEL)getter
setter:(SEL)setter
type:(const char*)type {
self = [super initWithGetter:getter setter:setter];
if (self) {
DCHECK_NE(strcmp(type, @encode(id)), 0)
<< "CRWNonObjectPropertyStore must not be used for object types";
_type = type;
NSGetSizeAndAlignment(type, &_size, nullptr);
_valueData = [NSMutableData dataWithLength:_size];
}
return self;
}
- (const void*)valueBytes {
return _valueData.bytes;
}
- (void)setValueFromBytes:(const void*)bytes {
self.hasValue = YES;
[_valueData replaceBytesInRange:NSMakeRange(0, _valueData.length)
withBytes:bytes];
}
- (void)clearValue {
self.hasValue = NO;
[_valueData resetBytesInRange:NSMakeRange(0, _valueData.length)];
}
@end
@implementation CRWPropertiesStore {
// A list of stores for registered properties.
NSMutableArray<CRWPropertyStore*>* _propertyStores;
// A dictionary where:
// - The key is NSValue for SEL of the getter of the property.
// - The value is a store of the property.
NSMutableDictionary<NSValue*, CRWPropertyStore*>* _propertyStoreByGetter;
// A dictionary where:
// - The key is NSValue for SEL of the setter of the property.
// - The value is a store of the property.
NSMutableDictionary<NSValue*, CRWPropertyStore*>* _propertyStoreBySetter;
}
- (instancetype)init {
self = [super init];
if (self) {
_propertyStores = [[NSMutableArray alloc] init];
_propertyStoreByGetter = [[NSMutableDictionary alloc] init];
_propertyStoreBySetter = [[NSMutableDictionary alloc] init];
}
return self;
}
- (void)registerObjectPropertyWithGetter:(SEL)getter
setter:(SEL)setter
attribute:(CRWStoredPropertyAttribute)attribute {
CRWPropertyStore* propertyStore;
switch (attribute) {
case CRWStoredPropertyAttributeStrong:
propertyStore = [[CRWStrongPropertyStore alloc] initWithGetter:getter
setter:setter];
break;
case CRWStoredPropertyAttributeWeak:
propertyStore = [[CRWWeakPropertyStore alloc] initWithGetter:getter
setter:setter];
break;
case CRWStoredPropertyAttributeCopy:
propertyStore = [[CRWCopyPropertyStore alloc] initWithGetter:getter
setter:setter];
break;
}
[self registerPropertyWithStore:propertyStore];
}
- (void)registerNonObjectPropertyWithGetter:(SEL)getter
setter:(SEL)setter
type:(const char*)type {
[self registerPropertyWithStore:[[CRWNonObjectPropertyStore alloc]
initWithGetter:getter
setter:setter
type:type]];
}
- (void)registerPropertyWithStore:(CRWPropertyStore*)store {
[_propertyStores addObject:store];
NSValue* getterKey = [NSValue valueWithPointer:store.getter];
DCHECK(!_propertyStoreByGetter[getterKey])
<< "A property with getter "
<< NSStringFromSelector(store.getter).UTF8String
<< " has already been registered to the property store";
_propertyStoreByGetter[getterKey] = store;
NSValue* setterKey = [NSValue valueWithPointer:store.setter];
DCHECK(!_propertyStoreBySetter[setterKey])
<< "A property with setter "
<< NSStringFromSelector(store.setter).UTF8String
<< " has already been registered to the property store";
_propertyStoreBySetter[setterKey] = store;
}
- (void)savePropertiesFromObject:(id)object {
for (CRWPropertyStore* propertyStore in _propertyStores) {
// Invoke the getter against |object|.
NSMethodSignature* signature =
[object methodSignatureForSelector:propertyStore.getter];
NSInvocation* invocation =
[NSInvocation invocationWithMethodSignature:signature];
invocation.selector = propertyStore.getter;
[invocation invokeWithTarget:object];
// Store the return value in the property store.
NSMutableData* data = [NSMutableData dataWithLength:propertyStore.size];
[invocation getReturnValue:data.mutableBytes];
[propertyStore setValueFromBytes:data.bytes];
}
}
- (void)loadPropertiesToObject:(id)object {
for (CRWPropertyStore* propertyStore in _propertyStores) {
if (!propertyStore.hasValue) {
continue;
}
// Invoke the setter against |object| with the first argument set to the
// value in the property store.
NSMethodSignature* signature =
[object methodSignatureForSelector:propertyStore.setter];
NSInvocation* invocation =
[NSInvocation invocationWithMethodSignature:signature];
invocation.selector = propertyStore.setter;
// const_cast is necessary because the first parameter of
// -setArgument:atIndex: is somehow typed void* instead of const void*. But
// it shouldn't modify its content in reality. The first argument is at
// index 2 because the index 0 and 1 are for self and _cmd.
[invocation setArgument:const_cast<void*>(propertyStore.valueBytes)
atIndex:2];
[invocation invokeWithTarget:object];
}
}
- (void)clearValues {
for (CRWPropertyStore* propertyStore in _propertyStores) {
[propertyStore clearValue];
}
}
- (BOOL)forwardInvocationToPropertiesStore:(NSInvocation*)invocation {
CRWPropertyStore* getterPropertyStore =
_propertyStoreByGetter[[NSValue valueWithPointer:invocation.selector]];
if (getterPropertyStore) {
// This is the getter of a registered property. Return the stored value.
return [self returnValueInPropertyStore:getterPropertyStore
forInvocation:invocation];
}
CRWPropertyStore* setterPropertyStore =
_propertyStoreBySetter[[NSValue valueWithPointer:invocation.selector]];
if (setterPropertyStore) {
// This is the setter of a registered property. Retrieve the assigned value
// and store it.
return [self storeArgumentOfInvocation:invocation
toPropertyStore:setterPropertyStore];
}
return NO;
}
// Returns a value in |propertyStore| as a return value of |invocation|. Returns
// YES on success.
- (BOOL)returnValueInPropertyStore:(CRWPropertyStore*)propertyStore
forInvocation:(NSInvocation*)invocation {
const char* returnType = invocation.methodSignature.methodReturnType;
if (strcmp(returnType, propertyStore.type) != 0) {
// It has NOTREACHED() followed by returning NO as an exception to the style
// guide. This is because it should never reach here in the current version
// of iOS, but it might reach here in a *future* version of iOS for the same
// build, even though it should be quite rare. This happens if this class is
// used for properties of an iOS standard class and Apple changes the type
// of an existing property. When it happens, this method gives up handling
// the specific property and continues execution by returning NO, rather
// than causing buffer overflow.
NOTREACHED()
<< "The return value type of "
<< NSStringFromSelector(invocation.selector).UTF8String << " ("
<< returnType
<< ") does not match the type registered to CRWPropertiesStore ("
<< propertyStore.type << ")";
return NO;
}
// const_cast is necessary because the first parameter of -setReturnVaule:
// is somehow typed void* instead of const void*. But it shouldn't modify
// its content in reality.
[invocation setReturnValue:const_cast<void*>(propertyStore.valueBytes)];
return YES;
}
// Stores the only argument of |invocation| to |propertyStore|. Returns YES on
// success.
- (BOOL)storeArgumentOfInvocation:(NSInvocation*)invocation
toPropertyStore:(CRWPropertyStore*)propertyStore {
NSUInteger numArguments = invocation.methodSignature.numberOfArguments;
DCHECK_EQ(numArguments, 3UL)
<< NSStringFromSelector(invocation.selector).UTF8String
<< " must take exactly 3 arguments (including self and _cmd) "
"but it takes "
<< numArguments;
// The argument is at index 2 because the index 0 and 1 are for self and _cmd.
const NSUInteger kArgumentIndex = 2;
const char* argumentType =
[invocation.methodSignature getArgumentTypeAtIndex:kArgumentIndex];
if (strcmp(argumentType, propertyStore.type) != 0) {
// It has NOTREACHED() followed by returning NO as an exception to the style
// guide. This is because it should never reach here in the current version
// of iOS, but it might reach here in a *future* version of iOS for the same
// build, even though it should be quite rare. This happens if this class is
// used for properties of an iOS standard class and Apple changes the type
// of an existing property. When it happens, this method gives up handling
// the specific property and continues execution by returning NO, rather
// than causing buffer overflow.
NOTREACHED()
<< "The argument type of "
<< NSStringFromSelector(invocation.selector).UTF8String << " ("
<< argumentType
<< ") does not match the type registered to CRWPropertiesStore ("
<< propertyStore.type << ")";
return NO;
}
NSMutableData* data = [NSMutableData dataWithLength:propertyStore.size];
[invocation getArgument:data.mutableBytes atIndex:kArgumentIndex];
[propertyStore setValueFromBytes:data.bytes];
return YES;
}
@end
ios/web/web_state/ui/crw_properties_store_unittest.mm deleted 100644 → 0
View file @ c4c43642
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#import "ios/web/web_state/ui/crw_properties_store.h"
#import <Foundation/Foundation.h>
#include "base/compiler_specific.h"
#include "testing/gtest/include/gtest/gtest.h"
#import "testing/gtest_mac.h"
#include "testing/platform_test.h"
#if !defined(__has_feature) || !__has_feature(objc_arc)
#error "This file requires ARC support."
#endif
// A protocol with properties used for the test.
@protocol CRWTestProtocol <NSObject>
@property(nonatomic) NSInteger nonObjectProperty;
@property(nonatomic, strong) NSObject* strongProperty;
@property(nonatomic, weak) NSObject* weakProperty;
@property(nonatomic, copy) NSObject* copiedProperty;
@end
// An object with properties used for the test.
@interface CRWTestObject : NSObject <CRWTestProtocol>
@end
@implementation CRWTestObject
@synthesize nonObjectProperty = _nonObjectProperty;
@synthesize strongProperty = _strongProperty;
@synthesize weakProperty = _weakProperty;
@synthesize copiedProperty = _copiedProperty;
@end
// An object which forwards all invocations to a CRWPropertiesStore.
@interface CRWTestProxy : NSObject
@property(nonatomic) CRWPropertiesStore* propertiesStore;
@end
// Conform to the protocol in a category, which allows to conform to a protocol
// without implementing the properties explicitly.
@interface CRWTestProxy (CRWTestProtocol) <CRWTestProtocol>
@end
@implementation CRWTestProxy
- (NSMethodSignature*)methodSignatureForSelector:(SEL)sel {
return [CRWTestObject instanceMethodSignatureForSelector:sel];
}
- (void)forwardInvocation:(NSInvocation*)invocation {
[_propertiesStore forwardInvocationToPropertiesStore:invocation];
}
@end
class CRWPropertiesStoreTest : public PlatformTest {
public:
CRWPropertiesStoreTest()
: properties_store_([[CRWPropertiesStore alloc] init]),
proxy_([[CRWTestProxy alloc] init]) {
[properties_store_
registerNonObjectPropertyWithGetter:@selector(nonObjectProperty)
setter:@selector(setNonObjectProperty:)
type:@encode(NSInteger)];
[properties_store_
registerObjectPropertyWithGetter:@selector(strongProperty)
setter:@selector(setStrongProperty:)
attribute:CRWStoredPropertyAttributeStrong];
[properties_store_
registerObjectPropertyWithGetter:@selector(weakProperty)
setter:@selector(setWeakProperty:)
attribute:CRWStoredPropertyAttributeWeak];
[properties_store_
registerObjectPropertyWithGetter:@selector(copiedProperty)
setter:@selector(setCopiedProperty:)
attribute:CRWStoredPropertyAttributeCopy];
proxy_.propertiesStore = properties_store_;
}
protected:
CRWPropertiesStore* properties_store_;
CRWTestProxy* proxy_;
};
// Tests that a value of an non-object type property is preserved when accessed
// via -forwardInvocationToPropertiesStore:.
TEST_F(CRWPropertiesStoreTest, PreserveForwardedNonObjectProperty) {
EXPECT_EQ(0, proxy_.nonObjectProperty);
proxy_.nonObjectProperty = 1;
EXPECT_EQ(1, proxy_.nonObjectProperty);
}
// Tests that a value of a strong property is preserved when accessed via
// -forwardInvocationToPropertiesStore:.
TEST_F(CRWPropertiesStoreTest, PreserveForwardedStrongProperty) {
NSObject* strong_value = [[NSObject alloc] init];
__weak NSObject* weak_value = strong_value;
EXPECT_FALSE(proxy_.strongProperty);
proxy_.strongProperty = strong_value;
EXPECT_EQ(strong_value, proxy_.strongProperty);
// Verify that the object is retained by CRWPropertiesStore.
strong_value = nil;
EXPECT_EQ(weak_value, proxy_.strongProperty);
}
// Tests that a value of a weak property is preserved when accessed via
// -forwardInvocationToPropertiesStore:.
TEST_F(CRWPropertiesStoreTest, PreserveForwardedWeakProperty) {
NSObject* strong_value = [[NSObject alloc] init];
EXPECT_FALSE(proxy_.weakProperty);
proxy_.weakProperty = strong_value;
EXPECT_EQ(strong_value, proxy_.weakProperty);
// Verify that the object is NOT retained by CRWPropertiesStore.
strong_value = nil;
EXPECT_FALSE(proxy_.weakProperty);
}
// Tests that a value of a copy property is preserved when accessed via
// -forwardInvocationToPropertiesStore:.
TEST_F(CRWPropertiesStoreTest, PreserveForwardedCopiedProperty) {
// Use an NSMutableString as an example object here so that its -copy returns
// a different object from itself. -[NSString copy] may return itself.
NSMutableString* string = [@"hoge" mutableCopy];
EXPECT_FALSE(proxy_.copiedProperty);
proxy_.copiedProperty = string;
// Verify that it is copied i.e., it -isEqual: to the original object but not
// the same instance.
EXPECT_NSEQ(string, proxy_.copiedProperty);
EXPECT_NE(string, proxy_.copiedProperty);
}
// Tests that -savePropertiesFromObject: works as expected.
TEST_F(CRWPropertiesStoreTest, SaveProperties) {
NSObject* object_value1 = [[NSObject alloc] init];
NSObject* object_value2 = [[NSObject alloc] init];
NSMutableString* copyable_value = [@"hoge" mutableCopy];
CRWTestObject* object = [[CRWTestObject alloc] init];
object.nonObjectProperty = 1;
object.strongProperty = object_value1;
object.weakProperty = object_value2;
object.copiedProperty = copyable_value;
[properties_store_ savePropertiesFromObject:object];
EXPECT_EQ(1, proxy_.nonObjectProperty);
EXPECT_EQ(object_value1, proxy_.strongProperty);
EXPECT_EQ(object_value2, proxy_.weakProperty);
EXPECT_NSEQ(copyable_value, proxy_.copiedProperty);
}
// Tests that -loadPropertiesToObject: works as expected.
TEST_F(CRWPropertiesStoreTest, LoadProperties) {
NSObject* object_value1 = [[NSObject alloc] init];
NSObject* object_value2 = [[NSObject alloc] init];
NSMutableString* copyable_value = [@"hoge" mutableCopy];
proxy_.nonObjectProperty = 1;
proxy_.strongProperty = object_value1;
proxy_.weakProperty = object_value2;
proxy_.copiedProperty = copyable_value;
CRWTestObject* object = [[CRWTestObject alloc] init];
[properties_store_ loadPropertiesToObject:object];
EXPECT_EQ(1, object.nonObjectProperty);
EXPECT_EQ(object_value1, object.strongProperty);
EXPECT_EQ(object_value2, object.weakProperty);
EXPECT_NSEQ(copyable_value, object.copiedProperty);
}
// Tests that -clearValues works as expected.
TEST_F(CRWPropertiesStoreTest, ClearValues) {
NSObject* object_value1 = [[NSObject alloc] init];
NSObject* object_value2 = [[NSObject alloc] init];
NSMutableString* copyable_value = [@"hoge" mutableCopy];
proxy_.nonObjectProperty = 1;
proxy_.strongProperty = object_value1;
proxy_.weakProperty = object_value2;
proxy_.copiedProperty = copyable_value;
[properties_store_ clearValues];
EXPECT_EQ(0, proxy_.nonObjectProperty);
EXPECT_FALSE(proxy_.strongProperty);
EXPECT_FALSE(proxy_.weakProperty);
EXPECT_FALSE(proxy_.copiedProperty);
}
ios/web/web_state/ui/crw_web_view_scroll_view_proxy.mm
View file @ 05bc8d81
...@@ -11,7 +11,6 @@ ...@@ -11,7 +11,6 @@
#import "base/ios/crb_protocol_observers.h" #import "base/ios/crb_protocol_observers.h"
#include "base/mac/foundation_util.h" #include "base/mac/foundation_util.h"
#include "ios/web/common/features.h" #include "ios/web/common/features.h"
#import "ios/web/web_state/ui/crw_properties_store.h"
#import "ios/web/web_state/ui/crw_web_view_scroll_view_delegate_proxy.h" #import "ios/web/web_state/ui/crw_web_view_scroll_view_delegate_proxy.h"
#if !defined(__has_feature) || !__has_feature(objc_arc) #if !defined(__has_feature) || !__has_feature(objc_arc)
...@@ -111,132 +110,6 @@ ...@@ -111,132 +110,6 @@
NSMutableDictionary<NSString*, NSMutableDictionary<NSString*,
NSMapTable<id, CRWKeyValueObserverForwarder*>*>* NSMapTable<id, CRWKeyValueObserverForwarder*>*>*
_keyValueObserverForwarders; _keyValueObserverForwarders;
// A storage used to preserve values of the scroll view properties on
// resetting the underlying scroll view.
CRWPropertiesStore* _propertiesStore;
}
// Returns a new instance of CRWPropertiesStore used in this class.
+ (CRWPropertiesStore*)propertiesStore {
CRWPropertiesStore* store = [[CRWPropertiesStore alloc] init];
// A list of properties preserved on resetting the underlying scroll view.
//
// The underlying scroll view can be nil or can be reassigned. Properties
// of the underlying scroll view are usually not preserved when the scroll
// view is reassigned. Properties listed here will be preserved i.e.:
// - If the property is assigned while the underlying scroll view is nil,
// the assignment is applied when the underlying scroll view is
// assigned.
// - The property is preserved when the underlying scroll view is
// reassigned.
//
// This list should contain all properties of UIScrollView and its
// ancestor classes (not limited to properties explicitly declared in
// CRWWebViewScrollViewProxy) which:
// - is a readwrite property
// - AND is supposed to be modified directly, considering it's a scroll
// view of a web view. e.g., |frame| and |subviews| do not meet this
// condition because they are managed by the web view.
//
// Properties not explicitly declared in CRWWebViewScrollViewProxy can still
// be accessed via -asUIScrollView, so they should be preserved as well.
// UIScrollView properties.
[store registerNonObjectPropertyWithGetter:@selector(isScrollEnabled)
setter:@selector(setScrollEnabled:)
type:@encode(BOOL)];
[store
registerNonObjectPropertyWithGetter:@selector(isDirectionalLockEnabled)
setter:@selector(setDirectionalLockEnabled:)
type:@encode(BOOL)];
[store registerNonObjectPropertyWithGetter:@selector(isPagingEnabled)
setter:@selector(setPagingEnabled:)
type:@encode(BOOL)];
[store registerNonObjectPropertyWithGetter:@selector(scrollsToTop)
setter:@selector(setScrollsToTop:)
type:@encode(BOOL)];
[store registerNonObjectPropertyWithGetter:@selector(bounces)
setter:@selector(setBounces:)
type:@encode(BOOL)];
[store registerNonObjectPropertyWithGetter:@selector(alwaysBounceVertical)
setter:@selector(setAlwaysBounceVertical:)
type:@encode(BOOL)];
[store
registerNonObjectPropertyWithGetter:@selector(alwaysBounceHorizontal)
setter:@selector(setAlwaysBounceHorizontal:)
type:@encode(BOOL)];
[store registerNonObjectPropertyWithGetter:@selector
(showsHorizontalScrollIndicator)
setter:@selector
(setShowsHorizontalScrollIndicator:)
type:@encode(BOOL)];
[store registerNonObjectPropertyWithGetter:@selector
(showsVerticalScrollIndicator)
setter:@selector
(setShowsVerticalScrollIndicator:)
type:@encode(BOOL)];
[store
registerNonObjectPropertyWithGetter:@selector(canCancelContentTouches)
setter:@selector(setCanCancelContentTouches:)
type:@encode(BOOL)];
[store registerNonObjectPropertyWithGetter:@selector(delaysContentTouches)
setter:@selector(setDelaysContentTouches:)
type:@encode(BOOL)];
[store
registerNonObjectPropertyWithGetter:@selector(keyboardDismissMode)
setter:@selector(setKeyboardDismissMode:)
type:@encode(
UIScrollViewKeyboardDismissMode)];
[store registerNonObjectPropertyWithGetter:@selector(indexDisplayMode)
setter:@selector(setIndexDisplayMode:)
type:@encode(
UIScrollViewIndexDisplayMode)];
[store
registerNonObjectPropertyWithGetter:@selector(indicatorStyle)
setter:@selector(setIndicatorStyle:)
type:@encode(UIScrollViewIndicatorStyle)];
// UIView properties.
[store registerObjectPropertyWithGetter:@selector(backgroundColor)
setter:@selector(setBackgroundColor:)
attribute:CRWStoredPropertyAttributeCopy];
[store registerNonObjectPropertyWithGetter:@selector(isHidden)
setter:@selector(setHidden:)
type:@encode(BOOL)];
[store registerNonObjectPropertyWithGetter:@selector(alpha)
setter:@selector(setAlpha:)
type:@encode(CGFloat)];
[store registerNonObjectPropertyWithGetter:@selector(isOpaque)
setter:@selector(setOpaque:)
type:@encode(BOOL)];
[store registerObjectPropertyWithGetter:@selector(tintColor)
setter:@selector(setTintColor:)
attribute:CRWStoredPropertyAttributeStrong];
[store registerNonObjectPropertyWithGetter:@selector(tintAdjustmentMode)
setter:@selector(setTintAdjustmentMode:)
type:@encode(UIViewTintAdjustmentMode)];
[store
registerNonObjectPropertyWithGetter:@selector(clearsContextBeforeDrawing)
setter:@selector
(setClearsContextBeforeDrawing:)
type:@encode(BOOL)];
[store registerObjectPropertyWithGetter:@selector(maskView)
setter:@selector(setMaskView:)
attribute:CRWStoredPropertyAttributeStrong];
[store
registerNonObjectPropertyWithGetter:@selector(isUserInteractionEnabled)
setter:@selector(setUserInteractionEnabled:)
type:@encode(BOOL)];
[store registerNonObjectPropertyWithGetter:@selector(isMultipleTouchEnabled)
setter:@selector(setMultipleTouchEnabled:)
type:@encode(BOOL)];
[store registerNonObjectPropertyWithGetter:@selector(isExclusiveTouch)
setter:@selector(setExclusiveTouch:)
type:@encode(BOOL)];
return store;
} }
- (instancetype)init { - (instancetype)init {
...@@ -265,11 +138,6 @@ ...@@ -265,11 +138,6 @@
// including those defined in third-party categories, because it provides // including those defined in third-party categories, because it provides
// -asUIScrollView method. // -asUIScrollView method.
_underlyingScrollView = [[UIScrollView alloc] init]; _underlyingScrollView = [[UIScrollView alloc] init];
if (base::FeatureList::IsEnabled(
web::features::kPreserveScrollViewProperties)) {
_propertiesStore = [self.class propertiesStore];
}
} }
return self; return self;
} }
...@@ -296,38 +164,26 @@ ...@@ -296,38 +164,26 @@
scrollView = [[UIScrollView alloc] init]; scrollView = [[UIScrollView alloc] init];
} }
// Clean up the delegate/observers of the old scroll view, and save its // Clean up the delegate/observers of the old scroll view.
// properties for later restoration.
[self.underlyingScrollView setDelegate:nil]; [self.underlyingScrollView setDelegate:nil];
[self stopObservingScrollView:self.underlyingScrollView]; [self stopObservingScrollView:self.underlyingScrollView];
if (base::FeatureList::IsEnabled(
web::features::kPreserveScrollViewProperties) &&
self.underlyingScrollView) {
// TODO(crbug.com/1023250): Simplify this by directly assigning the
// properties of the old scroll view to those in the new scroll view.
[_propertiesStore savePropertiesFromObject:self.underlyingScrollView];
}
// Set up the delegate/observers of the new scroll view, and restore its // Set up the delegate/observers of the new scroll view.
// properties.
DCHECK(!scrollView.delegate); DCHECK(!scrollView.delegate);
scrollView.delegate = self.delegateProxy; scrollView.delegate = self.delegateProxy;
[self startObservingScrollView:scrollView]; [self startObservingScrollView:scrollView];
if (base::FeatureList::IsEnabled( if (base::FeatureList::IsEnabled(
web::features::kPreserveScrollViewProperties) && web::features::kPreserveScrollViewProperties)) {
scrollView) { [self preservePropertiesFromOldScrollView:self.underlyingScrollView
// TODO(crbug.com/1023250): Simplify this by directly assigning the toNewScrollView:scrollView];
// properties of the old scroll view to those in the new scroll view.
[_propertiesStore loadPropertiesToObject:scrollView];
// Clear the stored values of the properties. This prevents from keeping
// retaining old property values.
[_propertiesStore clearValues];
} }
self.underlyingScrollView = scrollView; self.underlyingScrollView = scrollView;
// TODO(crbug.com/1023250): Restore these using CRWPropertiesStore once the // TODO(crbug.com/1023250): Restore these in
// feature flag kPreserveScrollViewProperties is removed. // -preservePropertiesFromOldScrollView:toNewScrollView: once the feature flag
// kPreserveScrollViewProperties is removed.
if (_storedClipsToBounds) { if (_storedClipsToBounds) {
scrollView.clipsToBounds = *_storedClipsToBounds; scrollView.clipsToBounds = *_storedClipsToBounds;
} }
...@@ -341,10 +197,58 @@ ...@@ -341,10 +197,58 @@
[_observers webViewScrollViewProxyDidSetScrollView:self]; [_observers webViewScrollViewProxyDidSetScrollView:self];
} }
// Preserves properties of the underlying scroll view when it changes from
// |oldScrollView| to |newScrollView|.
//
// This is necessary to avoid losing properties set against the proxy when the
// underlying scroll view is reset.
- (void)preservePropertiesFromOldScrollView:(UIScrollView*)oldScrollView
toNewScrollView:(UIScrollView*)newScrollView {
// This method should preserve all properties of UIScrollView and its
// ancestor classes (not limited to properties explicitly declared in
// CRWWebViewScrollViewProxy) which:
// - is a readwrite property
// - AND is supposed to be modified directly, considering it's a scroll
// view of a web view. e.g., |frame| and |subviews| do not meet this
// condition because they are managed by the web view.
//
// Properties not explicitly declared in CRWWebViewScrollViewProxy can still
// be accessed via -asUIScrollView, so they should be preserved as well.
// UIScrollView properties.
newScrollView.scrollEnabled = oldScrollView.scrollEnabled;
newScrollView.directionalLockEnabled = oldScrollView.directionalLockEnabled;
newScrollView.pagingEnabled = oldScrollView.pagingEnabled;
newScrollView.scrollsToTop = oldScrollView.scrollsToTop;
newScrollView.bounces = oldScrollView.bounces;
newScrollView.alwaysBounceVertical = oldScrollView.alwaysBounceVertical;
newScrollView.alwaysBounceHorizontal = oldScrollView.alwaysBounceHorizontal;
newScrollView.showsHorizontalScrollIndicator =
oldScrollView.showsHorizontalScrollIndicator;
newScrollView.showsVerticalScrollIndicator =
oldScrollView.showsVerticalScrollIndicator;
newScrollView.canCancelContentTouches = oldScrollView.canCancelContentTouches;
newScrollView.delaysContentTouches = oldScrollView.delaysContentTouches;
newScrollView.keyboardDismissMode = oldScrollView.keyboardDismissMode;
newScrollView.indexDisplayMode = oldScrollView.indexDisplayMode;
newScrollView.indicatorStyle = oldScrollView.indicatorStyle;
// UIView properties.
newScrollView.backgroundColor = oldScrollView.backgroundColor;
newScrollView.hidden = oldScrollView.hidden;
newScrollView.alpha = oldScrollView.alpha;
newScrollView.opaque = oldScrollView.opaque;
newScrollView.tintColor = oldScrollView.tintColor;
newScrollView.tintAdjustmentMode = oldScrollView.tintAdjustmentMode;
newScrollView.clearsContextBeforeDrawing =
oldScrollView.clearsContextBeforeDrawing;
newScrollView.maskView = oldScrollView.maskView;
newScrollView.userInteractionEnabled = oldScrollView.userInteractionEnabled;
newScrollView.multipleTouchEnabled = oldScrollView.multipleTouchEnabled;
newScrollView.exclusiveTouch = oldScrollView.exclusiveTouch;
}
- (BOOL)clipsToBounds { - (BOOL)clipsToBounds {
if (!self.underlyingScrollView && _storedClipsToBounds) {
return *_storedClipsToBounds;
}
return self.underlyingScrollView.clipsToBounds; return self.underlyingScrollView.clipsToBounds;
} }
...@@ -355,13 +259,7 @@ ...@@ -355,13 +259,7 @@
- (UIScrollViewContentInsetAdjustmentBehavior)contentInsetAdjustmentBehavior - (UIScrollViewContentInsetAdjustmentBehavior)contentInsetAdjustmentBehavior
API_AVAILABLE(ios(11.0)) { API_AVAILABLE(ios(11.0)) {
if (self.underlyingScrollView) { return [self.underlyingScrollView contentInsetAdjustmentBehavior];
return [self.underlyingScrollView contentInsetAdjustmentBehavior];
} else if (_storedContentInsetAdjustmentBehavior) {
return *_storedContentInsetAdjustmentBehavior;
} else {
return UIScrollViewContentInsetAdjustmentAutomatic;
}
} }
- (void)setContentInsetAdjustmentBehavior: - (void)setContentInsetAdjustmentBehavior:
...@@ -375,7 +273,7 @@ ...@@ -375,7 +273,7 @@
} }
- (NSArray<__kindof UIView*>*)subviews { - (NSArray<__kindof UIView*>*)subviews {
return self.underlyingScrollView ? [self.underlyingScrollView subviews] : @[]; return [self.underlyingScrollView subviews];
} }
#pragma mark - #pragma mark -
...@@ -422,41 +320,15 @@ ...@@ -422,41 +320,15 @@
- (NSMethodSignature*)methodSignatureForSelector:(SEL)sel { - (NSMethodSignature*)methodSignatureForSelector:(SEL)sel {
// Called when this proxy is accessed through -asUIScrollView and the method // Called when this proxy is accessed through -asUIScrollView and the method
// is not implemented in this class. Do not call [self.underlyingScrollView // is not implemented in this class.
// methodSignatureForSelector:] here instead because self.underlyingScrollView return [self.underlyingScrollView methodSignatureForSelector:sel];
// may be nil.
return [UIScrollView instanceMethodSignatureForSelector:sel];
} }
- (void)forwardInvocation:(NSInvocation*)invocation { - (void)forwardInvocation:(NSInvocation*)invocation {
// Called when this proxy is accessed through -asUIScrollView and the method // Called when this proxy is accessed through -asUIScrollView and the method
// is not implemented in this class. // is not implemented in this class. Forwards the invocation to the undelrying
if (self.underlyingScrollView) { // scroll view.
// Forwards the invocation to the undelrying scroll view. [invocation invokeWithTarget:self.underlyingScrollView];
[invocation invokeWithTarget:self.underlyingScrollView];
} else {
BOOL handled = NO;
if (base::FeatureList::IsEnabled(
web::features::kPreserveScrollViewProperties)) {
// Forwards the invocation to the property store. If it is an invocation
// of a getter or setter of a preserved property, it gets or sets the
// property in the property store, which is later restored in the
// underlying scroll view.
handled =
[_propertiesStore forwardInvocationToPropertiesStore:invocation];
}
// If it is not an invocation of a getter or setter of a preserved property,
// do nothing and return a zero value. This must be done explicitly because
// it looks not guaranteed to return a zero value if it does nothing here or
// call [invocation invokeWithTarget:nil].
if (!handled && invocation.methodSignature.methodReturnLength > 0) {
// NSMutableData is initialized with zero bytes.
NSMutableData* zeroData = [NSMutableData
dataWithLength:invocation.methodSignature.methodReturnLength];
[invocation setReturnValue:zeroData.mutableBytes];
}
}
} }
#pragma mark - NSObject #pragma mark - NSObject
......
ios/web/web_state/ui/crw_web_view_scroll_view_proxy_unittest.mm
View file @ 05bc8d81
...@@ -590,7 +590,7 @@ TEST_F(CRWWebViewScrollViewProxyTest, RemoveKVObserver) { ...@@ -590,7 +590,7 @@ TEST_F(CRWWebViewScrollViewProxyTest, RemoveKVObserver) {
// - the setter is called when the underlying scroll view is not set // - the setter is called when the underlying scroll view is not set
// - the getter is called after the underlying scroll view is still not set // - the getter is called after the underlying scroll view is still not set
TEST_F(CRWWebViewScrollViewProxyTest, TEST_F(CRWWebViewScrollViewProxyTest,
PreservePropertiesWhileUnderlyingScrollViewIsNil) { PreservePropertiesWhileUnderlyingScrollViewIsAbsent) {
base::test::ScopedFeatureList scoped_feature_list; base::test::ScopedFeatureList scoped_feature_list;
scoped_feature_list.InitAndEnableFeature( scoped_feature_list.InitAndEnableFeature(
web::features::kPreserveScrollViewProperties); web::features::kPreserveScrollViewProperties);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment