Chromium Code Reviews
Help | Chromium Project | Sign in
(1)

Issue 2805133004: Local NTP: Deploy strict-dynamic CSP

Can't Edit
Can't Publish+Mail
Start Review
Created:
2 weeks, 3 days ago by Marc Treib (OOO till April 25)
Modified:
1 week, 4 days ago
Reviewers:
sfiera
CC:
chromium-reviews, skanuj+watch_chromium.org, melevin+watch_chromium.org, donnd+watch_chromium.org, jfweitz+watch_chromium.org, David Black, arv+watch_chromium.org, samarth+watch_chromium.org, kmadhusu+watch_chromium.org, Jered
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Local NTP: Deploy strict-dynamic CSP BUG=704518 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Patch Set 1 #

Total comments: 22

Patch Set 2 : review #

Patch Set 3 : shave ALL the yaks! #

Patch Set 4 : fix #

Total comments: 9

Patch Set 5 : review #

Patch Set 6 : remove GoogleSearchProviderService #

Patch Set 7 : . #

Patch Set 8 : rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+335 lines, -60 lines) Patch
M chrome/browser/resources/local_ntp/local_ntp.html View 1 2 1 chunk +4 lines, -2 lines 0 comments Download
M chrome/browser/resources/local_ntp/local_ntp.js View 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/browser/resources/local_ntp/most_visited_single.js View 1 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/search/local_ntp_source.h View 1 2 3 4 5 3 chunks +17 lines, -0 lines 0 comments Download
M chrome/browser/search/local_ntp_source.cc View 1 2 3 4 5 6 12 chunks +151 lines, -27 lines 0 comments Download
M chrome/browser/ui/search/local_ntp_browsertest.cc View 1 5 chunks +137 lines, -3 lines 0 comments Download
M chrome/browser/ui/search/new_tab_page_interceptor_browsertest.cc View 1 2 chunks +20 lines, -27 lines 0 comments Download
Commit queue not available (can’t edit this change).

Depends on Patchset:

Dependent Patchsets:

Messages

Total messages: 43 (28 generated)
Marc Treib (OOO till April 25)
PTAL! By itself, this doesn't do anything useful, but it's a prerequisite for Doodle and ...
2 weeks, 3 days ago (2017-04-07 16:46:15 UTC) #5
sfiera
https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js File chrome/browser/resources/local_ntp/local_ntp.js (right): https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js#newcode7 chrome/browser/resources/local_ntp/local_ntp.js:7: // local_ntp.html and in LocalNtpSource::GetContentSecurityPolicyScriptSrc. On 2017/04/07 16:46:15, Marc ...
2 weeks ago (2017-04-10 08:53:22 UTC) #8
Marc Treib (OOO till April 25)
Comments addressed. NewTabPageInterceptorTest is still failing; I'll fix that soon. https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js File chrome/browser/resources/local_ntp/local_ntp.js (right): https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js#newcode7 ...
2 weeks ago (2017-04-10 10:11:18 UTC) #9
sfiera
https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js File chrome/browser/resources/local_ntp/local_ntp.js (right): https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js#newcode7 chrome/browser/resources/local_ntp/local_ntp.js:7: // local_ntp.html and in LocalNtpSource::GetContentSecurityPolicyScriptSrc. On 2017/04/10 10:11:18, Marc ...
2 weeks ago (2017-04-10 10:28:54 UTC) #10
Marc Treib (OOO till April 25)
https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js File chrome/browser/resources/local_ntp/local_ntp.js (right): https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js#newcode7 chrome/browser/resources/local_ntp/local_ntp.js:7: // local_ntp.html and in LocalNtpSource::GetContentSecurityPolicyScriptSrc. On 2017/04/10 10:28:53, sfiera ...
2 weeks ago (2017-04-10 11:44:03 UTC) #11
sfiera
LGTM https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js File chrome/browser/resources/local_ntp/local_ntp.js (right): https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js#newcode7 chrome/browser/resources/local_ntp/local_ntp.js:7: // local_ntp.html and in LocalNtpSource::GetContentSecurityPolicyScriptSrc. On 2017/04/10 11:44:03, ...
2 weeks ago (2017-04-10 12:08:34 UTC) #12
Marc Treib (OOO till April 25)
https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js File chrome/browser/resources/local_ntp/local_ntp.js (right): https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js#newcode7 chrome/browser/resources/local_ntp/local_ntp.js:7: // local_ntp.html and in LocalNtpSource::GetContentSecurityPolicyScriptSrc. On 2017/04/10 12:08:34, sfiera ...
2 weeks ago (2017-04-10 12:16:18 UTC) #13
sfiera
On 2017/04/10 12:16:18, Marc Treib wrote: > https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js > File chrome/browser/resources/local_ntp/local_ntp.js (right): > > https://codereview.chromium.org/2805133004/diff/1/chrome/browser/resources/local_ntp/local_ntp.js#newcode7 ...
2 weeks ago (2017-04-10 12:22:51 UTC) #14
Marc Treib (OOO till April 25)
PTAL again! The good news is that those failing tests pointed at an actual problem. ...
2 weeks ago (2017-04-10 15:29:45 UTC) #19
sfiera
https://codereview.chromium.org/2805133004/diff/60001/chrome/browser/search/google_search_provider_service.h File chrome/browser/search/google_search_provider_service.h (right): https://codereview.chromium.org/2805133004/diff/60001/chrome/browser/search/google_search_provider_service.h#newcode17 chrome/browser/search/google_search_provider_service.h:17: // Tracks whether the default search provider is Google. ...
1 week, 6 days ago (2017-04-11 09:45:17 UTC) #22
Marc Treib (OOO till April 25)
Sorry for the missing tests and comments! I guess I was too happy that I ...
1 week, 6 days ago (2017-04-11 12:37:59 UTC) #25
sfiera
LGTM https://codereview.chromium.org/2805133004/diff/60001/chrome/browser/search/google_search_provider_service.h File chrome/browser/search/google_search_provider_service.h (right): https://codereview.chromium.org/2805133004/diff/60001/chrome/browser/search/google_search_provider_service.h#newcode17 chrome/browser/search/google_search_provider_service.h:17: // Tracks whether the default search provider is ...
1 week, 6 days ago (2017-04-11 14:07:35 UTC) #28
Marc Treib (OOO till April 25)
https://codereview.chromium.org/2805133004/diff/60001/chrome/browser/search/google_search_provider_service.h File chrome/browser/search/google_search_provider_service.h (right): https://codereview.chromium.org/2805133004/diff/60001/chrome/browser/search/google_search_provider_service.h#newcode17 chrome/browser/search/google_search_provider_service.h:17: // Tracks whether the default search provider is Google. ...
1 week, 5 days ago (2017-04-12 11:53:46 UTC) #31
Marc Treib (OOO till April 25)
All ready now! Do you want to take a last look at the diff between ...
1 week, 4 days ago (2017-04-13 08:30:15 UTC) #40
sfiera
1 week, 4 days ago (2017-04-13 08:53:30 UTC) #41
LGTM
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld cc6ac46