Skip to content

Commit

Permalink
Implement API review feedback for WKWebExtensionAction
Browse files Browse the repository at this point in the history
rdar://118007942
https://bugs.webkit.org/show_bug.cgi?id=264270

Reviewed by Timothy Hatcher.

Implement feedback for the first round of review for WKWebExtensionAction. This mostly contains
mostly minor changes such as name changes.

* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionAction.h:
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionAction.mm:
(-[_WKWebExtensionAction label]):
(-[_WKWebExtensionAction presentsPopup]):
(-[_WKWebExtensionAction displayLabel]): Deleted.
(-[_WKWebExtensionAction hasPopup]): Deleted.
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionContext.h:
* Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionControllerDelegate.h:
* Source/WebKit/UIProcess/Extensions/Cocoa/API/WebExtensionContextAPIActionCocoa.mm:
(WebKit::WebExtensionContext::actionGetTitle):
(WebKit::WebExtensionContext::actionSetTitle):
(WebKit::WebExtensionContext::actionOpenPopup):
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionActionCocoa.mm:
(WebKit::WebExtensionAction::WebExtensionAction):
(WebKit::WebExtensionAction::popupWebView):
(WebKit::WebExtensionAction::readyToPresentPopup):
(WebKit::WebExtensionAction::label const):
(WebKit::WebExtensionAction::setLabel):
(WebKit::WebExtensionAction::displayLabel const): Deleted.
(WebKit::WebExtensionAction::setDisplayLabel): Deleted.
* Source/WebKit/UIProcess/Extensions/Cocoa/WebExtensionContextCocoa.mm:
(WebKit::WebExtensionContext::performAction):
* Source/WebKit/UIProcess/Extensions/WebExtensionAction.h:
(WebKit::WebExtensionAction::presentsPopup const):
(WebKit::WebExtensionAction::hasPopup const): Deleted.
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIAction.mm:
(TestWebKitAPI::TEST):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIExtension.mm:
(TestWebKitAPI::TEST):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebExtensionAPIRuntime.mm:
(TestWebKitAPI::TEST):
* Tools/TestWebKitAPI/cocoa/TestWebExtensionsDelegate.h:
* Tools/TestWebKitAPI/cocoa/TestWebExtensionsDelegate.mm:
(-[TestWebExtensionsDelegate webExtensionController:presentPopupForAction:forExtensionContext:completionHandler:]):
(-[TestWebExtensionsDelegate webExtensionController:presentActionPopup:forExtensionContext:completionHandler:]): Deleted.

Canonical link: https://commits.webkit.org/270330@main
  • Loading branch information
kiaraarose committed Nov 7, 2023
1 parent c72276a commit 9a90643
Show file tree
Hide file tree
Showing 13 changed files with 79 additions and 76 deletions.
9 changes: 6 additions & 3 deletions Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionAction.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ WK_CLASS_AVAILABLE(macos(WK_MAC_TBA), ios(WK_IOS_TBA))
NS_SWIFT_NAME(_WKWebExtension.Action)
@interface _WKWebExtensionAction : NSObject

- (instancetype)init NS_UNAVAILABLE;
- (instancetype)new NS_UNAVAILABLE;

/*! @abstract The extension context to which this action is related. */
@property (nonatomic, readonly, weak) _WKWebExtensionContext *webExtensionContext;

Expand All @@ -82,10 +85,10 @@ NS_SWIFT_NAME(_WKWebExtension.Action)
#endif

/*! @abstract The localized display label for the action. */
@property (nonatomic, readonly, copy) NSString *displayLabel;
@property (nonatomic, readonly, copy) NSString *label;

/*! @abstract The badge text for the action. */
@property (nonatomic, nullable, readonly, copy) NSString *badgeText;
@property (nonatomic, readonly, copy) NSString *badgeText;

/*! @abstract A Boolean value indicating whether the action is enabled. */
@property (nonatomic, readonly, getter=isEnabled) BOOL enabled;
Expand All @@ -95,7 +98,7 @@ NS_SWIFT_NAME(_WKWebExtension.Action)
@discussion Use this property to check if the action has a popup before attempting to access the `popupWebView` property.
@seealso popupWebView
*/
@property (nonatomic, readonly) BOOL hasPopup;
@property (nonatomic, readonly) BOOL presentsPopup;

