Commit fe0e620b authored by edchin's avatar edchin

[ios] Remove use of default snapshot image

The default snapshot image is a simply a 2x2 white pixel image. This image is
unnecessarily passed around and unnecessarily complicates code.
This CL removes use of a default snapshot image, and updates the API
contract to use nil as the sentinel value for failed retrieval or
generation.

Bug: 918032
Change-Id: Ie7d8312cad6d05912474c6205c1bbb4ec9d242e0
Reviewed-on: https://chromium-review.googlesource.com/c/1392264Reviewed-by: default avatarMark Cogan <marq@chromium.org>
Reviewed-by: default avataredchin <edchin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619565}
parent 0871953b
......@@ -1423,21 +1423,18 @@ enum class EnterTabSwitcherSnapshotResult {
BOOL loading = currentWebState->IsLoading();
SnapshotTabHelper::FromWebState(currentWebState)
->UpdateSnapshotWithCallback(^(UIImage* snapshot) {
BOOL failed =
!snapshot ||
snapshot == SnapshotTabHelper::GetDefaultSnapshotImage();
EnterTabSwitcherSnapshotResult snapshotResult;
if (loading && failed) {
if (loading && !snapshot) {
snapshotResult =
EnterTabSwitcherSnapshotResult::kPageLoadingAndSnapshotFailed;
} else if (loading && !failed) {
} else if (loading && snapshot) {
snapshotResult = EnterTabSwitcherSnapshotResult::
kPageLoadingAndSnapshotSucceeded;
} else if (!loading && failed) {
} else if (!loading && !snapshot) {
snapshotResult = EnterTabSwitcherSnapshotResult::
kPageNotLoadingAndSnapshotFailed;
} else {
DCHECK(!loading && !failed);
DCHECK(!loading && snapshot);
snapshotResult = EnterTabSwitcherSnapshotResult::
kPageNotLoadingAndSnapshotSucceeded;
}
......
......@@ -31,8 +31,7 @@ class WebState;
- (CGSize)snapshotSize;
// Gets a color snapshot for the current page, calling |callback| once it has
// been retrieved or regenerated. If the snapshot cannot be generated, the
// |callback| will be called with nil.
// been retrieved. Invokes |callback| with nil if a snapshot does not exist.
- (void)retrieveSnapshot:(void (^)(UIImage*))callback;
// Gets a grey snapshot for the current page, calling |callback| once it has
......@@ -59,9 +58,6 @@ class WebState;
// Requests deletion of the current page snapshot from disk and memory.
- (void)removeSnapshot;
// Returns an image to use as replacement of a missing snapshot.
+ (UIImage*)defaultSnapshotImage;
// The SnapshotGenerator delegate.
@property(nonatomic, weak) id<SnapshotGeneratorDelegate> delegate;
......
......@@ -87,20 +87,11 @@ BOOL ViewHierarchyContainsWKWebView(UIView* view) {
- (void)retrieveSnapshot:(void (^)(UIImage*))callback {
DCHECK(callback);
void (^wrappedCallback)(UIImage*) = ^(UIImage* image) {
if (!image) {
image = [SnapshotGenerator defaultSnapshotImage];
}
callback(image);
};
SnapshotCache* snapshotCache = self.snapshotCache;
if (snapshotCache) {
[snapshotCache retrieveImageForSessionID:_snapshotSessionId
callback:wrappedCallback];
if (self.snapshotCache) {
[self.snapshotCache retrieveImageForSessionID:_snapshotSessionId
callback:callback];
} else {
wrappedCallback(nil);
callback(nil);
}
}
......@@ -128,12 +119,9 @@ BOOL ViewHierarchyContainsWKWebView(UIView* view) {
- (UIImage*)updateSnapshot {
UIImage* snapshot = [self generateSnapshotWithOverlays:YES];
// Return default snapshot without caching it if the generation failed.
if (!snapshot) {
return [[self class] defaultSnapshotImage];
if (snapshot) {
[self.snapshotCache setImage:snapshot withSessionID:_snapshotSessionId];
}
[self.snapshotCache setImage:snapshot withSessionID:_snapshotSessionId];
return snapshot;
}
......@@ -218,23 +206,6 @@ BOOL ViewHierarchyContainsWKWebView(UIView* view) {
[self.snapshotCache removeImageWithSessionID:_snapshotSessionId];
}
+ (UIImage*)defaultSnapshotImage {
static UIImage* defaultSnapshotImage = nil;
if (!defaultSnapshotImage) {
CGRect frame = CGRectMake(0, 0, 2, 2);
UIGraphicsBeginImageContext(frame.size);
[[UIColor whiteColor] setFill];
CGContextFillRect(UIGraphicsGetCurrentContext(), frame);
UIImage* result = UIGraphicsGetImageFromCurrentImageContext();
UIGraphicsEndImageContext();
defaultSnapshotImage =
[result stretchableImageWithLeftCapWidth:1 topCapHeight:1];
}
return defaultSnapshotImage;
}
#pragma mark - Private methods
// Returns the frame of the snapshot. Will return an empty rectangle if the
......
......@@ -44,27 +44,31 @@ class SnapshotTabHelper : public infobars::InfoBarManager::Observer,
// Retrieves a color snapshot for the current page, invoking |callback|
// with the image. The callback may be called synchronously is there is
// a cached snapshot available in memory, otherwise it will be invoked
// asynchronously after retrieved from disk or re-generated.
// asynchronously after retrieved from disk. Invokes |callback| with nil if a
// snapshot does not exist.
void RetrieveColorSnapshot(void (^callback)(UIImage*));
// Retrieves a grey snapshot for the current page, invoking |callback|
// with the image. The callback may be called synchronously is there is
// a cached snapshot available in memory, otherwise it will be invoked
// asynchronously after retrieved from disk or re-generated.
// asynchronously after retrieved from disk or re-generated. Invokes
// |callback| with nil if a snapshot does not exist.
void RetrieveGreySnapshot(void (^callback)(UIImage*));
// Asynchronously generates a new snapshot, updates the snapshot cache, and
// runs |callback| with the new snapshot image.
// invokes |callback| with the new snapshot image. Invokes |callback| with nil
// if snapshot generation fails.
void UpdateSnapshotWithCallback(void (^callback)(UIImage*));
// DEPRECATED(crbug.com/917929): Use the asynchronous function
// |UpdateSnapshotWithCallback()| for all new callsites.
// Generates a new snapshot, updates the snapshot cache, and returns the new
// snapshot image.
// snapshot image. Returns nil if snapshot generation fails.
UIImage* UpdateSnapshot();
// Generates a new snapshot without any overlays, and returns the new snapshot
// image. This does not update the snapshot cache.
// image. This does not update the snapshot cache. Returns nil if snapshot
// generation fails.
UIImage* GenerateSnapshotWithoutOverlays();
// Requests deletion of the current page snapshot from disk and memory.
......@@ -73,9 +77,6 @@ class SnapshotTabHelper : public infobars::InfoBarManager::Observer,
// Instructs the helper not to snapshot content for the next page load event.
void IgnoreNextLoad();
// Returns an image to use as replacement of a missing snapshot.
static UIImage* GetDefaultSnapshotImage();
private:
SnapshotTabHelper(web::WebState* web_state, NSString* session_id);
......
......@@ -99,11 +99,6 @@ void SnapshotTabHelper::IgnoreNextLoad() {
ignore_next_load_ = true;
}
// static
UIImage* SnapshotTabHelper::GetDefaultSnapshotImage() {
return [SnapshotGenerator defaultSnapshotImage];
}
SnapshotTabHelper::SnapshotTabHelper(web::WebState* web_state,
NSString* session_id)
: web_state_(web_state),
......@@ -158,8 +153,7 @@ void SnapshotTabHelper::PageLoaded(
return;
PageLoadedSnapshotResult snapshotResult =
PageLoadedSnapshotResult::kSnapshotSucceeded;
if (!snapshot ||
snapshot == SnapshotTabHelper::GetDefaultSnapshotImage()) {
if (!snapshot) {
snapshotResult =
PageLoadedSnapshotResult::kSnapshotAttemptedAndFailed;
}
......
......@@ -164,8 +164,8 @@ TEST_F(SnapshotTabHelperTest, RetrieveColorSnapshotCachedSnapshot) {
EXPECT_EQ(delegate_.snapshotTakenCount, 0u);
}
// Tests that RetrieveColorSnapshot returns the default snapshot image when
// there is no cached snapshot and the WebState web usage is disabled.
// Tests that RetrieveColorSnapshot returns nil when there is no cached snapshot
// and the WebState web usage is disabled.
TEST_F(SnapshotTabHelperTest, RetrieveColorSnapshotWebUsageDisabled) {
web_state_.SetWebUsageEnabled(false);
......@@ -181,15 +181,12 @@ TEST_F(SnapshotTabHelperTest, RetrieveColorSnapshotWebUsageDisabled) {
run_loop.Run();
ASSERT_TRUE(snapshot);
EXPECT_TRUE(
UIImagesAreEqual(snapshot, SnapshotTabHelper::GetDefaultSnapshotImage()));
EXPECT_FALSE(snapshot);
EXPECT_EQ(delegate_.snapshotTakenCount, 0u);
}
// Tests that RetrieveColorSnapshot returns the default snapshot image when
// there is no cached snapshot and the delegate says it is not possible to
// take a snapshot.
// Tests that RetrieveColorSnapshot returns nil when there is no cached snapshot
// and the delegate says it is not possible to take a snapshot.
TEST_F(SnapshotTabHelperTest, RetrieveColorSnapshotCannotTakeSnapshot) {
delegate_.canTakeSnapshot = YES;
......@@ -205,9 +202,7 @@ TEST_F(SnapshotTabHelperTest, RetrieveColorSnapshotCannotTakeSnapshot) {
run_loop.Run();
ASSERT_TRUE(snapshot);
EXPECT_TRUE(
UIImagesAreEqual(snapshot, SnapshotTabHelper::GetDefaultSnapshotImage()));
EXPECT_FALSE(snapshot);
EXPECT_EQ(delegate_.snapshotTakenCount, 0u);
}
......@@ -234,8 +229,8 @@ TEST_F(SnapshotTabHelperTest, RetrieveGreySnapshotCachedSnapshot) {
EXPECT_EQ(delegate_.snapshotTakenCount, 0u);
}
// Tests that RetrieveGreySnapshot returns the default snapshot image when
// there is no cached snapshot and the WebState web usage is disabled.
// Tests that RetrieveGreySnapshot returns nil when there is no cached snapshot
// and the WebState web usage is disabled.
TEST_F(SnapshotTabHelperTest, RetrieveGreySnapshotWebUsageDisabled) {
web_state_.SetWebUsageEnabled(false);
......@@ -251,14 +246,12 @@ TEST_F(SnapshotTabHelperTest, RetrieveGreySnapshotWebUsageDisabled) {
run_loop.Run();
ASSERT_TRUE(snapshot);
EXPECT_TRUE(UIImagesAreEqual(
snapshot, GreyImage(SnapshotTabHelper::GetDefaultSnapshotImage())));
EXPECT_FALSE(snapshot);
EXPECT_EQ(delegate_.snapshotTakenCount, 0u);
}
// Tests that RetrieveGreySnapshot returns the default snapshot image when
// there is no cached snapshot and the WebState web usage is disabled.
// Tests that RetrieveGreySnapshot returns nil when there is no cached snapshot
// and the WebState web usage is disabled.
TEST_F(SnapshotTabHelperTest, RetrieveGreySnapshotCannotTakeSnapshot) {
delegate_.canTakeSnapshot = YES;
base::RunLoop run_loop;
......@@ -273,9 +266,7 @@ TEST_F(SnapshotTabHelperTest, RetrieveGreySnapshotCannotTakeSnapshot) {
run_loop.Run();
ASSERT_TRUE(snapshot);
EXPECT_TRUE(UIImagesAreEqual(
snapshot, GreyImage(SnapshotTabHelper::GetDefaultSnapshotImage())));
EXPECT_FALSE(snapshot);
EXPECT_EQ(delegate_.snapshotTakenCount, 0u);
}
......
......@@ -4195,7 +4195,7 @@ NSString* const kBrowserViewControllerSnackbarCategory =
->RetrieveColorSnapshot(^(UIImage* image) {
if (PagePlaceholderTabHelper::FromWebState(webStateBeingActivated)
->will_add_placeholder_for_next_navigation()) {
[swipeView setImage:SnapshotTabHelper::GetDefaultSnapshotImage()];
[swipeView setImage:nil];
} else {
[swipeView setImage:image];
}
......
......@@ -191,7 +191,7 @@ const CGFloat kResizeFactor = 4;
if (PagePlaceholderTabHelper::FromWebState(tab.webState)
->will_add_placeholder_for_next_navigation() &&
!ios::device_util::IsSingleCoreDevice()) {
[card setImage:SnapshotTabHelper::GetDefaultSnapshotImage()];
[card setImage:nil];
dispatch_async(priorityQueue, ^{
UIImage* greyImage = [self smallGreyImage:image];
dispatch_async(dispatch_get_main_queue(), ^{
......
......@@ -40,6 +40,7 @@
_imageView = [[UIImageView alloc] initWithFrame:CGRectZero];
[_imageView setClipsToBounds:YES];
[_imageView setContentMode:UIViewContentModeScaleAspectFill];
[_imageView setBackgroundColor:[UIColor whiteColor]];
[self addSubview:_imageView];
_topToolbarSnapshot = [[UIImageView alloc] initWithFrame:CGRectZero];
......
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