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

Issue 2433793003: Generalize Previews Type code (Closed)

Created:
4 years, 2 months ago by RyanSturm
Modified:
4 years, 2 months ago
Reviewers:
Dmitry Titov, tbansal1
CC:
chromium-reviews, romax+watch_chromium.org, fgorski+watch_chromium.org, dewittj+watch_chromium.org, petewil+watch_chromium.org, chili+watch_chromium.org, dimich+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Generalize Previews Type code This moves PreviewType to previews_experiments.h and chagnes IsOfflinePreviewsEnabled to IsPreviewsTypeEnabled to allow other previews to use the code in a similar fashion. BUG=615564 TBR=dimich Committed: https://crrev.com/72a715afdc50e013d0c96a2042ecb3e86b428d08 Cr-Commit-Position: refs/heads/master@{#426932}

Patch Set 1 #

Patch Set 2 : typo #

Total comments: 1

Patch Set 3 : tbansal comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+29 lines, -18 lines) Patch
M chrome/browser/android/offline_pages/offline_page_request_job.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/android/offline_pages/offline_page_request_job_unittest.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/previews/core/previews_black_list.h View 1 chunk +1 line, -0 lines 0 comments Download
M components/previews/core/previews_decider.h View 1 chunk +1 line, -1 line 0 comments Download
M components/previews/core/previews_experiments.h View 1 chunk +8 lines, -2 lines 0 comments Download
M components/previews/core/previews_experiments.cc View 1 2 2 chunks +9 lines, -2 lines 0 comments Download
M components/previews/core/previews_experiments_unittest.cc View 1 1 chunk +2 lines, -2 lines 0 comments Download
M components/previews/core/previews_io_data.h View 1 chunk +2 lines, -1 line 0 comments Download
M components/previews/core/previews_io_data.cc View 2 chunks +1 line, -2 lines 0 comments Download
M components/previews/core/previews_opt_out_store.h View 1 chunk +1 line, -6 lines 0 comments Download
M components/previews/core/previews_opt_out_store_sql.h View 1 chunk +1 line, -0 lines 0 comments Download
M components/previews/core/previews_ui_service.h View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 31 (24 generated)
RyanSturm
tbansal: PTAL
4 years, 2 months ago (2016-10-19 23:31:11 UTC) #15
tbansal1
lgtm % nit. https://codereview.chromium.org/2433793003/diff/40001/components/previews/core/previews_experiments.cc File components/previews/core/previews_experiments.cc (right): https://codereview.chromium.org/2433793003/diff/40001/components/previews/core/previews_experiments.cc#newcode118 components/previews/core/previews_experiments.cc:118: return false; Add NOTREACHED() in default?
4 years, 2 months ago (2016-10-21 16:17:20 UTC) #17
RyanSturm
dimich: PTAL at the header change, TBR'd because there is no functional change.
4 years, 2 months ago (2016-10-21 22:11:23 UTC) #21
RyanSturm
4 years, 2 months ago (2016-10-21 22:11:37 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2433793003/60001
4 years, 2 months ago (2016-10-21 22:12:24 UTC) #27
commit-bot: I haz the power
Committed patchset #3 (id:60001)
4 years, 2 months ago (2016-10-22 00:12:10 UTC) #29
commit-bot: I haz the power
4 years, 2 months ago (2016-10-22 00:15:14 UTC) #31
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/72a715afdc50e013d0c96a2042ecb3e86b428d08
Cr-Commit-Position: refs/heads/master@{#426932}

Powered by Google App Engine
This is Rietveld 408576698