Skip to content

Commit

Permalink
Cherry-pick d6540a3. rdar://126492909
Browse files Browse the repository at this point in the history
    Regression(277427@main) Crash under AuxiliaryProcessProxy::notifyPreferencesChanged()
    https://bugs.webkit.org/show_bug.cgi?id=272695
    rdar://126492909

    Reviewed by Per Arne Vollan.

    We were using a HashMap to store preferences whose key was a std::pair<String, String>.
    The first String was the domain and the second the preference name. However, for global
    preferences, the domain is null, causing a crash when hashing the key.

    To address an issue, we now store global preferences in a separate HashMap.

    * Source/WebKit/Shared/AuxiliaryProcess.h:
    * Source/WebKit/Shared/AuxiliaryProcess.messages.in:
    * Source/WebKit/Shared/Cocoa/AuxiliaryProcessCocoa.mm:
    (WebKit::AuxiliaryProcess::preferencesDidUpdate):
    * Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:
    (WebKit::AuxiliaryProcessProxy::didChangeThrottleState):
    * Source/WebKit/UIProcess/AuxiliaryProcessProxy.h:
    * Source/WebKit/UIProcess/Cocoa/AuxiliaryProcessProxyCocoa.mm:
    (WebKit::AuxiliaryProcessProxy::notifyPreferencesChanged):

    Canonical link: https://commits.webkit.org/277514@main
  • Loading branch information
cdumez authored and Mohsin Qureshi committed Apr 16, 2024
1 parent 6ae7097 commit 168c94d
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 8 deletions.
2 changes: 1 addition & 1 deletion Source/WebKit/Shared/AuxiliaryProcess.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class AuxiliaryProcess : public IPC::Connection::Client, public IPC::MessageSend

#if ENABLE(CFPREFS_DIRECT_MODE)
virtual void preferenceDidUpdate(const String& domain, const String& key, const std::optional<String>& encodedValue);
void preferencesDidUpdate(HashMap<std::pair<String, String>, std::optional<String>>);
void preferencesDidUpdate(HashMap<String, std::optional<String>> domainlessPreferences, HashMap<std::pair<String, String>, std::optional<String>> preferences);
#endif

protected:
Expand Down
2 changes: 1 addition & 1 deletion Source/WebKit/Shared/AuxiliaryProcess.messages.in
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ messages -> AuxiliaryProcess NotRefCounted {

#if ENABLE(CFPREFS_DIRECT_MODE)
PreferenceDidUpdate(String domain, String key, std::optional<String> encodedValue)
PreferencesDidUpdate(HashMap<std::pair<String, String>, std::optional<String>> preferences)
PreferencesDidUpdate(HashMap<String, std::optional<String>> domainlessPreferences, HashMap<std::pair<String, String>, std::optional<String>> preferences)
#endif

#if OS(LINUX)
Expand Down
4 changes: 3 additions & 1 deletion Source/WebKit/Shared/Cocoa/AuxiliaryProcessCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,10 @@ static void initializeTimerCoalescingPolicy()
}

#if ENABLE(CFPREFS_DIRECT_MODE)
void AuxiliaryProcess::preferencesDidUpdate(HashMap<std::pair<String, String>, std::optional<String>> preferences)
void AuxiliaryProcess::preferencesDidUpdate(HashMap<String, std::optional<String>> domainlessPreferences, HashMap<std::pair<String, String>, std::optional<String>> preferences)
{
for (auto& [key, value] : domainlessPreferences)
preferenceDidUpdate(String(), key, value);
for (auto& [key, value] : preferences)
preferenceDidUpdate(key.first, key.second, value);
}
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -554,8 +554,8 @@ void AuxiliaryProcessProxy::didChangeThrottleState(ProcessThrottleState state)
return;
m_isSuspended = isNowSuspended;
#if ENABLE(CFPREFS_DIRECT_MODE)
if (!m_isSuspended && !m_preferencesUpdatedWhileSuspended.isEmpty())
send(Messages::AuxiliaryProcess::PreferencesDidUpdate(std::exchange(m_preferencesUpdatedWhileSuspended, { })), 0);
if (!m_isSuspended && (!m_domainlessPreferencesUpdatedWhileSuspended.isEmpty() || !m_preferencesUpdatedWhileSuspended.isEmpty()))
send(Messages::AuxiliaryProcess::PreferencesDidUpdate(std::exchange(m_domainlessPreferencesUpdatedWhileSuspended, { }), std::exchange(m_preferencesUpdatedWhileSuspended, { })), 0);
#endif
}

Expand Down
1 change: 1 addition & 0 deletions Source/WebKit/UIProcess/AuxiliaryProcessProxy.h
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ class AuxiliaryProcessProxy
ExtensionCapabilityGrantMap m_extensionCapabilityGrants;
#endif
#if ENABLE(CFPREFS_DIRECT_MODE)
HashMap<String, std::optional<String>> m_domainlessPreferencesUpdatedWhileSuspended;
HashMap<std::pair<String /* domain */, String /* key */>, std::optional<String>> m_preferencesUpdatedWhileSuspended;
#endif
};
Expand Down
9 changes: 6 additions & 3 deletions Source/WebKit/UIProcess/Cocoa/AuxiliaryProcessProxyCocoa.mm
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,12 @@
#if ENABLE(CFPREFS_DIRECT_MODE)
void AuxiliaryProcessProxy::notifyPreferencesChanged(const String& domain, const String& key, const std::optional<String>& encodedValue)
{
if (m_isSuspended)
m_preferencesUpdatedWhileSuspended.set(std::pair { domain, key }, encodedValue);
else
if (m_isSuspended) {
if (domain.isNull())
m_domainlessPreferencesUpdatedWhileSuspended.set(key, encodedValue);
else
m_preferencesUpdatedWhileSuspended.set(std::pair { domain , key }, encodedValue);
} else
send(Messages::AuxiliaryProcess::PreferenceDidUpdate(domain, key, encodedValue), 0);
}
#endif
Expand Down

0 comments on commit 168c94d

Please sign in to comment.