Skip to content

Commit

Permalink
Refactor StoreAppHighlight message to be a reply to CreateAppHighligh…
Browse files Browse the repository at this point in the history
…tInSelectedRange

https://bugs.webkit.org/show_bug.cgi?id=271768
rdar://125081140

Reviewed by Chris Dumez.

StoreAppHighlight is logically a reply to CreateAppHighlightInSelectedRange that is always expected.
The messages.in machinery allows us to express this explicitly.
This helps clean up other ChromeClient code, etc, as well.

* Source/WebCore/Modules/highlight/AppHighlight.h:
(IPC::AsyncReplyError<WebCore::AppHighlight>::create):

* Source/WebCore/Modules/highlight/AppHighlightStorage.cpp:
(WebCore::AppHighlightStorage::storeAppHighlight):
* Source/WebCore/Modules/highlight/AppHighlightStorage.h:

* Source/WebCore/loader/EmptyClients.cpp:
(WebCore::EmptyChromeClient::storeAppHighlight const): Deleted.
* Source/WebCore/loader/EmptyClients.h:

* Source/WebCore/page/Chrome.cpp:
(WebCore::Chrome::storeAppHighlight const): Deleted.
* Source/WebCore/page/Chrome.h:
* Source/WebCore/page/ChromeClient.h:

* Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm:
(WebKit::WebPageProxy::createAppHighlightInSelectedRange):
* Source/WebKit/UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::storeAppHighlight): Deleted.
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.messages.in:

* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::storeAppHighlight const): Deleted.
* Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h:

* Source/WebKit/WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::createAppHighlightInSelectedRange):
* Source/WebKit/WebProcess/WebPage/WebPage.h:
* Source/WebKit/WebProcess/WebPage/WebPage.messages.in:

* Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.h:
* Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.mm:
(WebChromeClient::storeAppHighlight const): Deleted.

Canonical link: https://commits.webkit.org/272448.815@safari-7618-branch
  • Loading branch information
beidson committed Mar 28, 2024
1 parent 34d2dd8 commit 4935202
Show file tree
Hide file tree
Showing 19 changed files with 29 additions and 77 deletions.
13 changes: 12 additions & 1 deletion Source/WebCore/Modules/highlight/AppHighlight.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,17 @@ struct AppHighlight {
HighlightRequestOriginatedInApp requestOriginatedInApp;
};

}
} // namespace WebCore

namespace IPC {

template<typename AsyncReplyResult> struct AsyncReplyError;

template<> struct AsyncReplyError<WebCore::AppHighlight> {
static WebCore::AppHighlight create() { return { WebCore::FragmentedSharedBuffer::create(), std::nullopt, WebCore::CreateNewGroupForHighlight::No, WebCore::HighlightRequestOriginatedInApp::No }; }
};

} // namespace IPC


#endif // ENABLE(APP_HIGHLIGHTS)
7 changes: 3 additions & 4 deletions Source/WebCore/Modules/highlight/AppHighlightStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,16 @@ AppHighlightStorage::AppHighlightStorage(Document& document)

AppHighlightStorage::~AppHighlightStorage() = default;

void AppHighlightStorage::storeAppHighlight(Ref<StaticRange>&& range)
void AppHighlightStorage::storeAppHighlight(Ref<StaticRange>&& range, CompletionHandler<void(AppHighlight&&)>&& completionHandler)
{
auto data = createAppHighlightRangeData(range);
std::optional<String> text;

if (!data.text().isEmpty())
text = data.text();

AppHighlight highlight = {data.toSharedBuffer(), text, CreateNewGroupForHighlight::No, HighlightRequestOriginatedInApp::No};

m_document->page()->chrome().storeAppHighlight(WTFMove(highlight));
AppHighlight highlight = { data.toSharedBuffer(), text, CreateNewGroupForHighlight::No, HighlightRequestOriginatedInApp::No };
completionHandler(WTFMove(highlight));
}

