Don't check main frame requests through shields network delegate helper #8402
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
antonok-edm
requested review from
fmarier,
diracdeltas,
bbondy and
pilgrim-brave
fmarier
approved these changes
Mar 31, 2021
bbondy
approved these changes
Mar 31, 2021
iefremov
reviewed
Apr 1, 2021
iefremov
reviewed
Apr 1, 2021
| "domAutomationController.send(clickLink('" + cross_url.spec() + "'));"; | ||
| bool success = false; | ||
| EXPECT_TRUE( | ||
| ExecuteScriptAndExtractBool(web_contents(), clickLink.c_str(), &success)); |
There was a problem hiding this comment.
maybe rewrite this with EvalJs?
There was a problem hiding this comment.
good call, updated
iefremov
reviewed
Apr 1, 2021
| // helper. | ||
| const std::string location = | ||
| EvalJs(web_contents(), "window.location.href").ExtractString(); | ||
| ASSERT_STRNE("chrome-error://chromewebdata/", location.c_str()); |
There was a problem hiding this comment.
I think we do need it, when I remove it I get this:
../../third_party/googletest/src/googletest/include/gtest/gtest.h:1669:28: note: candidate function not viable: no known conversion from 'const std::string' (aka 'const basic_string<char>') to 'const char *' for 4th argument
GTEST_API_ AssertionResult CmpHelperSTRNE(const char* s1_expression,
^
../../third_party/googletest/src/googletest/include/gtest/gtest.h:1694:28: note: candidate function not viable: no known conversion from 'const char [30]' to 'const wchar_t *' for 3rd argument
GTEST_API_ AssertionResult CmpHelperSTRNE(const char* s1_expression,
iefremov
approved these changes
Apr 1, 2021
antonok-edm
force-pushed
the
no-more-blocked-by-extension
branch
from
7d51927 to
a49f954
Compare
brave-builds
pushed a commit
that referenced
this pull request
Apr 1, 2021
Merged
7 tasks
|
Verification PASSED on Test Cases from #8402 (comment):
Visited https://1-1ads.com and ensured that Brave brings up a "Suspicious site ahead" interstitial with a "Proceed" button as per the following:
Test Cases from brave/brave-browser#15047 (comment): Ensured that selecting the
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves brave/brave-browser#15047
Submitter Checklist:
QA/YesorQA/No;release-notes/includeorrelease-notes/exclude;OS/...) to the associated issuenpm run test -- brave_browser_tests,npm run test -- brave_unit_tests,npm run lint,npm run gn_check,npm run tslintgit rebase master(if needed)Reviewer Checklist:
gnAfter-merge Checklist:
changes has landed on
Test Plan:
Visit https://shivankaul.com and click on the LinkedIn icon. The navigation to LinkedIn should not be blocked, both when
#brave-domain-blockis enabled and disabled under brave://flags.Also, ensure that when
#brave-domain-blockis enabled under brave://flags, visiting https://1-1ads.com brings up a "Suspicious site ahead" interstitial with a "Proceed" button.