Commit 482140ae authored by Jiajun Ou's avatar Jiajun Ou Committed by Commit Bot

Implement NSFastEnumeration for BackForwardList API

Bug: 1045254
Change-Id: I797f1107a84bd235d0ffd7f69c7d46f11ee1f79a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2087232
Commit-Queue: Jiajun Ou <garzonou@google.com>
Reviewed-by: default avatarHiroshi Ichikawa <ichikawa@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747589}
parent 83073d57
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
@implementation CWVBackForwardListItemArray { @implementation CWVBackForwardListItemArray {
BOOL _isBackList; BOOL _isBackList;
CWVBackForwardListItem* _fastEnumerationItemCache;
} }
- (instancetype)initWithBackForwardList:(CWVBackForwardList*)list - (instancetype)initWithBackForwardList:(CWVBackForwardList*)list
...@@ -23,6 +25,8 @@ ...@@ -23,6 +25,8 @@
if (self) { if (self) {
_list = list; _list = list;
_isBackList = isBackList; _isBackList = isBackList;
_fastEnumerationItemCache = nil;
} }
return self; return self;
} }
...@@ -69,6 +73,34 @@ ...@@ -69,6 +73,34 @@
return [[CWVBackForwardListItem alloc] initWithNavigationItem:item]; return [[CWVBackForwardListItem alloc] initWithNavigationItem:item];
} }
#pragma mark - NSFastEnumeration
- (NSUInteger)countByEnumeratingWithState:(NSFastEnumerationState*)state
objects:(id __unsafe_unretained[])stackbuf
count:(NSUInteger)len {
DCHECK_GE(self.count, state->state);
DCHECK_GE(len, 1UL);
if (self.count == state->state) {
return 0;
}
// |state| is a pure C-struct so retaining an item in |state| is impossible.
// So, |_fastEnumerationItemCache| is designed to retain the returned item.
// It is the caller's responsibility to ensure this is not deallocated during
// enumeration, so a caller should not use nested for-in statement on the same
// list, otherwise it will cause a use-after-free bug.
_fastEnumerationItemCache = [self objectAtIndexedSubscript:state->state];
// This tells Obj-C runtime that |self| is immutable (because
// |&state->extra[0]| is constant) during enumeration.
state->mutationsPtr = &state->extra[0];
state->itemsPtr = stackbuf;
state->itemsPtr[0] = _fastEnumerationItemCache;
++state->state;
return 1;
}
@end @end
@implementation CWVBackForwardList @implementation CWVBackForwardList
......
...@@ -15,8 +15,24 @@ NS_ASSUME_NONNULL_BEGIN ...@@ -15,8 +15,24 @@ NS_ASSUME_NONNULL_BEGIN
@class CWVWebView; @class CWVWebView;
// This just behaves like a NSArray<CWVBackForwardListItem*>. // This just behaves like a NSArray<CWVBackForwardListItem*>.
//
// |objectAtIndexedSubscript:| and |NSFastEnumeration| are implemented to
// allow to use [] operator and a for-in statement to iterate |self|, like:
// for (CWVBackForwardListItem* item in backList) { ... }
// CWVBackForwardListItem* lastItem = backList[backList.count-1];
//
// WARNING: Do not use nested for-in statements to iterate one list at the
// same time, otherwise it will cause a use-after-free bug. Example:
// for (CWVBackForwardListItem* item in forwardList) { // OK
// for (CWVBackForwardListItem* item2 in forwardList) { // BAD!!
// // |item| will be deallocated here so the above line is BAD!
// }
// // |item| has been deallocated, so using |item| will cause a
// // use-after-free bug.
// for (CWVBackForwardListItem* item2 in backList) {} // OK
// }
CWV_EXPORT CWV_EXPORT
@interface CWVBackForwardListItemArray : NSObject @interface CWVBackForwardListItemArray : NSObject <NSFastEnumeration>
// The number of items in this array-like |self|. // The number of items in this array-like |self|.
@property(nonatomic, readonly) NSUInteger count; @property(nonatomic, readonly) NSUInteger count;
......
...@@ -250,4 +250,33 @@ TEST_F(WebViewBackForwardListTest, TestBackForwardListItemAtIndex) { ...@@ -250,4 +250,33 @@ TEST_F(WebViewBackForwardListTest, TestBackForwardListItemAtIndex) {
EXPECT_FALSE([list itemAtIndex:3]); EXPECT_FALSE([list itemAtIndex:3]);
} }
// Tests if a CWVBackForwardListItemArray can be correctly iterated using
// a for-in statement.
TEST_F(WebViewBackForwardListTest, TestCWVBackForwardListItemArrayForInLoop) {
ASSERT_TRUE(test_server_->Start());
GenerateTestPageUrls();
// Go to page3
ASSERT_TRUE(test::LoadUrl(web_view_, net::NSURLWithGURL(page1_url_)));
ASSERT_TRUE(test::LoadUrl(web_view_, net::NSURLWithGURL(page2_url_)));
ASSERT_TRUE(test::LoadUrl(web_view_, net::NSURLWithGURL(page3_url_)));
// Now it should be in page3
ASSERT_NSEQ(@"page3",
test::EvaluateJavaScript(web_view_, @"document.title", nil));
CWVBackForwardList* list = web_view_.backForwardList;
ASSERT_EQ(2UL, list.backList.count);
size_t i = 0;
for (CWVBackForwardListItem* item in list.backList) {
EXPECT_NSEQ(list.backList[i], item);
++i;
}
EXPECT_EQ(i, list.backList.count);
i = 0;
for (CWVBackForwardListItem* _ __unused in list.forwardList) {
++i;
}
EXPECT_EQ(i, list.forwardList.count);
}
} // namespace ios_web_view } // namespace ios_web_view
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