/*!
@abstract A web view loaded with the popup page for this action, or `nil` if no popup is specified.
Expand Down
12 changes: 6 additions & 6 deletions Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionAction.mm
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ - (CocoaImage *)iconForSize:(CGSize)size
return _webExtensionAction->icon(size);
}

- (NSString *)displayLabel
- (NSString *)label
{
return _webExtensionAction->displayLabel();
return _webExtensionAction->label();
}

- (NSString *)badgeText
Expand All @@ -100,9 +100,9 @@ - (BOOL)isEnabled
return _webExtensionAction->isEnabled();
}

- (BOOL)hasPopup
- (BOOL)presentsPopup
{
return _webExtensionAction->hasPopup();
return _webExtensionAction->presentsPopup();
}

- (WKWebView *)popupWebView
Expand Down Expand Up @@ -144,7 +144,7 @@ - (CocoaImage *)iconForSize:(CGSize)size
return nil;
}

- (NSString *)displayLabel
- (NSString *)label
{
return nil;
}
Expand All @@ -159,7 +159,7 @@ - (BOOL)isEnabled
return NO;
}

- (BOOL)hasPopup
- (BOOL)presentsPopup
{
return NO;
}
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/UIProcess/API/Cocoa/_WKWebExtensionContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ WK_CLASS_AVAILABLE(macos(13.3), ios(16.4))
@discussion The returned object represents the action specific to the tab when provided; otherwise, it returns the default action. The default
action is useful when the context is unrelated to a specific tab. When possible, specify the tab to get the most context-relevant action.
*/
- (_WKWebExtensionAction *)actionForTab:(nullable id <_WKWebExtensionTab>)tab NS_SWIFT_NAME(action(for:));
- (nullable _WKWebExtensionAction *)actionForTab:(nullable id <_WKWebExtensionTab>)tab NS_SWIFT_NAME(action(for:));

/*!
@abstract Performs the extension action associated with the specified tab or performs the default action if `nil` is passed.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ WK_API_AVAILABLE(macos(13.3), ios(16.4))
called when the web view for the popup is fully loaded and ready to display. Implementing this method is needed if the app intends to support
programmatically showing the popup by the extension, although it is recommended for handling both programmatic and user-initiated cases.
*/
- (void)webExtensionController:(_WKWebExtensionController *)controller presentActionPopup:(_WKWebExtensionAction *)action forExtensionContext:(_WKWebExtensionContext *)context completionHandler:(void (^)(NSError * _Nullable error))completionHandler;
- (void)webExtensionController:(_WKWebExtensionController *)controller presentPopupForAction:(_WKWebExtensionAction *)action forExtensionContext:(_WKWebExtensionContext *)context completionHandler:(void (^)(NSError * _Nullable error))completionHandler;

