Commit 803c4d72 authored by dfalcantara's avatar dfalcantara Committed by Commit bot

Unify how OfflinePages is called

* Provide a single jumping off point that avoids memory leaks when using
  the OfflinePageDownloadBridge from the menus.

* Provide a single function for determining whether the offline pages button
  should be clickable.

BUG=616324

Review-Url: https://codereview.chromium.org/2322773002
Cr-Commit-Position: refs/heads/master@{#417130}
parent e7134ec7
...@@ -90,7 +90,6 @@ import org.chromium.chrome.browser.nfc.BeamController; ...@@ -90,7 +90,6 @@ import org.chromium.chrome.browser.nfc.BeamController;
import org.chromium.chrome.browser.nfc.BeamProvider; import org.chromium.chrome.browser.nfc.BeamProvider;
import org.chromium.chrome.browser.offlinepages.OfflinePageBridge; import org.chromium.chrome.browser.offlinepages.OfflinePageBridge;
import org.chromium.chrome.browser.offlinepages.OfflinePageUtils; import org.chromium.chrome.browser.offlinepages.OfflinePageUtils;
import org.chromium.chrome.browser.offlinepages.downloads.OfflinePageDownloadBridge;
import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper; import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
import org.chromium.chrome.browser.pageinfo.WebsiteSettingsPopup; import org.chromium.chrome.browser.pageinfo.WebsiteSettingsPopup;
import org.chromium.chrome.browser.partnercustomizations.PartnerBrowserCustomizations; import org.chromium.chrome.browser.partnercustomizations.PartnerBrowserCustomizations;
...@@ -1563,13 +1562,8 @@ public abstract class ChromeActivity extends AsyncInitializationActivity ...@@ -1563,13 +1562,8 @@ public abstract class ChromeActivity extends AsyncInitializationActivity
addOrEditBookmark(currentTab); addOrEditBookmark(currentTab);
RecordUserAction.record("MobileMenuAddToBookmarks"); RecordUserAction.record("MobileMenuAddToBookmarks");
} else if (id == R.id.offline_page_id) { } else if (id == R.id.offline_page_id) {
final OfflinePageDownloadBridge bridge = DownloadUtils.downloadOfflinePage(this, currentTab);
new OfflinePageDownloadBridge(currentTab.getProfile());
bridge.startDownload(currentTab);
bridge.destroy();
RecordUserAction.record("MobileMenuDownloadPage"); RecordUserAction.record("MobileMenuDownloadPage");
DownloadUtils.recordDownloadPageMetrics(currentTab);
DownloadUtils.showDownloadStartToast(this);
} else if (id == R.id.reload_menu_id) { } else if (id == R.id.reload_menu_id) {
if (currentTab.isLoading()) { if (currentTab.isLoading()) {
currentTab.stopLoading(); currentTab.stopLoading();
......
...@@ -16,6 +16,7 @@ import org.chromium.chrome.browser.ChromeFeatureList; ...@@ -16,6 +16,7 @@ import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.ShortcutHelper; import org.chromium.chrome.browser.ShortcutHelper;
import org.chromium.chrome.browser.UrlConstants; import org.chromium.chrome.browser.UrlConstants;
import org.chromium.chrome.browser.bookmarks.BookmarkBridge; import org.chromium.chrome.browser.bookmarks.BookmarkBridge;
import org.chromium.chrome.browser.download.DownloadUtils;
import org.chromium.chrome.browser.multiwindow.MultiWindowUtils; import org.chromium.chrome.browser.multiwindow.MultiWindowUtils;
import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper; import org.chromium.chrome.browser.omaha.UpdateMenuItemHelper;
import org.chromium.chrome.browser.preferences.ManagedPreferencesUtils; import org.chromium.chrome.browser.preferences.ManagedPreferencesUtils;
...@@ -110,10 +111,9 @@ public class AppMenuPropertiesDelegate { ...@@ -110,10 +111,9 @@ public class AppMenuPropertiesDelegate {
MenuItem offlineMenuItem = menu.findItem(R.id.offline_page_id); MenuItem offlineMenuItem = menu.findItem(R.id.offline_page_id);
if (offlineMenuItem != null) { if (offlineMenuItem != null) {
if (ChromeFeatureList.isEnabled("DownloadsUi")) { if (ChromeFeatureList.isEnabled("DownloadsUi")) {
boolean isValidTab = !currentTab.isOfflinePage() offlineMenuItem.setEnabled(
&& !currentTab.isShowingErrorPage() DownloadUtils.isAllowedToDownloadPage(currentTab));
&& !currentTab.isShowingInterstitialPage();
offlineMenuItem.setEnabled(!isChromeScheme && !isIncognito && isValidTab);
Drawable drawable = offlineMenuItem.getIcon(); Drawable drawable = offlineMenuItem.getIcon();
if (drawable != null) { if (drawable != null) {
int iconTint = ApiCompatibilityUtils.getColor( int iconTint = ApiCompatibilityUtils.getColor(
......
...@@ -15,6 +15,8 @@ import org.chromium.chrome.browser.ChromeTabbedActivity; ...@@ -15,6 +15,8 @@ import org.chromium.chrome.browser.ChromeTabbedActivity;
import org.chromium.chrome.browser.IntentHandler; import org.chromium.chrome.browser.IntentHandler;
import org.chromium.chrome.browser.UrlConstants; import org.chromium.chrome.browser.UrlConstants;
import org.chromium.chrome.browser.download.ui.BackendProvider; import org.chromium.chrome.browser.download.ui.BackendProvider;
import org.chromium.chrome.browser.download.ui.BackendProvider.DownloadDelegate;
import org.chromium.chrome.browser.offlinepages.downloads.OfflinePageDownloadBridge;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType; import org.chromium.chrome.browser.tabmodel.TabModel.TabLaunchType;
import org.chromium.chrome.browser.tabmodel.document.TabDelegate; import org.chromium.chrome.browser.tabmodel.document.TabDelegate;
...@@ -98,4 +100,39 @@ public class DownloadUtils { ...@@ -98,4 +100,39 @@ public class DownloadUtils {
RecordUserAction.record( RecordUserAction.record(
"Android.DownloadManager.CheckForExternallyRemovedItems"); "Android.DownloadManager.CheckForExternallyRemovedItems");
} }
/**
* Trigger the download of an Offline Page.
* @param context Context to pull resources from.
*/
public static void downloadOfflinePage(Context context, Tab tab) {
final OfflinePageDownloadBridge bridge = new OfflinePageDownloadBridge(tab.getProfile());
bridge.startDownload(tab);
bridge.destroy();
DownloadUtils.recordDownloadPageMetrics(tab);
DownloadUtils.showDownloadStartToast(context);
}
/**
* Whether the user should be allowed to download the current page.
* @param tab Tab displaying the page that will be downloaded.
* @return Whether the "Download Page" button should be enabled.
*/
public static boolean isAllowedToDownloadPage(Tab tab) {
if (tab == null) return false;
// Don't allow downloading internal pages.
if (tab.getUrl().startsWith(UrlConstants.CHROME_SCHEME)) return false;
if (tab.getUrl().startsWith(UrlConstants.CHROME_NATIVE_SCHEME)) return false;
if (tab.isShowingErrorPage()) return false;
if (tab.isShowingInterstitialPage()) return false;
// Don't allow re-downloading the currently displayed offline page.
if (tab.isOfflinePage()) return false;
// Offline pages isn't supported in Incognito.
if (tab.isIncognito()) return false;
return true;
}
} }
...@@ -19,7 +19,7 @@ import android.view.WindowManager; ...@@ -19,7 +19,7 @@ import android.view.WindowManager;
import org.chromium.base.ApiCompatibilityUtils; import org.chromium.base.ApiCompatibilityUtils;
import org.chromium.chrome.R; import org.chromium.chrome.R;
import org.chromium.chrome.browser.ChromeFeatureList; import org.chromium.chrome.browser.ChromeFeatureList;
import org.chromium.chrome.browser.UrlConstants; import org.chromium.chrome.browser.download.DownloadUtils;
import org.chromium.chrome.browser.ntp.NewTabPage; import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.tab.Tab; import org.chromium.chrome.browser.tab.Tab;
import org.chromium.chrome.browser.toolbar.ToolbarTablet; import org.chromium.chrome.browser.toolbar.ToolbarTablet;
...@@ -568,13 +568,7 @@ public class LocationBarTablet extends LocationBarLayout { ...@@ -568,13 +568,7 @@ public class LocationBarTablet extends LocationBarLayout {
private boolean isSaveOfflineButtonEnabled() { private boolean isSaveOfflineButtonEnabled() {
if (mToolbarDataProvider == null) return false; if (mToolbarDataProvider == null) return false;
Tab tab = mToolbarDataProvider.getTab(); return DownloadUtils.isAllowedToDownloadPage(mToolbarDataProvider.getTab());
if (tab == null) return false;
boolean isChromeScheme = tab.getUrl().startsWith(UrlConstants.CHROME_SCHEME)
|| tab.getUrl().startsWith(UrlConstants.CHROME_NATIVE_SCHEME);
boolean isValidTab = !tab.isOfflinePage() && !tab.isShowingErrorPage()
&& !tab.isShowingInterstitialPage();
return !isChromeScheme && isValidTab;
} }
private boolean shouldShowPageActionButtons() { private boolean shouldShowPageActionButtons() {
......
...@@ -23,7 +23,6 @@ import org.chromium.chrome.browser.NavigationPopup; ...@@ -23,7 +23,6 @@ import org.chromium.chrome.browser.NavigationPopup;
import org.chromium.chrome.browser.device.DeviceClassManager; import org.chromium.chrome.browser.device.DeviceClassManager;
import org.chromium.chrome.browser.download.DownloadUtils; import org.chromium.chrome.browser.download.DownloadUtils;
import org.chromium.chrome.browser.ntp.NewTabPage; import org.chromium.chrome.browser.ntp.NewTabPage;
import org.chromium.chrome.browser.offlinepages.downloads.OfflinePageDownloadBridge;
import org.chromium.chrome.browser.omnibox.LocationBar; import org.chromium.chrome.browser.omnibox.LocationBar;
import org.chromium.chrome.browser.omnibox.LocationBarTablet; import org.chromium.chrome.browser.omnibox.LocationBarTablet;
import org.chromium.chrome.browser.partnercustomizations.HomepageManager; import org.chromium.chrome.browser.partnercustomizations.HomepageManager;
...@@ -308,13 +307,8 @@ public class ToolbarTablet extends ToolbarLayout implements OnClickListener { ...@@ -308,13 +307,8 @@ public class ToolbarTablet extends ToolbarLayout implements OnClickListener {
mTabSwitcherListener.onClick(mAccessibilitySwitcherButton); mTabSwitcherListener.onClick(mAccessibilitySwitcherButton);
} }
} else if (mSaveOfflineButton == v) { } else if (mSaveOfflineButton == v) {
Tab tab = getToolbarDataProvider().getTab(); DownloadUtils.downloadOfflinePage(getContext(), getToolbarDataProvider().getTab());
OfflinePageDownloadBridge bridge = new OfflinePageDownloadBridge(tab.getProfile());
bridge.startDownload(tab);
bridge.destroy();
RecordUserAction.record("MobileToolbarDownloadPage"); RecordUserAction.record("MobileToolbarDownloadPage");
DownloadUtils.recordDownloadPageMetrics(tab);
DownloadUtils.showDownloadStartToast(getContext());
} }
} }
......
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