⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 271190 in webkit


Ignore:
Timestamp:
Jan 5, 2021, 7:27:09 PM (6 years ago)
Author:
Aditya Keerthi
Message:

REGRESSION (r261157): Crash in WKSelectPopover when running as iPhone app on iPad
​https://bugs.webkit.org/show_bug.cgi?id=220065
<rdar://problem/71932792>

Reviewed by Darin Adler.

Source/WebKit:

r261157 changed how WebKit determined the user interface idiom for
WKWebViews that were created in daemons. Since daemons do not create
UIApplications, WebKit cannot use UIDevice in these instances. Instead,
the user interface idiom determination logic was updated to include
a check for device hardware, using MobileGestalt, in cases where no
UIApplication was created.

Since WebKit only determines the user interface idiom once (then
storing the obtained value), the added determination logic breaks
down for iPhone apps on iPad. Consider the following sequence of
events, eventually leading to a crash when interacting with a
<select> element:

  1. A WKWebView is created prior to UIApplication initialization. This can be achieved by creating one in another class's "load" method.
  1. Since the app is physically running on an iPad, WebKit determines the user interface idiom to be "iPad" and saves this information.
  1. Once the app actually launches, UIApplication is initialized. Since this is an iPhone app on iPad, the actual user interface idiom is "iPhone". However, WebKit does not know this, since it uses the saved user interface idiom from (2).
  1. When tapping a <select> element, WebKit checks its saved idiom, an attempts to present a UIPopoverController (the standard behavior under the iPad idiom).
  1. However, since the actual idiom is iPhone, UIKit throws an NSInvalidArgumentException, since UIPopoverController should not be used when running under the iPhone idiom.

A simple fix for the crash would be to call into UIKit to check the
idiom each time it is required, rather than relying on the saved bit.
However, this does not address the more general issue, which is that
WebKit's saved idiom and the actual idiom can be out of sync.

Consequently, the approach taken is to update the saved idiom when
it changes. This should only occur when a UIApplication is initialized.
Hence, we listen for UIApplicationDidFinishLaunchingNotification,
and if the actual idiom is different from our saved idiom, we update the
saved idiom and also notify the WebProcess.

  • Shared/UserInterfaceIdiom.h:
  • Shared/UserInterfaceIdiom.mm:

(WebKit::updateCurrentUserInterfaceIdiom):

This method checks the actual idiom and updates the saved idiom if
they differ. Returns true only if the idiom was updated.

  • UIProcess/Cocoa/WebProcessPoolCocoa.mm:

(WebKit::WebProcessPool::registerNotificationObservers):

Listen for UIApplicationDidFinishLaunchingNotification if the idiom
can change (UIApplication is uninitialized). Then, if the actual
and saved idiom differ, notify all WebProcesses of the change.

(WebKit::WebProcessPool::unregisterNotificationObservers):

  • UIProcess/WebProcessPool.h:
  • WebProcess/WebProcess.h:
  • WebProcess/WebProcess.messages.in:
  • WebProcess/cocoa/WebProcessCocoa.mm:

(WebKit::WebProcess::userInterfaceIdiomDidChange):

Update the saved idiom using the information from the UIProcess.

Tools:

Added an API test to exercise the previously crashing, and now fixed,
codepath. Note that the test will trivially pass on iPhones, since the
crash only occurs on iPads. See below for an explanation of how the
test functions on iPads.

  • TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
  • TestWebKitAPI/Tests/ios/UserInterfaceIdiomUpdate.mm: Added.

(TestWebKitAPI::TEST):

Since TestWebKitAPI is not an application, [UIApplication sharedApplication]
is nil when the test creates a WKWebView. Consequently, when the WKWebView
is created, WebKit uses the device hardware information to determine the user
interface idiom (iPad). Now, before the test interacts with the WKWebView,
the user interface idiom is changed to an iPhone and the UIApplication is
initialized. Note that the UIApplicationDidFinishLaunchingNotification
is manually posted, since there is no UIApplicationDelegate.

Without the changes in this patch, the test will crash, since the user
interface idiom maintained by WebKit would not be updated, and would
remain iPad rather than iPhone. Hence, when focusing a select element, the test
would attempt to present a UIPopoverController. However, since the idiom
is actually iPhone, UIKit will throw an NSInvalidArgumentException.

