Commit 0a8d0279 authored by Dana Fried's avatar Dana Fried Committed by Commit Bot

Working hover card implementation using a state machine.

Much more reliable capture based on a state machine. Now thumbnail
capture walks through the following steps:
 - Navigating to new page
 - Navigation complete
 - Capture requested
 - Frame capture
 - Cooldown*
 - Capture complete**

 * Cooldown allows us to continue capturing for a short period of time
   after a page indicates it's loaded; this gives the renderer time to
   actually draw the final version of the page
** Once we've captured a background page, there's no further need to try
   to capture it again unless it navigates, or the user makes it active,
   interacts, and then leaves again (a case we handle separately).

Typically these states move forward. However, there are potential issues
with trying to frame capture a visible/active tab, so we pause video
capture while a page is active and then return to capturing it after the
user switches away. Since we do not show preview images for the current
tab in desktop mode, it's not a problem. For tablet mode (Mohnstrudel),
if the tabstrip is observing the thumbnail image for the current tab,
the tab will be captured.

This complex behavior of stepping forward and backwards based on whether
a tab is visible, being observed, etc. is all handled by
ThumbnailTabHelper::ThumbnailTabHelperState::UpdateCaptureState().

Still to do:
 - Create a queue of thumbnails to be captured that prioritizes
   thumbnails for pages that are being or have recently been observed,
   to actively limit the number of thumbnails we are trying to observe
   at once in order to save memory. In-flight thumbnails are
   uncompressed, which can have a significant impact on RAM usage.

