Commit 60ef4a8c authored by edchin's avatar edchin Committed by Commit Bot

[ios] Fix page restoration placeholder

This CL fixes two bugs in the page restoration placeholder:

1) The alpha of the placeholder was not previously being reset to 1.0.

2) The page restoration placeholder was only being shown at the start
of a navigation. This means that the user will open a tab, it will
momentarily show blank (since the page has not yet been restored)
before showing the black/white placeholder. Now the black/white page
will show immediately as the user enters the tab.

Bug: 927843
Change-Id: If8ad4c60e05c84a240182316484e2657d46efaf0
Reviewed-on: https://chromium-review.googlesource.com/c/1450438
Commit-Queue: edchin <edchin@chromium.org>
Reviewed-by: default avataredchin <edchin@chromium.org>
Reviewed-by: default avatarEugene But <eugenebut@chromium.org>
Cr-Commit-Position: refs/heads/master@{#628579}
parent c42fdf5e
...@@ -43,6 +43,8 @@ class PagePlaceholderTabHelper ...@@ -43,6 +43,8 @@ class PagePlaceholderTabHelper
explicit PagePlaceholderTabHelper(web::WebState* web_state); explicit PagePlaceholderTabHelper(web::WebState* web_state);
// web::WebStateObserver overrides: // web::WebStateObserver overrides:
void WasShown(web::WebState* web_state) override;
void WasHidden(web::WebState* web_state) override;
void DidStartNavigation(web::WebState* web_state, void DidStartNavigation(web::WebState* web_state,
web::NavigationContext* navigation_context) override; web::NavigationContext* navigation_context) override;
void PageLoaded( void PageLoaded(
......
...@@ -42,6 +42,16 @@ void PagePlaceholderTabHelper::CancelPlaceholderForNextNavigation() { ...@@ -42,6 +42,16 @@ void PagePlaceholderTabHelper::CancelPlaceholderForNextNavigation() {
} }
} }
void PagePlaceholderTabHelper::WasShown(web::WebState* web_state) {
if (add_placeholder_for_next_navigation_) {
AddPlaceholder();
}
}
void PagePlaceholderTabHelper::WasHidden(web::WebState* web_state) {
RemovePlaceholder();
}
void PagePlaceholderTabHelper::DidStartNavigation( void PagePlaceholderTabHelper::DidStartNavigation(
web::WebState* web_state, web::WebState* web_state,
web::NavigationContext* navigation_context) { web::NavigationContext* navigation_context) {
...@@ -84,18 +94,22 @@ void PagePlaceholderTabHelper::AddPlaceholder() { ...@@ -84,18 +94,22 @@ void PagePlaceholderTabHelper::AddPlaceholder() {
__weak UIImageView* weak_placeholder_view = placeholder_view_; __weak UIImageView* weak_placeholder_view = placeholder_view_;
__weak UIView* weak_web_state_view = web_state_->GetView(); __weak UIView* weak_web_state_view = web_state_->GetView();
__weak id<CRWWebViewProxy> web_view_proxy = web_state_->GetWebViewProxy(); __weak id<CRWWebViewProxy> web_view_proxy = web_state_->GetWebViewProxy();
SnapshotTabHelper::FromWebState(web_state_)
->RetrieveGreySnapshot(^(UIImage* snapshot) { SnapshotTabHelper* snapshotTabHelper =
CGRect frame = weak_web_state_view.frame; SnapshotTabHelper::FromWebState(web_state_);
UIEdgeInsets inset = web_view_proxy.contentInset; if (snapshotTabHelper) {
frame.origin.x += inset.left; snapshotTabHelper->RetrieveGreySnapshot(^(UIImage* snapshot) {
frame.origin.y += inset.top; CGRect frame = weak_web_state_view.frame;
frame.size.width -= (inset.right + inset.left); UIEdgeInsets inset = web_view_proxy.contentInset;
frame.size.height -= (inset.bottom + inset.top); frame.origin.x += inset.left;
weak_placeholder_view.frame = frame; frame.origin.y += inset.top;
weak_placeholder_view.image = snapshot; frame.size.width -= (inset.right + inset.left);
[weak_web_state_view addSubview:weak_placeholder_view]; frame.size.height -= (inset.bottom + inset.top);
}); weak_placeholder_view.frame = frame;
weak_placeholder_view.image = snapshot;
[weak_web_state_view addSubview:weak_placeholder_view];
});
}
// Remove placeholder if it takes too long to load the page. // Remove placeholder if it takes too long to load the page.
base::ThreadTaskRunnerHandle::Get()->PostDelayedTask( base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
...@@ -119,5 +133,6 @@ void PagePlaceholderTabHelper::RemovePlaceholder() { ...@@ -119,5 +133,6 @@ void PagePlaceholderTabHelper::RemovePlaceholder() {
} }
completion:^(BOOL finished) { completion:^(BOOL finished) {
[weak_placeholder_view removeFromSuperview]; [weak_placeholder_view removeFromSuperview];
weak_placeholder_view.alpha = 1.0f;
}]; }];
} }
...@@ -53,6 +53,36 @@ class PagePlaceholderTabHelperTest : public PlatformTest { ...@@ -53,6 +53,36 @@ class PagePlaceholderTabHelperTest : public PlatformTest {
UIView* web_state_view_ = nil; UIView* web_state_view_ = nil;
}; };
// Tests that placeholder is not shown after WasShown() if it was not requested.
TEST_F(PagePlaceholderTabHelperTest, TabShownAndPlaceholderNotShown) {
ASSERT_FALSE(tab_helper()->displaying_placeholder());
ASSERT_FALSE(tab_helper()->will_add_placeholder_for_next_navigation());
web_state_->WasShown();
EXPECT_FALSE(tab_helper()->displaying_placeholder());
EXPECT_FALSE(tab_helper()->will_add_placeholder_for_next_navigation());
}
// Tests that placeholder is shown after WasShown() if it was requested.
TEST_F(PagePlaceholderTabHelperTest, TabShownAndPlaceholderShown) {
ASSERT_FALSE(tab_helper()->displaying_placeholder());
ASSERT_FALSE(tab_helper()->will_add_placeholder_for_next_navigation());
tab_helper()->AddPlaceholderForNextNavigation();
ASSERT_FALSE(tab_helper()->displaying_placeholder());
EXPECT_TRUE(tab_helper()->will_add_placeholder_for_next_navigation());
web_state_->WasShown();
EXPECT_TRUE(tab_helper()->displaying_placeholder());
EXPECT_TRUE(tab_helper()->will_add_placeholder_for_next_navigation());
}
// Tests that placeholder is removed after WasHidden().
TEST_F(PagePlaceholderTabHelperTest, TabHiddenAndPlaceholderRemoved) {
tab_helper()->AddPlaceholderForNextNavigation();
web_state_->WasShown();
ASSERT_TRUE(tab_helper()->displaying_placeholder());
web_state_->WasHidden();
EXPECT_FALSE(tab_helper()->displaying_placeholder());
}
// Tests that placeholder is not shown after DidStartNavigation if it was not // Tests that placeholder is not shown after DidStartNavigation if it was not
// requested. // requested.
TEST_F(PagePlaceholderTabHelperTest, NotShown) { TEST_F(PagePlaceholderTabHelperTest, NotShown) {
......
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