void AppHighlightStorage::restoreAndScrollToAppHighlight(Ref<FragmentedSharedBuffer>&& buffer, ScrollToHighlight scroll)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/Modules/highlight/AppHighlightStorage.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class AppHighlightStorage final {
AppHighlightStorage(Document&);
~AppHighlightStorage();

WEBCORE_EXPORT void storeAppHighlight(Ref<StaticRange>&&);
WEBCORE_EXPORT void storeAppHighlight(Ref<StaticRange>&&, CompletionHandler<void(AppHighlight&&)>&&);
WEBCORE_EXPORT void restoreAndScrollToAppHighlight(Ref<FragmentedSharedBuffer>&&, ScrollToHighlight);
void restoreUnrestoredAppHighlights();
MonotonicTime lastRangeSearchTime() const { return m_timeAtLastRangeSearch; }
Expand Down
8 changes: 0 additions & 8 deletions Source/WebCore/loader/EmptyClients.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -568,14 +568,6 @@ std::unique_ptr<DateTimeChooser> EmptyChromeClient::createDateTimeChooser(DateTi

#endif

#if ENABLE(APP_HIGHLIGHTS)

void EmptyChromeClient::storeAppHighlight(AppHighlight&&) const
{
}

#endif

void EmptyChromeClient::setTextIndicator(const TextIndicatorData&) const
{
}
Expand Down
4 changes: 0 additions & 4 deletions Source/WebCore/loader/EmptyClients.h
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,6 @@ class EmptyChromeClient : public ChromeClient {
std::unique_ptr<DateTimeChooser> createDateTimeChooser(DateTimeChooserClient&) final;
#endif

#if ENABLE(APP_HIGHLIGHTS)
void storeAppHighlight(AppHighlight&&) const final;
#endif

void setTextIndicator(const TextIndicatorData&) const final;

DisplayRefreshMonitorFactory* displayRefreshMonitorFactory() const final;
Expand Down
7 changes: 0 additions & 7 deletions Source/WebCore/page/Chrome.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -534,13 +534,6 @@ void Chrome::dispatchViewportPropertiesDidChange(const ViewportArguments& argume
m_client->dispatchViewportPropertiesDidChange(arguments);
}

#if ENABLE(APP_HIGHLIGHTS)
void Chrome::storeAppHighlight(AppHighlight&& highlight) const
{
m_client->storeAppHighlight(WTFMove(highlight));
}
#endif

void Chrome::setCursor(const Cursor& cursor)
{
m_client->setCursor(cursor);
Expand Down
4 changes: 0 additions & 4 deletions Source/WebCore/page/Chrome.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,6 @@ class Chrome : public HostWindow {

std::unique_ptr<WorkerClient> createWorkerClient(SerialFunctionDispatcher&);

#if ENABLE(APP_HIGHLIGHTS)
void storeAppHighlight(AppHighlight&&) const;
#endif

void runOpenPanel(LocalFrame&, FileChooser&);
void showShareSheet(ShareDataWithParsedURL&, CompletionHandler<void(bool)>&&);
void showContactPicker(const ContactsRequestData&, CompletionHandler<void(std::optional<Vector<ContactInfo>>&&)>&&);
Expand Down
4 changes: 0 additions & 4 deletions Source/WebCore/page/ChromeClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -350,10 +350,6 @@ class ChromeClient {
virtual std::unique_ptr<DateTimeChooser> createDateTimeChooser(DateTimeChooserClient&) = 0;
#endif

#if ENABLE(APP_HIGHLIGHTS)
virtual void storeAppHighlight(WebCore::AppHighlight&&) const = 0;
#endif

virtual void setTextIndicator(const TextIndicatorData&) const = 0;

virtual void runOpenPanel(LocalFrame&, FileChooser&) = 0;
Expand Down
6 changes: 5 additions & 1 deletion Source/WebKit/UIProcess/Cocoa/WebPageProxyCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,11 @@ static bool exceedsRenderTreeSizeSizeThreshold(uint64_t thresholdSize, uint64_t

setUpHighlightsObserver();

send(Messages::WebPage::CreateAppHighlightInSelectedRange(createNewGroup, requestOriginatedInApp));
auto completionHandler = [this, protectedThis = Ref { *this }] (WebCore::AppHighlight&& highlight) {
MESSAGE_CHECK(!highlight.highlight->isEmpty());
pageClient().storeAppHighlight(highlight);
};
sendWithAsyncReply(Messages::WebPage::CreateAppHighlightInSelectedRange(createNewGroup, requestOriginatedInApp), WTFMove(completionHandler));
}

void WebPageProxy::restoreAppHighlightsAndScrollToIndex(const Vector<Ref<SharedMemory>>& highlights, const std::optional<unsigned> index)
Expand Down
10 changes: 0 additions & 10 deletions Source/WebKit/UIProcess/WebPageProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12166,16 +12166,6 @@ void WebPageProxy::getTextFragmentMatch(CompletionHandler<void(const String&)>&&
sendWithAsyncReply(Messages::WebPage::GetTextFragmentMatch(), WTFMove(callback));
}

#if ENABLE(APP_HIGHLIGHTS)
void WebPageProxy::storeAppHighlight(const WebCore::AppHighlight& highlight)
{
MESSAGE_CHECK(m_process, !highlight.highlight->isEmpty());

pageClient().storeAppHighlight(highlight);

}
#endif

namespace {
enum class CompletionCondition {
Cancellation,
Expand Down
1 change: 0 additions & 1 deletion Source/WebKit/UIProcess/WebPageProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -2128,7 +2128,6 @@ class WebPageProxy final : public API::ObjectImpl<API::Object::Type::Page>, publ

#if ENABLE(APP_HIGHLIGHTS)
void createAppHighlightInSelectedRange(WebCore::CreateNewGroupForHighlight, WebCore::HighlightRequestOriginatedInApp);
void storeAppHighlight(const WebCore::AppHighlight&);
void restoreAppHighlightsAndScrollToIndex(const Vector<Ref<SharedMemory>>& highlights, const std::optional<unsigned> index);
void setAppHighlightsVisibility(const WebCore::HighlightVisibility);
bool appHighlightsVisibility();
Expand Down
4 changes: 0 additions & 4 deletions Source/WebKit/UIProcess/WebPageProxy.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -517,10 +517,6 @@ messages -> WebPageProxy {
[EnabledIf='attachmentElementEnabled()'] RequestAttachmentIcon(String identifier, String contentType, String path, String title, WebCore::FloatSize size)
#endif

#if ENABLE(APP_HIGHLIGHTS)
StoreAppHighlight(struct WebCore::AppHighlight info)
#endif

#if ENABLE(SPEECH_SYNTHESIS)
SpeechSynthesisVoiceList() -> (Vector<WebKit::WebSpeechSynthesisVoice> voiceList) Synchronous
SpeechSynthesisSpeak(String text, String lang, float volume, float rate, float pitch, MonotonicTime startTime, String voiceURI, String voiceName, String voiceLang, bool localService, bool defaultVoice) -> ()
Expand Down
10 changes: 0 additions & 10 deletions Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1490,16 +1490,6 @@ bool WebChromeClient::unwrapCryptoKey(const Vector<uint8_t>& wrappedKey, Vector<
return succeeded;
}

#if ENABLE(APP_HIGHLIGHTS)
void WebChromeClient::storeAppHighlight(WebCore::AppHighlight&& highlight) const
{
auto page = protectedPage();
highlight.isNewGroup = page->highlightIsNewGroup();
highlight.requestOriginatedInApp = page->highlightRequestOriginatedInApp();
page->send(Messages::WebPageProxy::StoreAppHighlight(highlight));
}
#endif

void WebChromeClient::setTextIndicator(const WebCore::TextIndicatorData& indicatorData) const
{
protectedPage()->setTextIndicator(indicatorData);
Expand Down
4 changes: 0 additions & 4 deletions Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -382,10 +382,6 @@ class WebChromeClient final : public WebCore::ChromeClient {
void isPlayingMediaDidChange(WebCore::MediaProducerMediaStateFlags) final;
void handleAutoplayEvent(WebCore::AutoplayEvent, OptionSet<WebCore::AutoplayEventFlags>) final;

#if ENABLE(APP_HIGHLIGHTS)
void storeAppHighlight(WebCore::AppHighlight&&) const final;
#endif

void setTextIndicator(const WebCore::TextIndicatorData&) const final;

bool wrapCryptoKey(const Vector<uint8_t>&, Vector<uint8_t>&) const final;
Expand Down
8 changes: 6 additions & 2 deletions Source/WebKit/WebProcess/WebPage/WebPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8803,7 +8803,7 @@ void WebPage::revokeSandboxExtensions(Vector<Ref<SandboxExtension>>& sandboxExte
}

#if ENABLE(APP_HIGHLIGHTS)
bool WebPage::createAppHighlightInSelectedRange(WebCore::CreateNewGroupForHighlight createNewGroup, WebCore::HighlightRequestOriginatedInApp requestOriginatedInApp)
bool WebPage::createAppHighlightInSelectedRange(WebCore::CreateNewGroupForHighlight createNewGroup, WebCore::HighlightRequestOriginatedInApp requestOriginatedInApp, CompletionHandler<void(WebCore::AppHighlight&&)>&& completionHandler)
{
SetForScope highlightIsNewGroupScope { m_highlightIsNewGroup, createNewGroup };
SetForScope highlightRequestOriginScope { m_highlightRequestOriginatedInApp, requestOriginatedInApp };
Expand All @@ -8822,7 +8822,11 @@ bool WebPage::createAppHighlightInSelectedRange(WebCore::CreateNewGroupForHighli
return false;

document->appHighlightRegistry().addAnnotationHighlightWithRange(StaticRange::create(selectionRange.value()));
document->appHighlightStorage().storeAppHighlight(StaticRange::create(selectionRange.value()));
document->appHighlightStorage().storeAppHighlight(StaticRange::create(selectionRange.value()), [completionHandler = WTFMove(completionHandler), protectedThis = Ref { *this }, this] (WebCore::AppHighlight&& highlight) mutable {
highlight.isNewGroup = m_highlightIsNewGroup;
highlight.requestOriginatedInApp = m_highlightRequestOriginatedInApp;
completionHandler(WTFMove(highlight));
});

return true;
}
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/WebPage/WebPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -1587,7 +1587,7 @@ class WebPage : public API::ObjectImpl<API::Object::Type::BundlePage>, public IP
WebCore::HighlightRequestOriginatedInApp highlightRequestOriginatedInApp() const { return m_highlightRequestOriginatedInApp; }
WebCore::HighlightVisibility appHighlightsVisiblility() const { return m_appHighlightsVisible; }

bool createAppHighlightInSelectedRange(WebCore::CreateNewGroupForHighlight, WebCore::HighlightRequestOriginatedInApp);
bool createAppHighlightInSelectedRange(WebCore::CreateNewGroupForHighlight, WebCore::HighlightRequestOriginatedInApp, CompletionHandler<void(WebCore::AppHighlight&&)>&&);
void restoreAppHighlightsAndScrollToIndex(Vector<SharedMemoryHandle>&&, const std::optional<unsigned> index);
void setAppHighlightsVisibility(const WebCore::HighlightVisibility);
#endif
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/WebProcess/WebPage/WebPage.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ GenerateSyntheticEditingCommand(enum:uint8_t WebKit::SyntheticEditingCommandType
SetCanUseCredentialStorage(bool canUse)

#if ENABLE(APP_HIGHLIGHTS)
CreateAppHighlightInSelectedRange(enum:bool WebCore::CreateNewGroupForHighlight createNewGroup, enum:bool WebCore::HighlightRequestOriginatedInApp requestOrigin)
CreateAppHighlightInSelectedRange(enum:bool WebCore::CreateNewGroupForHighlight createNewGroup, enum:bool WebCore::HighlightRequestOriginatedInApp requestOrigin) -> (struct WebCore::AppHighlight info)
RestoreAppHighlightsAndScrollToIndex(Vector<WebKit::SharedMemory::Handle> memoryHandles, std::optional<unsigned> index)
SetAppHighlightsVisibility(enum:bool WebCore::HighlightVisibility highlightVisibility)
#endif
Expand Down
4 changes: 0 additions & 4 deletions Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,6 @@ class WebChromeClient : public WebCore::ChromeClient {
std::unique_ptr<WebCore::DateTimeChooser> createDateTimeChooser(WebCore::DateTimeChooserClient&) final;
#endif

#if ENABLE(APP_HIGHLIGHTS)
void storeAppHighlight(WebCore::AppHighlight&&) const final;
#endif

void setTextIndicator(const WebCore::TextIndicatorData&) const final;

#if ENABLE(POINTER_LOCK)
Expand Down
6 changes: 0 additions & 6 deletions Source/WebKitLegacy/mac/WebCoreSupport/WebChromeClient.mm
Original file line number Diff line number Diff line change
Expand Up @@ -724,12 +724,6 @@ - (void)setIsSelected:(BOOL)isSelected;
}
#endif

#if ENABLE(APP_HIGHLIGHTS)
void WebChromeClient::storeAppHighlight(WebCore::AppHighlight&&) const
{
}
#endif

void WebChromeClient::setTextIndicator(const WebCore::TextIndicatorData& indicatorData) const
{
}
Expand Down

0 comments on commit 4935202

Please sign in to comment.