Bug: 1057713, 1059862, 1068459
Fixes: 1068459
Change-Id: If81798c0d3df26a5c309e834483042a5b5f24857
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2145161
Commit-Queue: Dana Fried <dfried@chromium.org>
Reviewed-by: default avatarCollin Baker <collinbaker@chromium.org>
Cr-Commit-Position: refs/heads/master@{#761111}
parent 0e778ec5
...@@ -29,7 +29,6 @@ class ThumbnailTabHelper ...@@ -29,7 +29,6 @@ class ThumbnailTabHelper
private: private:
class TabStateTracker; class TabStateTracker;
friend class content::WebContentsUserData<ThumbnailTabHelper>; friend class content::WebContentsUserData<ThumbnailTabHelper>;
friend class TabStateTracker;
// Metrics enums and helper functions: // Metrics enums and helper functions:
enum class CaptureType; enum class CaptureType;
...@@ -61,7 +60,7 @@ class ThumbnailTabHelper ...@@ -61,7 +60,7 @@ class ThumbnailTabHelper
// before a page is frozen or swapped out. // before a page is frozen or swapped out.
void StartVideoCapture(); void StartVideoCapture();
void StopVideoCapture(); void StopVideoCapture();
void CaptureThumbnailOnTabSwitch(); void CaptureThumbnailOnTabHidden();
void StoreThumbnailForTabSwitch(base::TimeTicks start_time, void StoreThumbnailForTabSwitch(base::TimeTicks start_time,
const SkBitmap& bitmap); const SkBitmap& bitmap);
void StoreThumbnail(CaptureType type, const SkBitmap& bitmap); void StoreThumbnail(CaptureType type, const SkBitmap& bitmap);
......
...@@ -35,6 +35,7 @@ class ThumbnailWaiter : public ThumbnailImage::Observer { ...@@ -35,6 +35,7 @@ class ThumbnailWaiter : public ThumbnailImage::Observer {
DCHECK(!thumbnail_); DCHECK(!thumbnail_);
thumbnail_ = thumbnail; thumbnail_ = thumbnail;
scoped_observer_.Add(thumbnail); scoped_observer_.Add(thumbnail);
thumbnail_->RequestThumbnailImage();
run_loop_.Run(); run_loop_.Run();
return image_; return image_;
} }
...@@ -186,9 +187,6 @@ IN_PROC_BROWSER_TEST_F( ...@@ -186,9 +187,6 @@ IN_PROC_BROWSER_TEST_F(
EXPECT_EQ(kTabCount, browser2->tab_strip_model()->count()); EXPECT_EQ(kTabCount, browser2->tab_strip_model()->count());
CloseBrowserSynchronously(browser2); CloseBrowserSynchronously(browser2);
// Limit the number of restored tabs that are loaded.
TabLoaderTester::SetMaxLoadedTabCountForTesting(2);
// When the tab loader is created configure it for this test. This ensures // When the tab loader is created configure it for this test. This ensures
// that no more than 1 loading slot is used for the test. // that no more than 1 loading slot is used for the test.
base::RepeatingCallback<void(TabLoader*)> callback = base::BindRepeating( base::RepeatingCallback<void(TabLoader*)> callback = base::BindRepeating(
......
...@@ -319,70 +319,47 @@ class TabHoverCardBubbleView::FadeLabel : public views::Label { ...@@ -319,70 +319,47 @@ class TabHoverCardBubbleView::FadeLabel : public views::Label {
// Maintains a set of thumbnails to watch, ensuring the capture count on the // Maintains a set of thumbnails to watch, ensuring the capture count on the
// associated WebContents stays nonzero until a valid thumbnail has been // associated WebContents stays nonzero until a valid thumbnail has been
// captured. // captured.
class TabHoverCardBubbleView::ThumbnailWatcher { class TabHoverCardBubbleView::ThumbnailObserver
: public ThumbnailImage::Observer {
public: public:
explicit ThumbnailWatcher(TabHoverCardBubbleView* hover_card) explicit ThumbnailObserver(TabHoverCardBubbleView* hover_card)
: hover_card_(hover_card) {} : hover_card_(hover_card) {}
~ThumbnailWatcher() = default; ~ThumbnailObserver() override = default;
// Begin watching the specified thumbnail image for updates. Ideally, should // Begin watching the specified thumbnail image for updates. Ideally, should
// trigger the associated WebContents to load (if not loaded already) and // trigger the associated WebContents to load (if not loaded already) and
// retrieve a valid thumbnail. If too many thumbnails are being watched, the // retrieve a valid thumbnail. If too many thumbnails are being watched, the
// least-recently watched will be unwatched. // least-recently watched will be unwatched.
void Watch(scoped_refptr<ThumbnailImage> thumbnail_image) { void Observe(scoped_refptr<ThumbnailImage> thumbnail_image) {
ThumbnailImage* const ptr = thumbnail_image.get(); if (current_image_ == thumbnail_image)
auto it = recent_observers_.Get(ptr); return;
if (it == recent_observers_.end()) {
recent_observers_.Put(ptr, std::make_unique<ThumbnailObserver>( scoped_observer_.RemoveAll();
this, std::move(thumbnail_image))); current_image_ = std::move(thumbnail_image);
if (current_image_) {
scoped_observer_.Add(current_image_.get());
current_image_->RequestThumbnailImage();
} }
ptr->RequestThumbnailImage();
} }
// Returns the current (most recent) thumbnail being watched. // Returns the current (most recent) thumbnail being watched.
ThumbnailImage* current_image() const { const scoped_refptr<ThumbnailImage>& current_image() const {
return recent_observers_.empty() ? nullptr return current_image_;
: recent_observers_.begin()->first;
} }
void OnNewImage(const ThumbnailImage* thumbnail, gfx::ImageSkia image) { base::Optional<gfx::Size> GetThumbnailSizeHint() const override {
DCHECK(!recent_observers_.empty()); return TabStyle::GetPreviewImageSize();
if (recent_observers_.begin()->first == thumbnail)
hover_card_->OnThumbnailImageAvailable(std::move(image));
} }
private: void OnThumbnailImageAvailable(gfx::ImageSkia preview_image) override {
// Actually does the work of watching a single thumbnail. Cleans itself up hover_card_->OnThumbnailImageAvailable(std::move(preview_image));
// (including unregistering as an observer) on destruction. }
class ThumbnailObserver : public ThumbnailImage::Observer {
public:
ThumbnailObserver(ThumbnailWatcher* thumbnail_watcher,
scoped_refptr<ThumbnailImage> thumbnail_image)
: thumbnail_watcher_(thumbnail_watcher),
thumbnail_image_(std::move(thumbnail_image)) {
scoped_observer_.Add(thumbnail_image_.get());
}
~ThumbnailObserver() override = default;
base::Optional<gfx::Size> GetThumbnailSizeHint() const override {
return TabStyle::GetPreviewImageSize();
}
void OnThumbnailImageAvailable(gfx::ImageSkia preview_image) override {
thumbnail_watcher_->OnNewImage(thumbnail_image_.get(),
std::move(preview_image));
}
private:
ThumbnailWatcher* const thumbnail_watcher_;
scoped_refptr<ThumbnailImage> thumbnail_image_;
ScopedObserver<ThumbnailImage, ThumbnailImage::Observer> scoped_observer_{
this};
};
scoped_refptr<ThumbnailImage> current_image_;
TabHoverCardBubbleView* const hover_card_; TabHoverCardBubbleView* const hover_card_;
base::MRUCache<ThumbnailImage*, std::unique_ptr<ThumbnailObserver>> ScopedObserver<ThumbnailImage, ThumbnailImage::Observer> scoped_observer_{
recent_observers_{5}; this};
}; };
TabHoverCardBubbleView::TabHoverCardBubbleView(Tab* tab) TabHoverCardBubbleView::TabHoverCardBubbleView(Tab* tab)
...@@ -476,7 +453,7 @@ TabHoverCardBubbleView::TabHoverCardBubbleView(Tab* tab) ...@@ -476,7 +453,7 @@ TabHoverCardBubbleView::TabHoverCardBubbleView(Tab* tab)
std::make_unique<WidgetSlideAnimationDelegate>(this); std::make_unique<WidgetSlideAnimationDelegate>(this);
fade_animation_delegate_ = fade_animation_delegate_ =
std::make_unique<WidgetFadeAnimationDelegate>(widget_); std::make_unique<WidgetFadeAnimationDelegate>(widget_);
thumbnail_watcher_ = std::make_unique<ThumbnailWatcher>(this); thumbnail_observer_ = std::make_unique<ThumbnailObserver>(this);
constexpr int kFootnoteVerticalMargin = 8; constexpr int kFootnoteVerticalMargin = 8;
GetBubbleFrameView()->set_footnote_margins( GetBubbleFrameView()->set_footnote_margins(
...@@ -575,6 +552,7 @@ void TabHoverCardBubbleView::FadeOutToHide() { ...@@ -575,6 +552,7 @@ void TabHoverCardBubbleView::FadeOutToHide() {
delayed_show_timer_.Stop(); delayed_show_timer_.Stop();
if (!widget_->IsVisible()) if (!widget_->IsVisible())
return; return;
thumbnail_observer_->Observe(nullptr);
slide_animation_delegate_->StopAnimation(); slide_animation_delegate_->StopAnimation();
last_visible_timestamp_ = base::TimeTicks::Now(); last_visible_timestamp_ = base::TimeTicks::Now();
if (disable_animations_for_testing_) { if (disable_animations_for_testing_) {
...@@ -718,13 +696,17 @@ void TabHoverCardBubbleView::UpdateCardContent(const Tab* tab) { ...@@ -718,13 +696,17 @@ void TabHoverCardBubbleView::UpdateCardContent(const Tab* tab) {
domain_label_->SetText(domain); domain_label_->SetText(domain);
// If the preview image feature is not enabled, |preview_image_| will be null. // If the preview image feature is not enabled, |preview_image_| will be null.
if (preview_image_ && preview_image_->GetVisible()) { if (preview_image_) {
auto thumbnail = tab->data().thumbnail; if (preview_image_->GetVisible()) {
if (!thumbnail) { auto thumbnail = tab->data().thumbnail;
ClearPreviewImage(); if (!thumbnail) {
} else if (thumbnail != thumbnail_watcher_->current_image()) { ClearPreviewImage();
waiting_for_decompress_ = true; } else if (thumbnail != thumbnail_observer_->current_image()) {
thumbnail_watcher_->Watch(thumbnail); waiting_for_decompress_ = true;
thumbnail_observer_->Observe(thumbnail);
}
} else {
thumbnail_observer_->Observe(nullptr);
} }
} }
} }
......
...@@ -66,7 +66,7 @@ class TabHoverCardBubbleView : public views::BubbleDialogDelegateView { ...@@ -66,7 +66,7 @@ class TabHoverCardBubbleView : public views::BubbleDialogDelegateView {
class WidgetFadeAnimationDelegate; class WidgetFadeAnimationDelegate;
class WidgetSlideAnimationDelegate; class WidgetSlideAnimationDelegate;
class FadeLabel; class FadeLabel;
class ThumbnailWatcher; class ThumbnailObserver;
// Get delay in milliseconds based on tab width. // Get delay in milliseconds based on tab width.
base::TimeDelta GetDelay(int tab_width) const; base::TimeDelta GetDelay(int tab_width) const;
...@@ -100,7 +100,7 @@ class TabHoverCardBubbleView : public views::BubbleDialogDelegateView { ...@@ -100,7 +100,7 @@ class TabHoverCardBubbleView : public views::BubbleDialogDelegateView {
std::unique_ptr<WidgetFadeAnimationDelegate> fade_animation_delegate_; std::unique_ptr<WidgetFadeAnimationDelegate> fade_animation_delegate_;
// Used to animate the tab hover card's movement between tabs. // Used to animate the tab hover card's movement between tabs.
std::unique_ptr<WidgetSlideAnimationDelegate> slide_animation_delegate_; std::unique_ptr<WidgetSlideAnimationDelegate> slide_animation_delegate_;
std::unique_ptr<ThumbnailWatcher> thumbnail_watcher_; std::unique_ptr<ThumbnailObserver> thumbnail_observer_;
// Timestamp of the last time a hover card was visible, recorded before it is // Timestamp of the last time a hover card was visible, recorded before it is
// hidden. This is used for metrics. // hidden. This is used for metrics.
......
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