Skip to content

Commit

Permalink
Adapt to simplified flow of programmatically showing popups
Browse files Browse the repository at this point in the history
Chromium change:

https://source.chromium.org/chromium/chromium/src/+/072b6bf1b7db7f8b8cf2e2e3127a98a6b859d7bb

commit 072b6bf1b7db7f8b8cf2e2e3127a98a6b859d7bb
Author: Devlin Cronin <rdevlin.cronin@chromium.org>
Date:   Tue Feb 8 19:40:21 2022 +0000

    [Extensions UI] Simplify the flow of programmatically showing popups

    Extensions can programmatically open their toolbar action popups. Before
    this CL, the flow for this was:
      API Call
      -> ExtensionsToolbarContainer::ShowToolbarActionPopupForAPICall()
      -> ExtensionActionViewController::ExecuteAction() (#1)
      -> ExtensionActionViewController::ExecuteAction() (#2)
      -> TriggerPopup()

    This has the following issues:
    - ExecuteAction() is also used for executing other types of behaviors
      (like triggering an event listener), not just for opening popups.
    - ExecuteAction() may grant the extension tab permissions if it was
      indicated that the call was through a user gesture

    Both of these aren't *technically* issues, since we theoretically
    validate that the extension will open a popup (and not fire a listener)
    and never trigger the call as a user gesture. But, it's a tenuous
    balance.

    Refactor this to introduce a public TriggerPopupForAPI() method on
    ExtensionActionViewController that ExtensionsToolbarContainer can call
    directly. This clears up the issues above by making the contract and
    expectations clear, and ensures that there's no way we'd ever grant
    tab permissions or fire an event for a programmatic API call. As a
    bonus, we simplify the interface by removing one of the ExecuteAction()
    overloads and the need to specify `by_user` in the call, instead just
    having ExecuteUserAction().

    This will also make it easier to plumb through a callback for
    programmatic API calls when we need to wait for the popup to open.

    Bug: 1245093
  • Loading branch information
emerick committed Mar 14, 2022
1 parent 79d8ec1 commit 863a4de
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions browser/ui/views/brave_actions/brave_actions_container.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ void BraveActionsContainer::AddAction(const extensions::Extension* extension) {
// Handle if we are in a continuing pressed state for this extension.
if (is_rewards_pressed_ && id == brave_rewards_extension_id) {
is_rewards_pressed_ = false;
actions_[id].view_controller_->ExecuteAction(
true, ToolbarActionViewController::InvocationSource::kToolbarButton);
actions_[id].view_controller_->ExecuteUserAction(
ToolbarActionViewController::InvocationSource::kToolbarButton);
}
}
}
Expand Down Expand Up @@ -403,8 +403,8 @@ void BraveActionsContainer::OnBraveActionShouldTrigger(
actions_[extension_id].view_controller_
->ExecuteActionUI(*ui_relative_path);
else
actions_[extension_id].view_controller_->ExecuteAction(
true, ToolbarActionViewController::InvocationSource::kApi);
actions_[extension_id].view_controller_->ExecuteUserAction(
ToolbarActionViewController::InvocationSource::kApi);
}
}

Expand Down

0 comments on commit 863a4de

Please sign in to comment.