Bug 132442 - Fullscreen UI does not appear after WebProcess has crashed
Summary: Fullscreen UI does not appear after WebProcess has crashed
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Jeremy Jones
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2014-05-01 15:25 PDT by Jeremy Jones
Modified: 2014-05-05 08:26 PDT (History)
11 users (show)

See Also:


Attachments
Patch (7.21 KB, patch)
2014-05-01 15:30 PDT, Jeremy Jones
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (508.50 KB, application/zip)
2014-05-01 16:29 PDT, Build Bot
no flags Details
Patch (7.22 KB, patch)
2014-05-02 10:51 PDT, Jeremy Jones
no flags Details | Formatted Diff | Diff
Patch for landing (7.22 KB, patch)
2014-05-02 14:30 PDT, Jeremy Jones
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jeremy Jones 2014-05-01 15:25:52 PDT
Fullscreen UI does not appear after WebProcess has crashed
Comment 1 Jeremy Jones 2014-05-01 15:30:18 PDT
Created attachment 230616 [details]
Patch
Comment 2 Darin Adler 2014-05-01 15:58:31 PDT
Comment on attachment 230616 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=230616&action=review

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:595
> +    m_playerViewController = nil;

Not retained?

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:596
> +    m_viewController = nil;

Not retained?

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:597
> +    m_window = nil;

Not retained?

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:599
> +    m_videoLayer = nil;

Not retained?

> Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:601
> +    m_videoLayerContainer = nil;

Not retained?

> Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.h:50
> +    void invalidate();

Should mark this virtual and override, and also private instead of public.
Comment 3 Build Bot 2014-05-01 16:29:22 PDT
Comment on attachment 230616 [details]
Patch

Attachment 230616 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5960897684045824

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html
Comment 4 Build Bot 2014-05-01 16:29:25 PDT
Created attachment 230626 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 5 Jeremy Jones 2014-05-02 10:48:58 PDT
(In reply to comment #2)
> (From update of attachment 230616 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=230616&action=review
> 
> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:595
> > +    m_playerViewController = nil;
> 
> Not retained?

Automatically:
    RetainPtr<AVPlayerViewController> m_playerViewController;

> 
> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:596
> > +    m_viewController = nil;
> 
> Not retained?

Automatically:
    RetainPtr<UIViewController> m_viewController;

> 
> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:597
> > +    m_window = nil;
> 
> Not retained?

Automatically:
    RetainPtr<UIWindow> m_window;

> 
> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:599
> > +    m_videoLayer = nil;
> 
> Not retained?

Automatically:
    RetainPtr<CALayer> m_videoLayer;

> 
> > Source/WebCore/platform/ios/WebVideoFullscreenInterfaceAVKit.mm:601
> > +    m_videoLayerContainer = nil;
> 
> Not retained?

Automatically:
    RetainPtr<WebAVVideoLayer> m_videoLayerContainer;

> 
> > Source/WebKit2/UIProcess/ios/WebVideoFullscreenManagerProxy.h:50
> > +    void invalidate();
> 
> Should mark this virtual and override, and also private instead of public.

Added 'virtual' and 'override'.  But invalidate() exists to be called from WebPageProxy::resetState() so it can't be private.
Comment 6 Jeremy Jones 2014-05-02 10:51:32 PDT
Created attachment 230673 [details]
Patch
Comment 7 Jeremy Jones 2014-05-02 14:30:44 PDT
Created attachment 230700 [details]
Patch for landing
Comment 8 WebKit Commit Bot 2014-05-02 14:43:17 PDT
Comment on attachment 230700 [details]
Patch for landing

Rejecting attachment 230700 [details] from commit-queue.

jeremyj-wk@apple.com does not have committer permissions according to http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/common/config/contributors.json.

- If you do not have committer rights please read http://webkit.org/coding/contributing.html for instructions on how to use bugzilla flags.

- If you have committer rights please correct the error in Tools/Scripts/webkitpy/common/config/contributors.json by adding yourself to the file (no review needed).  The commit-queue restarts itself every 2 hours.  After restart the commit-queue will correctly respect your committer rights.
Comment 9 WebKit Commit Bot 2014-05-02 16:11:45 PDT
Comment on attachment 230700 [details]
Patch for landing

Clearing flags on attachment: 230700

Committed r168205: <http://trac.webkit.org/changeset/168205>
Comment 10 WebKit Commit Bot 2014-05-02 16:11:55 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Jon Lee 2014-05-05 08:26:29 PDT
<rdar://problem/16208054>