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

Issue 8800006: Support chrome-extension:// scheme in URLPattern. (Closed)

Created:
7 years, 9 months ago by battre
Modified:
7 years, 9 months ago
Reviewers:
palmer, Aaron Boodman, security, Matt Perry
CC:
chromium-reviews, jstritar+watch_chromium.org, Erik does not do reviews, jam, mihaip+watch_chromium.org, dpranke-watch+content_chromium.org, joi+watch-content_chromium.org, darin-cc_chromium.org, Paweł Hajdan Jr.
Visibility:
Public.

Description

Support chrome-extension:// scheme in URLPattern. This CL is mostly motivated to allow users of the web request API to filter network traffic to the extension. The URLParser was configured to broadly before in that is allowed parsing URLPattern::SCHEME_ALL. BUG=105656 TEST=no Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=113907

Patch Set 1 #

Total comments: 3

Patch Set 2 : Addressed jam's comments #

Total comments: 6

Patch Set 3 : Addressed Aaron's comments #

Patch Set 4 : Limit subscription to http, https, ftp, file, chrome-extension #

Patch Set 5 : Limit subscription to http, https, ftp, file, chrome-extension #

Patch Set 6 : Improved documentation #

Total comments: 4

Patch Set 7 : Addressed Aaron's comments #

Patch Set 8 : Merged with ToT #

Patch Set 9 : Fixed merge issues in unit tests #

Unified diffs Side-by-side diffs Delta from patch set Stats (+91 lines, -16 lines) Patch
M chrome/browser/extensions/extension_tabs_module.cc View 1 2 3 4 5 1 chunk +3 lines, -0 lines 0 comments Download
M chrome/browser/extensions/extension_webrequest_api.cc View 1 2 3 4 5 6 7 1 chunk +5 lines, -1 line 0 comments Download
M chrome/common/extensions/docs/match_patterns.html View 1 2 4 chunks +22 lines, -2 lines 0 comments Download
M chrome/common/extensions/docs/static/match_patterns.html View 1 2 4 chunks +22 lines, -2 lines 0 comments Download
M chrome/common/extensions/docs/static/webRequest.html View 1 2 1 chunk +2 lines, -1 line 0 comments Download
M chrome/common/extensions/docs/webRequest.html View 1 2 3 4 5 6 7 1 chunk +2 lines, -1 line 0 comments Download
M chrome/common/extensions/url_pattern.h View 1 2 3 4 5 6 7 2 chunks +9 lines, -5 lines 0 comments Download
M chrome/common/extensions/url_pattern.cc View 1 2 3 4 5 6 7 3 chunks +3 lines, -2 lines 0 comments Download
M chrome/common/extensions/url_pattern_unittest.cc View 1 2 3 4 5 6 7 8 4 chunks +23 lines, -2 lines 0 comments Download

Messages

Total messages: 25 (0 generated)
battre
aa: main reviewer because you expressed interest in this in url_pattern.cc security: for request to ...
7 years, 9 months ago (2011-12-05 14:23:07 UTC) #1
jam
http://codereview.chromium.org/8800006/diff/1/content/public/common/url_constants.h File content/public/common/url_constants.h (right): http://codereview.chromium.org/8800006/diff/1/content/public/common/url_constants.h#newcode27 content/public/common/url_constants.h:27: CONTENT_EXPORT extern const char kChromeExtensionScheme[]; this doesn't belong in ...
7 years, 9 months ago (2011-12-05 17:48:24 UTC) #2
battre
Thanks, John. Moving you to CC. http://codereview.chromium.org/8800006/diff/1/content/public/common/url_constants.h File content/public/common/url_constants.h (right): http://codereview.chromium.org/8800006/diff/1/content/public/common/url_constants.h#newcode27 content/public/common/url_constants.h:27: CONTENT_EXPORT extern const ...
7 years, 9 months ago (2011-12-05 18:36:43 UTC) #3
Matt Perry
We should be careful to only allow this for the webRequest API. We don't want ...
7 years, 9 months ago (2011-12-05 20:26:50 UTC) #4
palmer
If I am installing extension B, and it wants permission to intercept traffic for extension ...
7 years, 9 months ago (2011-12-05 21:33:58 UTC) #5
Matt Perry
On 2011/12/05 21:33:58, Chris P. wrote: > If I am installing extension B, and it ...
7 years, 9 months ago (2011-12-05 22:49:31 UTC) #6
Aaron Boodman
Agree with Matt's comment, but I think this CL won't have any effect at all ...
7 years, 9 months ago (2011-12-06 06:40:31 UTC) #7
Aaron Boodman
For the future. http://codereview.chromium.org/8800006/diff/4001/chrome/common/extensions/docs/static/webRequest.html File chrome/common/extensions/docs/static/webRequest.html (right): http://codereview.chromium.org/8800006/diff/4001/chrome/common/extensions/docs/static/webRequest.html#newcode199 chrome/common/extensions/docs/static/webRequest.html:199: <code>*://www.google.com/foo*bar</code>, <code>chrome-extension://*/*</code>. I don't think that ...
7 years, 9 months ago (2011-12-06 06:43:22 UTC) #8
battre
On 2011/12/05 20:26:50, Matt Perry wrote: > We should be careful to only allow this ...
7 years, 9 months ago (2011-12-06 12:14:15 UTC) #9
battre
On 2011/12/06 06:40:31, Aaron Boodman wrote: > Agree with Matt's comment, but I think this ...
7 years, 9 months ago (2011-12-06 12:45:16 UTC) #10
battre
http://codereview.chromium.org/8800006/diff/4001/chrome/common/extensions/docs/static/webRequest.html File chrome/common/extensions/docs/static/webRequest.html (right): http://codereview.chromium.org/8800006/diff/4001/chrome/common/extensions/docs/static/webRequest.html#newcode199 chrome/common/extensions/docs/static/webRequest.html:199: <code>*://www.google.com/foo*bar</code>, <code>chrome-extension://*/*</code>. On 2011/12/06 06:43:23, Aaron Boodman wrote: > ...
7 years, 9 months ago (2011-12-06 12:51:35 UTC) #11
Matt Perry
Actually, thinking about this some more, I don't think this change does anything. We currently ...
7 years, 9 months ago (2011-12-06 20:28:01 UTC) #12
Aaron Boodman
On Tue, Dec 6, 2011 at 4:14 AM, <battre@chromium.org> wrote: > On 2011/12/05 20:26:50, Matt ...
7 years, 9 months ago (2011-12-06 20:31:17 UTC) #13
battre
On 2011/12/06 20:31:17, Aaron Boodman wrote: > On Tue, Dec 6, 2011 at 4:14 AM, ...
7 years, 9 months ago (2011-12-09 14:14:53 UTC) #14
Aaron Boodman
I think we need kathy's help with the docs. I see you have a separate ...
7 years, 9 months ago (2011-12-09 16:28:25 UTC) #15
Aaron Boodman
LGTM w/ above. No need for another review from me.
7 years, 9 months ago (2011-12-09 16:28:46 UTC) #16
battre
http://codereview.chromium.org/8800006/diff/22001/chrome/common/extensions/docs/static/manifest.html File chrome/common/extensions/docs/static/manifest.html (right): http://codereview.chromium.org/8800006/diff/22001/chrome/common/extensions/docs/static/manifest.html#newcode435 chrome/common/extensions/docs/static/manifest.html:435: the chrome-extension:// scheme in match patterns for host permissions. ...
7 years, 9 months ago (2011-12-09 17:07:51 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/battre@chromium.org/8800006/22014
7 years, 9 months ago (2011-12-09 17:16:13 UTC) #18
commit-bot: I haz the power
Try job failure for 8800006-22014 (retry) on linux_rel for step "compile" (clobber build). It's a ...
7 years, 9 months ago (2011-12-09 17:50:58 UTC) #19
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/battre@chromium.org/8800006/25012
7 years, 9 months ago (2011-12-09 18:00:10 UTC) #20
commit-bot: I haz the power
Try job failure for 8800006-25012 (retry) on mac_rel for step "browser_tests". It's a second try, ...
7 years, 9 months ago (2011-12-09 19:51:46 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/battre@chromium.org/8800006/25012
7 years, 9 months ago (2011-12-09 20:18:34 UTC) #22
commit-bot: I haz the power
Try job failure for 8800006-25012 (retry) on win_rel for step "ui_tests". It's a second try, ...
7 years, 9 months ago (2011-12-09 23:18:46 UTC) #23
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/battre@chromium.org/8800006/25012
7 years, 9 months ago (2011-12-09 23:24:07 UTC) #24
commit-bot: I haz the power
7 years, 9 months ago (2011-12-10 01:01:09 UTC) #25
Change committed as 113907

Powered by Google App Engine
This is Rietveld 408576698