Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow first party iframes embedded in 3rd-party origins plus exceptio… (uplift to 1.9.x) #5494

Merged
merged 2 commits into from
May 28, 2020

Conversation

bsclifton
Copy link
Member

@bsclifton bsclifton commented May 7, 2020

Uplift of #5433
fix brave/brave-browser#8629
fix brave/brave-browser#9564
fix brave/brave-browser#9105

Approved, please ensure that before merging:

  • You have checked CI and the builds, lint, and tests all pass or are not related to your PR.
  • You have tested your change on Nightly.
  • The PR milestones match the branch they are landing to.

After you merge:

  • The associated issue milestone is set to the smallest version that the changes is landed on.

@bsclifton bsclifton added this to the 1.9.x - Beta milestone May 7, 2020
@bsclifton bsclifton requested a review from a team May 7, 2020 22:24
@bsclifton bsclifton self-assigned this May 7, 2020
@kjozwiak
Copy link
Member

Looks like test-browser failed on macOS as per https://ci.brave.com/job/pr-brave-browser-fix-google-drive-autoplay-bj-1.9.x/1/execution/node/498/log/. This is a known intermittent issue which will be addressed via brave/brave-browser#9339 and doesn't block uplift.

19:59:08  [  FAILED  ] BraveRewardsBrowserTest.NotVerifiedWallet, where TypeParam =  and GetParam() =  (6148 ms)
19:59:08  [540/540] BraveRewardsBrowserTest.NotVerifiedWallet (6339 ms)
19:59:08  2 tests failed:
19:59:08      BraveNetworkDelegateBrowserTest.ThirdPartyCookiesBlockedNestedFirstPartyIframe (../../brave/browser/net/brave_network_delegate_browsertest.cc:252)
19:59:08      BraveRewardsBrowserTest.NotVerifiedWallet (../../brave/components/brave_rewards/browser/rewards_service_browsertest.cc:2326)

Travis also failed due to TEST_SUITE=test-security because of the yargs-parser npm package which is a known issue and not blocking uplift.

@kjozwiak
Copy link
Member

However, concerned about test-browser failing on Linux as per https://ci.brave.com/job/pr-brave-browser-fix-google-drive-autoplay-bj-1.9.x/1/execution/node/420/log/

19:16:17  [  FAILED  ] BraveNetworkDelegateBrowserTest.ThirdPartyCookiesBlockedNestedFirstPartyIframe, where TypeParam =  and GetParam() =  (760 ms)
19:16:17  [541/544] BraveNetworkDelegateBrowserTest.ThirdPartyCookiesBlockedNestedFirstPartyIframe (913 ms)
19:16:18  [542/544] PerfPredictorTabHelperTest.ScriptBlockHasSavings (651 ms)
19:16:19  [543/544] BraveRewardsBrowserTest.PanelShowsCorrectPublisherData (1307 ms)
19:16:25  [544/544] BraveRewardsBrowserTest.ProcessPendingContributions (5503 ms)
19:16:25  1 test failed:
19:16:25      BraveNetworkDelegateBrowserTest.ThirdPartyCookiesBlockedNestedFirstPartyIframe (../../brave/browser/net/brave_network_delegate_browsertest.cc:252)

I haven't seen the above before and it's a third party cookie failure which this issue touches. @bsclifton @bridiver can I get a confirmation/double check that the above is an intermittent issue and not related to the above chance?

@bsclifton
Copy link
Member Author

bsclifton commented May 12, 2020

Definitely not expected. @bridiver did I miss something while uplifting?

@bsclifton
Copy link
Member Author

Given the failing test and also how close we are to shipping 1.9, I'm going to close this out

@bsclifton bsclifton closed this May 19, 2020
@bsclifton bsclifton deleted the fix-google-drive-autoplay-bj-1.9.x branch May 19, 2020 06:34
@bsclifton bsclifton removed this from the 1.9.x - Release milestone May 19, 2020
@bsclifton bsclifton restored the fix-google-drive-autoplay-bj-1.9.x branch May 26, 2020 23:15
@bsclifton bsclifton reopened this May 26, 2020
@bsclifton bsclifton force-pushed the fix-google-drive-autoplay-bj-1.9.x branch from 6555d7d to 04690c0 Compare May 26, 2020 23:16
@bsclifton
Copy link
Member Author

Re-opening as we pulled a few issues into 1.9 Hotfix 1... going to test locally 😄

Gave PR a rebase so we can let CI run again

@bsclifton
Copy link
Member Author

Found the problem with the test - definitely my bad; stemmed from when I left out test in merge for this PR:
#5410

Working on fix... Should only be affecting test. Code should be G2G!

bridiver and others added 2 commits May 27, 2020 17:14
allow first party iframes embedded in 3rd-party origins plus exceptio…
- test failure was caused by bad merge in #5410
- manually updated test with proper logic
- other tests had to be updated because 1.9 doesn't have #5318
@bsclifton bsclifton added this to the 1.9.x - Release Hotfix 1 milestone May 28, 2020
@bsclifton bsclifton force-pushed the fix-google-drive-autoplay-bj-1.9.x branch from 4f214e9 to f0f8538 Compare May 28, 2020 00:15
@bsclifton
Copy link
Member Author

Rebased and pushed latest 😄 I ran all browser tests locally and they're passing!

@pes10k
Copy link
Contributor

pes10k commented May 28, 2020

I've confirmed that the original issue being solved here (the side panel widget on wordpress) works correctly on nightly now

@rebron
Copy link
Collaborator

rebron commented May 28, 2020

Tested on Beta: brave/brave-browser#9064 (comment)

@bsclifton
Copy link
Member Author

CI looks great - there was one browser test failure:
https://ci.brave.com/job/pr-brave-browser-fix-google-drive-autoplay-bj-1.9.x/5/execution/node/476/log/

18:39:20  [  FAILED  ] BraveRewardsBrowserTest.NotVerifiedWallet, where TypeParam =  and GetParam() =  (6248 ms)
18:39:20  [541/541] BraveRewardsBrowserTest.NotVerifiedWallet (6335 ms)
18:39:20  1 test failed:
18:39:20      BraveRewardsBrowserTest.NotVerifiedWallet (../../brave/components/brave_rewards/browser/rewards_service_browsertest.cc:2331)

This is a known issue captured with brave/brave-browser#9339 and it has been fixed in 1.11 😄

Copy link
Contributor

@srirambv srirambv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving after @bsclifton deliberated with @brave/uplift-approvers.

@bsclifton bsclifton merged commit 5ece9b0 into 1.9.x May 28, 2020
@bsclifton bsclifton deleted the fix-google-drive-autoplay-bj-1.9.x branch May 28, 2020 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants