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

Fixes extensions install prompt. #5772

Merged
merged 1 commit into from
Jun 9, 2020
Merged

Conversation

mkarolin
Copy link
Collaborator

@mkarolin mkarolin commented Jun 8, 2020

Fixes brave/brave-browser#10113

Removed BravePrompt that inherited from ExtensionInstallPrompt::Prompt
and overrode GetDialogTitle method. Instead, renamed the original method
to GetDialogTitle_ChromiumImpl and added our own GetDialogTitle.

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.

The override of extensions install prompt text has regressed with
Chromium 83 update.

Removed BravePrompt that inherited from ExtensionInstallPrompt::Prompt
and overrode GetDialogTitle method. Instead, renamed the original method
to GetDialogTitle_ChromiumImpl and added our own GetDialogTitle.

Fixes brave/brave-browser#10113

The regression is due to Chromium change that was creating
ExtensionInstallPrompt::Prompt from
WebstorePrivateBeginInstallWithManifest3Function::OnWebstoreParseSuccess.

Chromium change:

https://chromium.googlesource.com/chromium/src/+/4f35260605f0a6fa5153bcc40cc9ed7b235fddec

commit 4f35260605f0a6fa5153bcc40cc9ed7b235fddec
Author: Danan S <[email protected]>
Date:   Fri Mar 27 00:41:17 2020 +0000

    Reland "Changes to Webstore Private API to support child extension installation"

    This relands the feature originally landed in
    99ffda8ef2c76aa79923281a57c82a70e68d0d45

    That CL was reverted due to an use-after-free error triggered by
    ExtensionWebstorePrivateApiTestChildInstallEnabled, but caused
    by already landed code used by the Webstore Private API.

    The actual fix for the use-after-free was fixed in http://crrev.com/c/2100548

    That fix involved changes in the ParentPermissionDialog APIs, which are
    reflected in this reland CL.

    Original change's description:
    > Revert "Changes to Webstore Private API to support child extension installation"
    >
    > This reverts commit 99ffda8ef2c76aa79923281a57c82a70e68d0d45.
    >
    > Reason for revert: browser_tests failing on https://ci.chromium.org/p/chromium/builders/ci/Linux%20Chromium%20OS%20ASan%20LSan%20Tests%20%281%29/37129 and https://ci.chromium.org/p/chromium/builders/ci/Linux%20ChromiumOS%20MSan%20Tests/18174
    >
    > Original change's description:
    > > Changes to Webstore Private API to support child extension installation
    > >
    > > These changes are required in order to prompt a child user to get
    > > parent permission when they attempt to install an extension in the
    > > Chrome Webstore.
    > >
    > > This CL also enables the feature by default.
    > >
    > > Bug: 957832
@mkarolin mkarolin added this to the 1.12.x - Nightly milestone Jun 8, 2020
@mkarolin mkarolin requested a review from bbondy June 8, 2020 15:15
@mkarolin mkarolin requested a review from bridiver as a code owner June 8, 2020 15:15
@mkarolin mkarolin self-assigned this Jun 8, 2020
@mbacchi mbacchi added CI/skip-ios Do not run CI builds for iOS CI/skip-linux CI/skip-macos-x64 Do not run CI builds for macOS x64 labels Jun 8, 2020
@mkarolin mkarolin requested a review from bsclifton June 9, 2020 18:15
@mkarolin mkarolin merged commit ef28e18 into master Jun 9, 2020
@mkarolin mkarolin deleted the maxk-fix-extensions-prompt branch June 9, 2020 18:51
mkarolin added a commit that referenced this pull request Jun 9, 2020
mkarolin added a commit that referenced this pull request Jun 9, 2020
@GeetaSarvadnya
Copy link

GeetaSarvadnya commented Jun 10, 2020

Verification passed on


Brave | 1.12.14 Chromium: 83.0.4103.97 (Official Build) nightly (64-bit)
-- | --
Revision | 326d148b9655369b86498d9ecca39f63dd2bdd2d-refs/branch-heads/4103@{#657}
OS | Windows 10 OS Version 1803 (Build 17134.1006)

image

  • Verified that Brave has not reviewed this extension for security and safety warning message is displayed while adding extension Google translate
    image

  • Verified that Brave has not reviewed this extension for security and safety warning message is displayed while adding extension dark reader
    image

  • Verified that Brave has not reviewed this extension for security and safety warning message is displayed while adding extension Edit this cookie
    image

  • Verified that extensions can be added to brave after reviewing warning message

  • Verified that LastPass/honey/pocket and other extensions can be added to brave without any issues

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/skip-ios Do not run CI builds for iOS CI/skip-macos-x64 Do not run CI builds for macOS x64
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extension not reviewed for security and safety warning message is not displayed while adding extensions
4 participants