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

Fix 7522: Disable WebSQL #4463

Merged
merged 1 commit into from
Feb 6, 2020
Merged

Fix 7522: Disable WebSQL #4463

merged 1 commit into from
Feb 6, 2020

Conversation

jumde
Copy link
Contributor

@jumde jumde commented Jan 29, 2020

fix brave/brave-browser#7522

Submitter Checklist:

Test Plan:

  1. open devtools
  2. window.openDatabase should be undefined

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

@jumde jumde force-pushed the disable_websql branch 3 times, most recently from fb79f49 to a9cff6d Compare January 31, 2020 00:26
@jumde jumde changed the title [WIP] Fix 7522: Disable WebSQL Fix 7522: Disable WebSQL Jan 31, 2020
@jumde jumde self-assigned this Jan 31, 2020
@diracdeltas
Copy link
Member

haven't tried it but the approach and test plan lgtm

void SetUpOnMainThread() override {
InProcessBrowserTest::SetUpOnMainThread();

content_client_.reset(new ChromeContentClient);
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need explicitly set again content client and browser content client here.
They also will be set properly for browser test.

ASSERT_TRUE(content::WaitForLoadStop(contents));
EXPECT_EQ(url, contents->GetURL());

bool websqlBlocked;
Copy link
Member

Choose a reason for hiding this comment

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

nit: websql_blocked is our convention.

@jumde jumde force-pushed the disable_websql branch 2 times, most recently from eaf2135 to fe6e9f8 Compare January 31, 2020 04:46
Copy link
Member

@simonhong simonhong left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@jumde jumde force-pushed the disable_websql branch 2 times, most recently from e1e7cdf to b652be8 Compare February 6, 2020 06:24
@jumde jumde merged commit a50fa1d into master Feb 6, 2020
@jumde jumde deleted the disable_websql branch February 6, 2020 22:41
@jumde jumde added this to the 1.6.x - Nightly milestone Feb 6, 2020
bsclifton added a commit that referenced this pull request Mar 5, 2020
Speed dial extension was affected; may impact others too.

Fixes brave/brave-browser#8542

-----

Revert "Merge pull request #4463 from brave/disable_websql"

This reverts commit a50fa1d, reversing
changes made to cd4edd3.
@bbondy bbondy modified the milestones: 1.6.x - Beta, 1.7.x - Dev Mar 10, 2020
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.

Disable Web SQL
4 participants