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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10224011: Add transparent support for filesystem URLs in URLPatterns. (Closed)

Created:
8 years, 8 months ago by ericu
Modified:
8 years, 7 months ago
Reviewers:
Aaron Boodman
CC:
chromium-reviews, Aaron Boodman, mihaip+watch_chromium.org
Visibility:
Public.

Description

Add transparent support for filesystem URLs in URLPatterns. Given that we don't want to add in filesystem-specific patterns, the simplest approach seems to be just to allow all patterns to match either outer or inner URLs. BUG=none TEST=unit tests Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=135049

Patch Set 1 #

Patch Set 2 : Remove a redundant line from a test. #

Total comments: 2

Patch Set 3 : Restrict inner_url handling to filesystem URLs. #

Total comments: 1

Patch Set 4 : Updated MatchesSecurityOrigin and added a comment, as discussed in IM. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+21 lines, -40 lines) Patch
M chrome/common/extensions/extension.cc View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/common/extensions/url_pattern.h View 1 2 3 3 chunks +3 lines, -14 lines 0 comments Download
M chrome/common/extensions/url_pattern.cc View 1 2 3 5 chunks +15 lines, -13 lines 0 comments Download
M chrome/common/extensions/url_pattern_unittest.cc View 1 4 chunks +3 lines, -12 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
ericu
8 years, 8 months ago (2012-04-25 22:35:41 UTC) #1
ericu
Ping. It would be nice to have this in for M20 along with the rest ...
8 years, 7 months ago (2012-04-30 22:21:51 UTC) #2
Aaron Boodman
http://codereview.chromium.org/10224011/diff/5002/chrome/common/extensions/url_pattern.cc File chrome/common/extensions/url_pattern.cc (right): http://codereview.chromium.org/10224011/diff/5002/chrome/common/extensions/url_pattern.cc#newcode304 chrome/common/extensions/url_pattern.cc:304: if (!MatchesScheme(test_url->scheme())) Perhaps this only makes sense for the ...
8 years, 7 months ago (2012-05-02 04:04:05 UTC) #3
ericu
http://codereview.chromium.org/10224011/diff/5002/chrome/common/extensions/url_pattern.cc File chrome/common/extensions/url_pattern.cc (right): http://codereview.chromium.org/10224011/diff/5002/chrome/common/extensions/url_pattern.cc#newcode304 chrome/common/extensions/url_pattern.cc:304: if (!MatchesScheme(test_url->scheme())) On 2012/05/02 04:04:05, Aaron Boodman wrote: > ...
8 years, 7 months ago (2012-05-02 16:51:16 UTC) #4
Aaron Boodman
lgtm http://codereview.chromium.org/10224011/diff/11002/chrome/common/extensions/url_pattern.cc File chrome/common/extensions/url_pattern.cc (right): http://codereview.chromium.org/10224011/diff/11002/chrome/common/extensions/url_pattern.cc#newcode302 chrome/common/extensions/url_pattern.cc:302: if (!test.SchemeIsFileSystem()) Can this go in MatchesScheme instead, ...
8 years, 7 months ago (2012-05-02 20:03:08 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ericu@chromium.org/10224011/18002
8 years, 7 months ago (2012-05-02 22:50:52 UTC) #6
ericu
On 2012/05/02 20:03:08, Aaron Boodman wrote: > lgtm > > http://codereview.chromium.org/10224011/diff/11002/chrome/common/extensions/url_pattern.cc > File chrome/common/extensions/url_pattern.cc (right): ...
8 years, 7 months ago (2012-05-02 22:53:00 UTC) #7
commit-bot: I haz the power
8 years, 7 months ago (2012-05-03 00:26:43 UTC) #8 
Change committed as 135049

Powered by Google App Engine
This is Rietveld 408576698