After the changes in this patch, the test does crash, as the change to the
user interface idiom after UIApplication creation is recognized by WebKit,
and the attempt to present a UIPopoverController under the iPhone idiom
does not occur.

Location:
trunk
Files:
1 added
10 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit/ChangeLog

    r271189 r271190  
     12021-01-05  Aditya Keerthi  <akeerthi@apple.com>
     2
     3        REGRESSION (r261157): Crash in WKSelectPopover when running as iPhone app on iPad
     4        https://bugs.webkit.org/show_bug.cgi?id=220065
     5        <rdar://problem/71932792>
     6
     7        Reviewed by Darin Adler.
     8
     9        r261157 changed how WebKit determined the user interface idiom for
     10        WKWebViews that were created in daemons. Since daemons do not create
     11        UIApplications, WebKit cannot use UIDevice in these instances. Instead,
     12        the user interface idiom determination logic was updated to include
     13        a check for device hardware, using MobileGestalt, in cases where no
     14        UIApplication was created.
     15
     16        Since WebKit only determines the user interface idiom once (then
     17        storing the obtained value), the added determination logic breaks
     18        down for iPhone apps on iPad. Consider the following sequence of
     19        events, eventually leading to a crash when interacting with a
     20        <select> element:
     21
     22        1. A WKWebView is created prior to UIApplication initialization. This
     23           can be achieved by creating one in another class's "load" method.
     24
     25        2. Since the app is physically running on an iPad, WebKit determines
     26           the user interface idiom to be "iPad" and saves this information.
     27
     28        3. Once the app actually launches, UIApplication is initialized.
     29           Since this is an iPhone app on iPad, the actual user interface
     30           idiom is "iPhone". However, WebKit does not know this, since
     31           it uses the saved user interface idiom from (2).
     32
     33        4. When tapping a <select> element, WebKit checks its saved idiom,
     34           an attempts to present a UIPopoverController (the standard behavior
     35           under the iPad idiom).
     36
     37        5. However, since the actual idiom is iPhone, UIKit throws an
     38           NSInvalidArgumentException, since UIPopoverController
     39           should not be used when running under the iPhone idiom.
     40
     41        A simple fix for the crash would be to call into UIKit to check the
     42        idiom each time it is required, rather than relying on the saved bit.
     43        However, this does not address the more general issue, which is that
     44        WebKit's saved idiom and the actual idiom can be out of sync.
     45
     46        Consequently, the approach taken is to update the saved idiom when
     47        it changes. This should only occur when a UIApplication is initialized.
     48        Hence, we listen for UIApplicationDidFinishLaunchingNotification,
     49        and if the actual idiom is different from our saved idiom, we update the
     50        saved idiom and also notify the WebProcess.
     51
     52        * Shared/UserInterfaceIdiom.h:
     53        * Shared/UserInterfaceIdiom.mm:
     54        (WebKit::updateCurrentUserInterfaceIdiom):
     55
     56        This method checks the actual idiom and updates the saved idiom if
     57        they differ. Returns true only if the idiom was updated.
     58
     59        * UIProcess/Cocoa/WebProcessPoolCocoa.mm:
     60        (WebKit::WebProcessPool::registerNotificationObservers):
     61
     62        Listen for UIApplicationDidFinishLaunchingNotification if the idiom
     63        can change (UIApplication is uninitialized). Then, if the actual
     64        and saved idiom differ, notify all WebProcesses of the change.
     65
     66        (WebKit::WebProcessPool::unregisterNotificationObservers):
     67        * UIProcess/WebProcessPool.h:
     68        * WebProcess/WebProcess.h:
     69        * WebProcess/WebProcess.messages.in:
     70        * WebProcess/cocoa/WebProcessCocoa.mm:
     71        (WebKit::WebProcess::userInterfaceIdiomDidChange):
     72
     73        Update the saved idiom using the information from the UIProcess.
     74
    1752021-01-05  Chris Dumez  <cdumez@apple.com>
    276

  • trunk/Source/WebKit/Shared/UserInterfaceIdiom.h

    r266804 r271190  
    3232bool currentUserInterfaceIdiomIsPadOrMac();
    3333void setCurrentUserInterfaceIdiomIsPadOrMac(bool);
     34bool updateCurrentUserInterfaceIdiom();
    3435
    3536}

  • trunk/Source/WebKit/Shared/UserInterfaceIdiom.mm

    r266804 r271190  
    8080}
    8181
     82bool updateCurrentUserInterfaceIdiom()
     83{
     84    bool isPad = userInterfaceIdiomIsPad();
     85    if (currentUserInterfaceIdiomIsPadOrMac() == isPad)
     86        return false;
     87
     88    setCurrentUserInterfaceIdiomIsPadOrMac(isPad);
     89    return true;
     90}
     91
    8292}
    8393

  • trunk/Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm

    r270723 r271190  
    681681    }];
    682682#endif
     683    if (![UIApplication sharedApplication]) {
     684        m_applicationLaunchObserver = [[NSNotificationCenter defaultCenter] addObserverForName:UIApplicationDidFinishLaunchingNotification object:nil queue:[NSOperationQueue currentQueue] usingBlock:^(NSNotification *notification) {
     685            if (WebKit::updateCurrentUserInterfaceIdiom()) {
     686                auto isPadOrMac = WebKit::currentUserInterfaceIdiomIsPadOrMac();
     687                sendToAllProcesses(Messages::WebProcess::UserInterfaceIdiomDidChange(isPadOrMac));
     688            }
     689        }];
     690    }
    683691#endif
    684692
    … …  
    712720#if PLATFORM(IOS_FAMILY)
    713721    [[NSNotificationCenter defaultCenter] removeObserver:m_accessibilityEnabledObserver.get()];
     722    [[NSNotificationCenter defaultCenter] removeObserver:m_applicationLaunchObserver.get()];
    714723#endif
    715724

  • trunk/Source/WebKit/UIProcess/WebProcessPool.h

    r271162 r271190  
    660660    RetainPtr<NSObject> m_activationObserver;
    661661    RetainPtr<NSObject> m_accessibilityEnabledObserver;
     662    RetainPtr<NSObject> m_applicationLaunchObserver;
    662663#endif
    663664

  • trunk/Source/WebKit/WebProcess/WebProcess.h

    r270720 r271190  
    537537
    538538#if PLATFORM(IOS_FAMILY)
     539    void userInterfaceIdiomDidChange(bool);
     540
    539541    bool shouldFreezeOnSuspension() const;
    540542    void updateFreezerStatus();

  • trunk/Source/WebKit/WebProcess/WebProcess.messages.in

    r269807 r271190  
    121121#endif
    122122
     123#if PLATFORM(IOS_FAMILY)
     124    UserInterfaceIdiomDidChange(bool isPadOrMac)
     125#endif
     126
    123127#if PLATFORM(IOS_FAMILY) && !PLATFORM(MACCATALYST)
    124128    BacklightLevelDidChange(float backlightLevel)

  • trunk/Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm

    r271038 r271190  
    903903}
    904904
     905void WebProcess::userInterfaceIdiomDidChange(bool isPadOrMac)
     906{
     907    WebKit::setCurrentUserInterfaceIdiomIsPadOrMac(isPadOrMac);
     908}
     909
    905910bool WebProcess::shouldFreezeOnSuspension() const
    906911{

  • trunk/Tools/ChangeLog

    r271182 r271190  
     12021-01-05  Aditya Keerthi  <akeerthi@apple.com>
     2
     3        REGRESSION (r261157): Crash in WKSelectPopover when running as iPhone app on iPad
     4        https://bugs.webkit.org/show_bug.cgi?id=220065
     5        <rdar://problem/71932792>
     6
     7        Reviewed by Darin Adler.
     8
     9        Added an API test to exercise the previously crashing, and now fixed,
     10        codepath. Note that the test will trivially pass on iPhones, since the
     11        crash only occurs on iPads. See below for an explanation of how the
     12        test functions on iPads.
     13
     14        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
     15        * TestWebKitAPI/Tests/ios/UserInterfaceIdiomUpdate.mm: Added.
     16        (TestWebKitAPI::TEST):
     17
     18        Since TestWebKitAPI is not an application, [UIApplication sharedApplication]
     19        is nil when the test creates a WKWebView. Consequently, when the WKWebView
     20        is created, WebKit uses the device hardware information to determine the user
     21        interface idiom (iPad). Now, before the test interacts with the WKWebView,
     22        the user interface idiom is changed to an iPhone and the UIApplication is
     23        initialized. Note that the UIApplicationDidFinishLaunchingNotification
     24        is manually posted, since there is no UIApplicationDelegate.
     25
     26        Without the changes in this patch, the test will crash, since the user
     27        interface idiom maintained by WebKit would not be updated, and would
     28        remain iPad rather than iPhone. Hence, when focusing a select element, the test
     29        would attempt to present a UIPopoverController. However, since the idiom
     30        is actually iPhone, UIKit will throw an NSInvalidArgumentException.
     31
     32        After the changes in this patch, the test does crash, as the change to the
     33        user interface idiom after UIApplication creation is recognized by WebKit,
     34        and the attempt to present a UIPopoverController under the iPhone idiom
     35        does not occur.
     36
    1372021-01-05  Jonathan Bedard  <jbedard@apple.com>
    238

  • trunk/Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj

    r271154 r271190  
    11041104                E5036F78211BC25400BFDBE2 /* color-drop.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = E5036F77211BC22800BFDBE2 /* color-drop.html */; };
    11051105                E589183C252BC90A0041DED5 /* DateTimeInputsAccessoryViewTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = E589183B252BC90A0041DED5 /* DateTimeInputsAccessoryViewTests.mm */; };
     1106                E5AA42F2259128AE00410A3D /* UserInterfaceIdiomUpdate.mm in Sources */ = {isa = PBXBuildFile; fileRef = E5AA42F1259128AE00410A3D /* UserInterfaceIdiomUpdate.mm */; };
    11061107                E5AA8D1D25151CC60051CC45 /* DateInputTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = E5AA8D1C25151CC60051CC45 /* DateInputTests.mm */; };
    11071108                EB230D40245E727900C66AD1 /* IDBCheckpointWAL.mm in Sources */ = {isa = PBXBuildFile; fileRef = EB230D3E245E726300C66AD1 /* IDBCheckpointWAL.mm */; };
    … …  
    28482849                E5036F77211BC22800BFDBE2 /* color-drop.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = "color-drop.html"; sourceTree = "<group>"; };
    28492850                E589183B252BC90A0041DED5 /* DateTimeInputsAccessoryViewTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = DateTimeInputsAccessoryViewTests.mm; sourceTree = "<group>"; };
     2851                E5AA42F1259128AE00410A3D /* UserInterfaceIdiomUpdate.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = UserInterfaceIdiomUpdate.mm; sourceTree = "<group>"; };
    28502852                E5AA8D1C25151CC60051CC45 /* DateInputTests.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = DateInputTests.mm; sourceTree = "<group>"; };
    28512853                EB230D3D245E722E00C66AD1 /* IDBCheckpointWAL.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = IDBCheckpointWAL.html; sourceTree = "<group>"; };
    … …  
    36643666                                F46849BD1EEF58E400B937FE /* UIPasteboardTests.mm */,
    36653667                                F402F56B23ECC2FB00865549 /* UIWKInteractionViewProtocol.mm */,
     3668                                E5AA42F1259128AE00410A3D /* UserInterfaceIdiomUpdate.mm */,
    36663669                                F43E3BBE20DADA1E00A4E7ED /* WKScrollViewTests.mm */,
    36673670                                514958BD1F7427AC00E87BAD /* WKWebViewAutofillTests.mm */,
    … …  
    55605563                                7C882E0A1C80C764006BF731 /* UserContentWorld.mm in Sources */,
    55615564                                7CCB99211D3B41F6003922F6 /* UserInitiatedActionInNavigationAction.mm in Sources */,
     5565                                E5AA42F2259128AE00410A3D /* UserInterfaceIdiomUpdate.mm in Sources */,
    55625566                                7CCE7F171A411AE600447C4C /* UserMedia.cpp in Sources */,
    55635567                                0799C3491EBA2D7B003B7532 /* UserMediaDisabled.mm in Sources */,
Note: See TracChangeset for help on using the changeset viewer.