Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(417)

Side by Side Diff: chrome/browser/android/offline_pages/recent_tab_helper.cc

Issue 2773273002: Last_n: do not save snapshot of custom tabs. (Closed)
Patch Set: Renamed method; added custom tab checks to tests. Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
OLDNEW
1 // Copyright (c) 2016 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2016 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #include "chrome/browser/android/offline_pages/recent_tab_helper.h" 5 #include "chrome/browser/android/offline_pages/recent_tab_helper.h"
6 6
7 #include <queue> 7 #include <queue>
8 #include <vector> 8 #include <vector>
9 9
10 #include "base/bind.h" 10 #include "base/bind.h"
(...skipping 31 matching lines...) Expand 10 before | Expand all | Expand 10 after
42 std::unique_ptr<offline_pages::OfflinePageArchiver> CreatePageArchiver( 42 std::unique_ptr<offline_pages::OfflinePageArchiver> CreatePageArchiver(
43 content::WebContents* web_contents) override { 43 content::WebContents* web_contents) override {
44 return base::MakeUnique<offline_pages::OfflinePageMHTMLArchiver>( 44 return base::MakeUnique<offline_pages::OfflinePageMHTMLArchiver>(
45 web_contents); 45 web_contents);
46 } 46 }
47 bool GetTabId(content::WebContents* web_contents, int* tab_id) override { 47 bool GetTabId(content::WebContents* web_contents, int* tab_id) override {
48 return offline_pages::OfflinePageUtils::GetTabId(web_contents, tab_id); 48 return offline_pages::OfflinePageUtils::GetTabId(web_contents, tab_id);
49 } 49 }
50 bool IsLowEndDevice() override { return is_low_end_device_; } 50 bool IsLowEndDevice() override { return is_low_end_device_; }
51 51
52 bool IsCustomTab(content::WebContents* web_contents) override {
53 return offline_pages::OfflinePageUtils::CurrentlyShownInCustomTab(
54 web_contents);
55 }
56
52 private: 57 private:
53 // Cached value of whether low end device. 58 // Cached value of whether low end device.
54 bool is_low_end_device_; 59 bool is_low_end_device_;
55 }; 60 };
56 } // namespace 61 } // namespace
57 62
58 namespace offline_pages { 63 namespace offline_pages {
59 64
60 using PageQuality = SnapshotController::PageQuality; 65 using PageQuality = SnapshotController::PageQuality;
61 66
(...skipping 56 matching lines...) Expand 10 before | Expand all | Expand 10 after
118 << web_contents()->GetLastCommittedURL().spec(); 123 << web_contents()->GetLastCommittedURL().spec();
119 ReportDownloadStatusToRequestCoordinator(new_downloads_snapshot_info.get(), 124 ReportDownloadStatusToRequestCoordinator(new_downloads_snapshot_info.get(),
120 false); 125 false);
121 return; 126 return;
122 } 127 }
123 128
124 // If there is an ongoing snapshot request, completely ignore this one and 129 // If there is an ongoing snapshot request, completely ignore this one and
125 // cancel the Background Offliner request. 130 // cancel the Background Offliner request.
126 // TODO(carlosk): it might be better to make the decision to schedule or not 131 // TODO(carlosk): it might be better to make the decision to schedule or not
127 // the background request here. See https://crbug.com/686165. 132 // the background request here. See https://crbug.com/686165.
128 // TODO(carlosk): there is an edge case that happens when the ongoing request
129 // was automatically and transparently scheduled by a navigation event and
130 // this call happens due to the user pressing the download button. The user's
131 // request to download the page will be immediately dismissed. See
132 // https://crbug.com/686283.
133 if (downloads_ongoing_snapshot_info_) { 133 if (downloads_ongoing_snapshot_info_) {
134 DVLOG(1) << "Ongoing request exist; ignored download request for: " 134 DVLOG(1) << "Ongoing request exist; ignored download request for: "
135 << web_contents()->GetLastCommittedURL().spec(); 135 << web_contents()->GetLastCommittedURL().spec();
136 ReportDownloadStatusToRequestCoordinator(new_downloads_snapshot_info.get(), 136 ReportDownloadStatusToRequestCoordinator(new_downloads_snapshot_info.get(),
137 true); 137 true);
138 return; 138 return;
139 } 139 }
140 140
141 // Stores the new snapshot info. 141 // Stores the new snapshot info.
142 downloads_ongoing_snapshot_info_ = std::move(new_downloads_snapshot_info); 142 downloads_ongoing_snapshot_info_ = std::move(new_downloads_snapshot_info);
(...skipping 116 matching lines...) Expand 10 before | Expand all | Expand 10 after
259 downloads_ongoing_snapshot_info_.get(), false); 259 downloads_ongoing_snapshot_info_.get(), false);
260 } 260 }
261 // And cancel any ongoing snapshots. 261 // And cancel any ongoing snapshots.
262 CancelInFlightSnapshots(); 262 CancelInFlightSnapshots();
263 } 263 }
264 264
265 void RecentTabHelper::WasHidden() { 265 void RecentTabHelper::WasHidden() {
266 if (!IsOffliningRecentPagesEnabled()) 266 if (!IsOffliningRecentPagesEnabled())
267 return; 267 return;
268 268
269 // Return immediately if last_n is not listening to tab hidden events, if a 269 // Do not save a snapshots if any of these are true:
270 // last_n snapshot is currently being saved or if the tab is closing. 270 // - Last_n is not listening to tab hidden events.
271 // - A last_n snapshot is currently being saved.
272 // - The tab is in the process of being closed.
273 // - The tab is currently presented as a custom tab.
271 if (!last_n_listen_to_tab_hidden_ || last_n_ongoing_snapshot_info_ || 274 if (!last_n_listen_to_tab_hidden_ || last_n_ongoing_snapshot_info_ ||
272 tab_is_closing_) { 275 tab_is_closing_ || delegate_->IsCustomTab(web_contents())) {
273 DVLOG(1) << "Will not snapshot for last_n (reasons: " 276 DVLOG(1) << "Will not snapshot for last_n (reasons: "
274 << !last_n_listen_to_tab_hidden_ << ", " 277 << !last_n_listen_to_tab_hidden_ << ", "
275 << !!last_n_ongoing_snapshot_info_ << ", " << tab_is_closing_ 278 << !!last_n_ongoing_snapshot_info_ << ", " << tab_is_closing_
279 << ", " << delegate_->IsCustomTab(web_contents())
276 << ") for: " << web_contents()->GetLastCommittedURL().spec(); 280 << ") for: " << web_contents()->GetLastCommittedURL().spec();
277 return; 281 return;
278 } 282 }
279 283
280 // Do not save if page quality is too low. 284 // Do not save if page quality is too low.
281 // Note: we assume page quality for a page can only increase. 285 // Note: we assume page quality for a page can only increase.
282 if (snapshot_controller_->current_page_quality() == PageQuality::POOR) { 286 if (snapshot_controller_->current_page_quality() == PageQuality::POOR) {
283 DVLOG(1) << "Will not snapshot for last_n (page quality too low) for: " 287 DVLOG(1) << "Will not snapshot for last_n (page quality too low) for: "
284 << web_contents()->GetLastCommittedURL().spec(); 288 << web_contents()->GetLastCommittedURL().spec();
285 return; 289 return;
(...skipping 191 matching lines...) Expand 10 before | Expand all | Expand 10 after
477 } 481 }
478 482
479 void RecentTabHelper::CancelInFlightSnapshots() { 483 void RecentTabHelper::CancelInFlightSnapshots() {
480 weak_ptr_factory_.InvalidateWeakPtrs(); 484 weak_ptr_factory_.InvalidateWeakPtrs();
481 downloads_ongoing_snapshot_info_.reset(); 485 downloads_ongoing_snapshot_info_.reset();
482 downloads_latest_saved_snapshot_info_.reset(); 486 downloads_latest_saved_snapshot_info_.reset();
483 last_n_ongoing_snapshot_info_.reset(); 487 last_n_ongoing_snapshot_info_.reset();
484 } 488 }
485 489
486 } // namespace offline_pages 490 } // namespace offline_pages
OLDNEW

Powered by Google App Engine
This is Rietveld 408576698