/*!
@abstract Called when an extension context wants to send a one-time message to an application.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@

ASSERT(action);

completionHandler(action->displayLabel(WebExtensionAction::FallbackWhenEmpty::No), std::nullopt);
completionHandler(action->label(WebExtensionAction::FallbackWhenEmpty::No), std::nullopt);
}

void WebExtensionContext::actionSetTitle(std::optional<WebExtensionWindowIdentifier> windowIdentifier, std::optional<WebExtensionTabIdentifier> tabIdentifier, const String& title, CompletionHandler<void(std::optional<String>)>&& completionHandler)
Expand All @@ -122,7 +122,7 @@

ASSERT(action);

action->setDisplayLabel(title);
action->setLabel(title);

completionHandler(std::nullopt);
}
Expand Down Expand Up @@ -196,7 +196,7 @@
}

if (auto activeTab = window->activeTab()) {
if (getAction(activeTab.get())->hasPopup())
if (getAction(activeTab.get())->presentsPopup())
performAction(activeTab.get(), UserTriggered::No);

completionHandler(std::nullopt);
Expand All @@ -211,14 +211,14 @@
return;
}

if (getAction(tab.get())->hasPopup())
if (getAction(tab.get())->presentsPopup())
performAction(tab.get(), UserTriggered::No);

completionHandler(std::nullopt);
return;
}

if (defaultAction().hasPopup())
if (defaultAction().presentsPopup())
performAction(nullptr, UserTriggered::No);

completionHandler(std::nullopt);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,10 @@ - (void)invalidateIntrinsicContentSize
: m_extensionContext(extensionContext)
{
auto delegate = extensionContext.extensionController()->delegate();
m_respondsToPresentPopup = [delegate respondsToSelector:@selector(webExtensionController:presentActionPopup:forExtensionContext:completionHandler:)];
m_respondsToPresentPopup = [delegate respondsToSelector:@selector(webExtensionController:presentPopupForAction:forExtensionContext:completionHandler:)];

if (!m_respondsToPresentPopup)
RELEASE_LOG_ERROR(Extensions, "%{public}@ does not implement the webExtensionController:presentActionPopup:forExtensionContext:completionHandler: method", delegate.debugDescription);
RELEASE_LOG_ERROR(Extensions, "%{public}@ does not implement the webExtensionController:presentPopupForAction:forExtensionContext:completionHandler: method", delegate.debugDescription);
}

WebExtensionAction::WebExtensionAction(WebExtensionContext& extensionContext, WebExtensionTab& tab)
Expand Down Expand Up @@ -233,7 +233,7 @@ - (void)invalidateIntrinsicContentSize

WKWebView *WebExtensionAction::popupWebView(LoadOnFirstAccess loadOnFirstAccess)
{
if (!hasPopup())
if (!presentsPopup())
return nil;

if (m_popupWebView || loadOnFirstAccess == LoadOnFirstAccess::No)
Expand Down Expand Up @@ -311,7 +311,7 @@ - (void)invalidateIntrinsicContentSize
auto* extensionController = extensionContext()->extensionController();
auto delegate = extensionController->delegate();

[delegate webExtensionController:extensionController->wrapper() presentActionPopup:wrapper() forExtensionContext:extensionContext()->wrapper() completionHandler:makeBlockPtr([this, protectedThis = Ref { *this }](NSError *error) {
[delegate webExtensionController:extensionController->wrapper() presentPopupForAction:wrapper() forExtensionContext:extensionContext()->wrapper() completionHandler:makeBlockPtr([this, protectedThis = Ref { *this }](NSError *error) {
if (error)
closePopupWebView();
}).get()];
Expand All @@ -336,7 +336,7 @@ - (void)invalidateIntrinsicContentSize
m_popupPresented = false;
}

String WebExtensionAction::displayLabel(FallbackWhenEmpty fallback) const
String WebExtensionAction::label(FallbackWhenEmpty fallback) const
{
if (!extensionContext())
return emptyString();
Expand All @@ -349,18 +349,18 @@ - (void)invalidateIntrinsicContentSize
}

if (m_tab)
return extensionContext()->getAction(m_tab->window().get())->displayLabel();
return extensionContext()->getAction(m_tab->window().get())->label();

if (m_window)
return extensionContext()->defaultAction().displayLabel();
return extensionContext()->defaultAction().label();

if (auto *defaultLabel = extensionContext()->extension().displayActionLabel(); defaultLabel.length || fallback == FallbackWhenEmpty::No)
return defaultLabel;

return extensionContext()->extension().displayName();
}

void WebExtensionAction::setDisplayLabel(String label)
void WebExtensionAction::setLabel(String label)
{
m_customLabel = label;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1693,7 +1693,7 @@ static _WKWebExtensionContextError toAPI(WebExtensionContext::Error error)
userGesturePerformed(*tab);

auto action = getOrCreateAction(tab);
if (action->hasPopup()) {
if (action->presentsPopup()) {
action->presentPopupWhenReady();
return;
}
Expand Down
6 changes: 3 additions & 3 deletions Source/WebKit/UIProcess/Extensions/WebExtensionAction.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,16 +74,16 @@ class WebExtensionAction : public API::ObjectImpl<API::Object::Type::WebExtensio
CocoaImage *icon(CGSize);
void setIconsDictionary(NSDictionary *);

String displayLabel(FallbackWhenEmpty = FallbackWhenEmpty::Yes) const;
void setDisplayLabel(String);
String label(FallbackWhenEmpty = FallbackWhenEmpty::Yes) const;
void setLabel(String);

String badgeText() const;
void setBadgeText(String);

bool isEnabled() const;
void setEnabled(std::optional<bool>);

bool hasPopup() const { return !popupPath().isEmpty(); }
bool presentsPopup() const { return !popupPath().isEmpty(); }
bool canProgrammaticallyPresentPopup() const { return m_respondsToPresentPopup; }

String popupPath() const;
Expand Down
Loading

0 comments on commit 9a90643

Please sign in to comment.