Bug 132442

Summary: Fullscreen UI does not appear after WebProcess has crashed
Product: WebKit Reporter: Jeremy Jones <jeremyj-wk>
Component: WebKit2Assignee: Jeremy Jones <jeremyj-wk>
Status: RESOLVED FIXED    
Severity: Normal CC: buildbot, commit-queue, eric.carlson, glenn, jer.noble, jonlee, philipj, rniwa, sergio, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: iPhone / iPad   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2
none
Patch
none
Patch for landing none

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>