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

Issue 1750113002: Remove "Show saved copy" button from error page (Closed)

Created:
4 years, 9 months ago by jianli
Modified:
4 years, 9 months ago
Reviewers:
mmenke, nasko, newt (away)
CC:
chromium-reviews, cbentzel+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Remove "Show saved copy" button from error page This is not longer needed after we redirect to offline copy when an online version fails to load. BUG=590886 Committed: https://crrev.com/9ea0b744e361b07bac102f1ae24e413effa8a7fc Cr-Commit-Position: refs/heads/master@{#378645}

Patch Set 1 #

Total comments: 6

Patch Set 2 : Address feedback #

Unified diffs Side-by-side diffs Delta from patch set Stats (+71 lines, -252 lines) Patch
M chrome/browser/android/tab_android.h View 2 chunks +0 lines, -3 lines 0 comments Download
M chrome/browser/android/tab_android.cc View 2 chunks +0 lines, -14 lines 0 comments Download
M chrome/browser/net/net_error_tab_helper.h View 1 chunk +1 line, -3 lines 0 comments Download
M chrome/browser/net/net_error_tab_helper.cc View 6 chunks +6 lines, -31 lines 0 comments Download
M chrome/common/render_messages.h View 1 4 chunks +3 lines, -11 lines 0 comments Download
M chrome/renderer/net/net_error_helper.h View 4 chunks +7 lines, -11 lines 0 comments Download
M chrome/renderer/net/net_error_helper.cc View 9 chunks +7 lines, -19 lines 0 comments Download
M chrome/renderer/net/net_error_page_controller.h View 1 chunk +0 lines, -3 lines 0 comments Download
M chrome/renderer/net/net_error_page_controller.cc View 2 chunks +0 lines, -6 lines 0 comments Download
M components/error_page.gypi View 1 chunk +0 lines, -1 line 0 comments Download
M components/error_page/common/BUILD.gn View 1 chunk +0 lines, -1 line 0 comments Download
M components/error_page/common/localized_error.h View 2 chunks +1 line, -2 lines 0 comments Download
M components/error_page/common/localized_error.cc View 2 chunks +11 lines, -21 lines 0 comments Download
M components/error_page/common/net_error_info.h View 1 1 chunk +3 lines, -2 lines 0 comments Download
D components/error_page/common/offline_page_types.h View 1 chunk +0 lines, -24 lines 0 comments Download
M components/error_page/renderer/net_error_helper_core.h View 1 8 chunks +9 lines, -17 lines 0 comments Download
M components/error_page/renderer/net_error_helper_core.cc View 11 chunks +11 lines, -26 lines 0 comments Download
M components/error_page/renderer/net_error_helper_core_unittest.cc View 9 chunks +9 lines, -32 lines 0 comments Download
M components/error_page_strings.grdp View 1 chunk +0 lines, -3 lines 0 comments Download
M components/neterror/resources/neterror.html View 1 chunk +0 lines, -6 lines 0 comments Download
M components/neterror/resources/neterror.js View 4 chunks +3 lines, -16 lines 0 comments Download

Messages

Total messages: 16 (6 generated)
jianli
mmenke for error pages newt for android nasko for render_messages
4 years, 9 months ago (2016-03-01 00:53:23 UTC) #2
mmenke
components/error_page LGTM, modulo comments. https://codereview.chromium.org/1750113002/diff/1/components/error_page/common/net_error_info.h File components/error_page/common/net_error_info.h (right): https://codereview.chromium.org/1750113002/diff/1/components/error_page/common/net_error_info.h#newcode49 components/error_page/common/net_error_info.h:49: NETWORK_ERROR_PAGE_SHOW_OFFLINE_COPY_BUTTON_SHOWN = 20, I think ...
4 years, 9 months ago (2016-03-01 20:11:21 UTC) #3
nasko
IPC LGTM with a comment. https://codereview.chromium.org/1750113002/diff/1/chrome/common/render_messages.h File chrome/common/render_messages.h (right): https://codereview.chromium.org/1750113002/diff/1/chrome/common/render_messages.h#newcode320 chrome/common/render_messages.h:320: // decide if offline ...
4 years, 9 months ago (2016-03-01 22:23:10 UTC) #4
jianli
https://codereview.chromium.org/1750113002/diff/1/chrome/common/render_messages.h File chrome/common/render_messages.h (right): https://codereview.chromium.org/1750113002/diff/1/chrome/common/render_messages.h#newcode320 chrome/common/render_messages.h:320: // decide if offline related button will be provided ...
4 years, 9 months ago (2016-03-01 22:53:16 UTC) #5
newt (away)
tab_android lgtm
4 years, 9 months ago (2016-03-01 23:30:04 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1750113002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1750113002/20001
4 years, 9 months ago (2016-03-01 23:43:58 UTC) #9
commit-bot: I haz the power
Try jobs failed on following builders: win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_ng/builds/182381)
4 years, 9 months ago (2016-03-02 00:30:26 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1750113002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1750113002/20001
4 years, 9 months ago (2016-03-02 00:32:33 UTC) #13
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 9 months ago (2016-03-02 01:19:56 UTC) #14
commit-bot: I haz the power
4 years, 9 months ago (2016-03-02 01:21:12 UTC) #16
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/9ea0b744e361b07bac102f1ae24e413effa8a7fc
Cr-Commit-Position: refs/heads/master@{#378645}

Powered by Google App Engine
This is Rietveld 408576698