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

Issue 2871533002: [Test] Added a test for activeTab URL filtering (Closed)

Created:
3 years, 7 months ago by Ivan Šandrk
Modified:
3 years, 7 months ago
Reviewers:
Devlin
CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[Test] Added a test for activeTab URL filtering Added a test to make sure that changes introduced in crrev.com/2858643002 to ExtensionActionAPI::DispatchExtensionActionClicked work as intended, ie. that the extension doesn't have access to tab.url unless it has the activeTab permission granted. TEST=This test succeeds with the aforementioned crrev patch, and fails without it. BUG=709513 Review-Url: https://codereview.chromium.org/2871533002 Cr-Commit-Position: refs/heads/master@{#474979} Committed: https://chromium.googlesource.com/chromium/src/+/de08ac6f6973f9771eb912e816d046abc791ac54

Patch Set 1 #

Patch Set 2 : Added a public session test (tests URL scrubbing) #

Total comments: 6

Patch Set 3 : Rebase #

Patch Set 4 : Comments, nits, ScopedTestPublicSessionLoginState #

Unified diffs Side-by-side diffs Delta from patch set Stats (+59 lines, -0 lines) Patch
M chrome/browser/extensions/active_tab_apitest.cc View 1 2 3 3 chunks +39 lines, -0 lines 0 comments Download
M chrome/test/data/extensions/api_test/active_tab/background.js View 1 2 3 1 chunk +20 lines, -0 lines 0 comments Download

Messages

Total messages: 32 (25 generated)
Ivan Šandrk
Hey Devlin, ptal! I promise these reviews will dry up soon :-)
3 years, 7 months ago (2017-05-08 14:54:11 UTC) #6
Devlin
Sorry for the delay on this. If it's okay, I'd like to wait 'til we're ...
3 years, 7 months ago (2017-05-15 20:56:27 UTC) #20
Devlin
https://codereview.chromium.org/2871533002/diff/60001/chrome/test/data/extensions/api_test/active_tab/background.js File chrome/test/data/extensions/api_test/active_tab/background.js (right): https://codereview.chromium.org/2871533002/diff/60001/chrome/test/data/extensions/api_test/active_tab/background.js#newcode49 chrome/test/data/extensions/api_test/active_tab/background.js:49: if (runCount == 1) { add some comments explaining ...
3 years, 7 months ago (2017-05-23 20:49:21 UTC) #21
Ivan Šandrk
https://codereview.chromium.org/2871533002/diff/60001/chrome/test/data/extensions/api_test/active_tab/background.js File chrome/test/data/extensions/api_test/active_tab/background.js (right): https://codereview.chromium.org/2871533002/diff/60001/chrome/test/data/extensions/api_test/active_tab/background.js#newcode49 chrome/test/data/extensions/api_test/active_tab/background.js:49: if (runCount == 1) { On 2017/05/23 20:49:21, Devlin ...
3 years, 7 months ago (2017-05-24 16:36:51 UTC) #24
Devlin
lgtm
3 years, 7 months ago (2017-05-24 21:22:00 UTC) #27
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/2871533002/100001
3 years, 7 months ago (2017-05-26 09:21:24 UTC) #29
commit-bot: I haz the power
3 years, 7 months ago (2017-05-26 10:33:50 UTC) #32
Message was sent while issue was closed.
Committed patchset #4 (id:100001) as
https://chromium.googlesource.com/chromium/src/+/de08ac6f6973f9771eb912e816d0...

Powered by Google App Engine
This is Rietveld 408576698