Skip to content

/ brave-core

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 layering violations in component updater and add missing dep #3211

Merged
merged 1 commit into from Aug 20, 2019
Merged

fix layering violations in component updater and add missing dep #3211

merged 1 commit into from Aug 20, 2019
+218 −404

Conversation

@bridiver
Copy link
Collaborator

bridiver commented Aug 20, 2019 •
edited

Submitter Checklist:

Test Plan:

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.
@bridiver bridiver requested review from jumde, pilgrim-brave and bbondy Aug 20, 2019
@mkarolin
mkarolin reviewed Aug 20, 2019
View changes
browser/extensions/brave_extension_install_prompt.cc Outdated
#include "brave/grit/brave_generated_resources.h"
#include "ui/base/l10n/l10n_util.h"

base::string16 BravePrompt::GetDialogTitle() const {
if (!extensions::BraveExtensionProvider::IsVetted(extension())) {
if (g_brave_browser_process->extension_whitelist_service()->IsVetted(

This comment has been minimized.

Sign in to view
Copy link
@mkarolin

mkarolin Aug 20, 2019

Contributor

! got lost.

This comment has been minimized.

Sign in to view
Copy link
@bridiver

bridiver Aug 20, 2019

Author Collaborator

fixed
@pilgrim-brave
pilgrim-brave reviewed Aug 20, 2019
View changes
Copy link
Contributor

pilgrim-brave left a comment

LGTM after fixing the missing ! that mkarolin mentioned
@mkarolin
mkarolin reviewed Aug 20, 2019
View changes
browser/extensions/brave_extension_provider.h
@@ -21,8 +21,7 @@ class BraveExtensionProvider : public ManagementPolicy::Provider {
base::string16* error) const override;
bool MustRemainInstalled(const Extension* extension,
base::string16* error) const override;
static bool IsVetted(const extensions::Extension* extension);
static bool IsVetted(const std::string id);

This comment has been minimized.

Sign in to view
Copy link
@mkarolin

mkarolin Aug 20, 2019

Contributor

Nit: extra line?

This comment has been minimized.

Sign in to view
Copy link
@bridiver

bridiver Aug 20, 2019

Author Collaborator

a blank line before private: is required by lint
@mkarolin
mkarolin reviewed Aug 20, 2019
View changes
components/brave_component_updater/browser/extension_whitelist_service.cc Outdated
@@ -38,6 +43,23 @@ bool ExtensionWhitelistService::IsBlacklisted(
return extension_whitelist_client_->isBlacklisted(extension_id.c_str());
}

bool ExtensionWhitelistService::IsVetted(const Extension* extension) const {
// This is a hardcoded list of vetted extensions, mostly

This comment has been minimized.

Sign in to view
Copy link
@mkarolin

mkarolin Aug 20, 2019

Contributor

This comment is probably better suited for whitelist.cc now.
@bridiver bridiver force-pushed the extension_whitelist branch from 7e6c2a6 to 3928f37 Aug 20, 2019
@mkarolin
mkarolin reviewed Aug 20, 2019
View changes
Copy link
Contributor

mkarolin left a comment

++
@bridiver bridiver force-pushed the extension_whitelist branch from 3928f37 to d01d5ba Aug 20, 2019
@bridiver
fix layering violations in component updater and add missing dep
Loading status checks…
ff32d2c 
fix brave/brave-browser#5744
@bridiver bridiver force-pushed the extension_whitelist branch from d01d5ba to ff32d2c Aug 20, 2019
@bridiver bridiver self-assigned this Aug 20, 2019
@bridiver bridiver added this to the 0.71.x - Nightly milestone Aug 20, 2019
@mkarolin
mkarolin approved these changes Aug 20, 2019
View changes
Copy link
Contributor

mkarolin left a comment

++
@bridiver bridiver merged commit dacefaf into master Aug 20, 2019
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/jenkins/pr-head This commit is being built
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
bridiver added a commit that referenced this pull request Aug 28, 2019
@bridiver
Merge pull request #3211 from brave/extension_whitelist
Loading status checks…
7afaf7a 
fix layering violations in component updater and add missing dep
@bridiver bridiver mentioned this pull request Aug 28, 2019
Closed
bridiver added a commit that referenced this pull request Aug 29, 2019
@bridiver
Merge pull request #3211 from brave/extension_whitelist
Loading status checks…
fcc9159 
fix layering violations in component updater and add missing dep
bridiver added a commit that referenced this pull request Aug 29, 2019
@bridiver
Merge pull request #3211 from brave/extension_whitelist
Loading status checks…
91a2c74 
fix layering violations in component updater and add missing dep
@bridiver bridiver mentioned this pull request Aug 29, 2019
Merged
bridiver added a commit that referenced this pull request Aug 29, 2019
@bridiver
Merge pull request #3211 from brave/extension_whitelist
341c607 
fix layering violations in component updater and add missing dep
@bsclifton bsclifton deleted the extension_whitelist branch Sep 12, 2019
@bsclifton
Copy link
Member

bsclifton commented Sep 12, 2019

Updated to be 0.71.x milestone, which is where this landed. Viewable in commit log here:
https://github.com/brave/brave-core/commits/0.71.x?after=886f8520767f78e257397cfa47ef585b35bf1aca+174

Will be uplifting to 0.70.x 👍
bsclifton added a commit that referenced this pull request Sep 12, 2019
@bridiver @bsclifton
Merge pull request #3211 from brave/extension_whitelist
Verified
This commit was signed with a verified signature.
bsclifton Brian Clifton
GPG key ID: 50ED18CA5C4C0F73 Learn about signing commits
8d630a6 
fix layering violations in component updater and add missing dep
@bsclifton bsclifton mentioned this pull request Sep 12, 2019
Merged
3 of 4 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkarolin mkarolin

@pilgrim-brave pilgrim-brave

@bbondy bbondy

@jumde jumde

Assignees

@bridiver bridiver

Labels
None yet
Projects
None yet
Milestone
  0.70.x - Release
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@bridiver @bsclifton @mkarolin @pilgrim-brave
You can’t perform that action at this time.