Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(59)

Issue 2978953002: [Extensions] Don't allow content scripts on the New Tab Page (Closed)

Created:
2 years, 12 months ago by Devlin
Modified:
2 years, 10 months ago
Reviewers:
karandeepb
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org, lazyboy
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Extensions] Don't allow content scripts on the New Tab Page Extensions wishing to modify the New Tab Page should do so through the use of the chrome url overrides [1] API, not through content scripts. Disallow content script injection on the New Tab Page. [1] https://developer.chrome.com/extensions/override BUG=662610 Review-Url: https://codereview.chromium.org/2978953002 Cr-Commit-Position: refs/heads/master@{#487664} Committed: https://chromium.googlesource.com/chromium/src/+/993682e9e2b181a14f9fd9e6a7f848e401a551e7

Patch Set 1 : . #

Total comments: 20

Patch Set 2 : Karan's #

Total comments: 2

Patch Set 3 : Remove logging! #

Unified diffs Side-by-side diffs Delta from patch set Stats (+94 lines, -0 lines) Patch
M chrome/browser/extensions/content_script_apitest.cc View 1 2 chunks +41 lines, -0 lines 0 comments Download
M chrome/renderer/extensions/renderer_permissions_policy_delegate.cc View 2 chunks +7 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/api_test/content_scripts/ntp/background.js View 1 chunk +26 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/api_test/content_scripts/ntp/manifest.json View 1 1 chunk +13 lines, -0 lines 0 comments Download
A chrome/test/data/extensions/api_test/content_scripts/ntp/script.js View 1 1 chunk +5 lines, -0 lines 0 comments Download
M extensions/common/manifest_constants.h View 1 chunk +1 line, -0 lines 0 comments Download
M extensions/common/manifest_constants.cc View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 34 (23 generated)
Devlin
Karan, mind taking a look?
2 years, 12 months ago (2017-07-14 17:54:07 UTC) #17
karandeepb
https://codereview.chromium.org/2978953002/diff/40001/chrome/browser/extensions/api/tabs/tabs_api.cc File chrome/browser/extensions/api/tabs/tabs_api.cc (right): https://codereview.chromium.org/2978953002/diff/40001/chrome/browser/extensions/api/tabs/tabs_api.cc#newcode1825 chrome/browser/extensions/api/tabs/tabs_api.cc:1825: Do we also need to update extension docs for ...
2 years, 11 months ago (2017-07-18 19:27:00 UTC) #18
Devlin
https://codereview.chromium.org/2978953002/diff/40001/chrome/browser/extensions/api/tabs/tabs_api.cc File chrome/browser/extensions/api/tabs/tabs_api.cc (right): https://codereview.chromium.org/2978953002/diff/40001/chrome/browser/extensions/api/tabs/tabs_api.cc#newcode1825 chrome/browser/extensions/api/tabs/tabs_api.cc:1825: On 2017/07/18 19:26:59, karandeepb wrote: > Do we also ...
2 years, 11 months ago (2017-07-18 20:53:46 UTC) #19
karandeepb
LGTM. But yeah, remove logging. https://codereview.chromium.org/2978953002/diff/40001/chrome/browser/extensions/api/tabs/tabs_api.cc File chrome/browser/extensions/api/tabs/tabs_api.cc (right): https://codereview.chromium.org/2978953002/diff/40001/chrome/browser/extensions/api/tabs/tabs_api.cc#newcode1825 chrome/browser/extensions/api/tabs/tabs_api.cc:1825: On 2017/07/18 20:53:46, Devlin ...
2 years, 11 months ago (2017-07-18 21:06:26 UTC) #20
Devlin
https://codereview.chromium.org/2978953002/diff/40001/chrome/test/data/extensions/api_test/content_scripts/ntp/manifest.json File chrome/test/data/extensions/api_test/content_scripts/ntp/manifest.json (right): https://codereview.chromium.org/2978953002/diff/40001/chrome/test/data/extensions/api_test/content_scripts/ntp/manifest.json#newcode9 chrome/test/data/extensions/api_test/content_scripts/ntp/manifest.json:9: "run_at": "document_start" On 2017/07/18 21:06:25, karandeepb wrote: > For ...
2 years, 11 months ago (2017-07-18 21:12:49 UTC) #23
karandeepb
https://codereview.chromium.org/2978953002/diff/40001/chrome/test/data/extensions/api_test/content_scripts/ntp/manifest.json File chrome/test/data/extensions/api_test/content_scripts/ntp/manifest.json (right): https://codereview.chromium.org/2978953002/diff/40001/chrome/test/data/extensions/api_test/content_scripts/ntp/manifest.json#newcode9 chrome/test/data/extensions/api_test/content_scripts/ntp/manifest.json:9: "run_at": "document_start" On 2017/07/18 21:12:49, Devlin wrote: > On ...
2 years, 11 months ago (2017-07-18 21:27:20 UTC) #24
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/2978953002/80001
2 years, 11 months ago (2017-07-18 23:02:09 UTC) #28
commit-bot: I haz the power
Committed patchset #3 (id:80001) as https://chromium.googlesource.com/chromium/src/+/993682e9e2b181a14f9fd9e6a7f848e401a551e7
2 years, 11 months ago (2017-07-18 23:26:03 UTC) #31
wOxxOm
Why the sudden change after many years of allowing content scripts in the iframe of ...
2 years, 11 months ago (2017-07-22 14:27:54 UTC) #32
Devlin
On 2017/07/22 14:27:54, wOxxOm wrote: > Why the sudden change after many years of allowing ...
2 years, 11 months ago (2017-07-25 14:22:57 UTC) #33
conrad
2 years, 10 months ago (2017-09-08 18:18:18 UTC) #34
Message was sent while issue was closed.
> I'll see if I can't post an announcement in chromium-extensions@ with this
> announcement (even though the policy's been published for awhile now, a little
> extra exposure couldn't hurt).

Please do post this — I just spent a bunch of time debugging why our extension
was no longer adding a link to the new tab page, and I'd have seen it if you
posted to the list :).

(As it was I had to find this commit in the git log, because
https://bugs.chromium.org/p/chromium/issues/detail?id=662610 cannot be viewed).

Conrad

Powered by Google App Engine
This is Rietveld 408576698