Bug 137594

Summary: Drop RENDER_OBJECT_TYPE_CASTS() for a lot of RenderObject subclasses
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Layout and RenderingAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, commit-queue, darin, kling, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 137424    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch none

Description Chris Dumez 2014-10-09 20:38:57 PDT
Drop RENDER_OBJECT_TYPE_CASTS() for RenderObject subclasses that no longer needed it or that required very few changes to do so. This will make sure developers start using is<>() / downcast<>() for those classes.
Comment 1 Chris Dumez 2014-10-09 20:41:02 PDT
Created attachment 239595 [details]
Patch
Comment 2 Darin Adler 2014-10-10 09:35:19 PDT
Comment on attachment 239595 [details]
Patch

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

> Source/WebCore/rendering/RenderVideo.cpp:284
>      RenderObject* parent = renderer->parent();
>      if (!parent)
> -        return 0;
> +        return nullptr;
>      
> -    RenderFullScreen* fullScreen = parent->isRenderFullScreen() ? toRenderFullScreen(parent) : 0;
> +    RenderFullScreen* fullScreen = is<RenderFullScreen>(*parent) ? downcast<RenderFullScreen>(parent) : nullptr;
>      if (!fullScreen)
> -        return 0;
> +        return nullptr;
>      
>      return fullScreen->placeholder();

We should write this:

    RenderObject* parent = renderer->parent();
    return is<RenderFullScreen>(parent) ? downcast<RenderFullScreen>(*parent).placeholder() : nullptr;

I think the two line version is better than the longer version.
Comment 3 Chris Dumez 2014-10-10 09:41:46 PDT
Created attachment 239629 [details]
Patch
Comment 4 WebKit Commit Bot 2014-10-10 10:23:24 PDT
Comment on attachment 239629 [details]
Patch

Rejecting attachment 239629 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.appspot.com', '--bot-id=webkit-cq-01', 'build', '--no-clean', '--no-update', '--build-style=release', '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit

Last 500 characters of output:
nCommand.dia -c /Volumes/Data/EWS/WebKit/Source/WebCore/editing/ReplaceNodeWithSpanCommand.cpp -o /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/ReplaceNodeWithSpanCommand.o

** BUILD FAILED **


The following build commands failed:
	CompileC /Volumes/Data/EWS/WebKit/WebKitBuild/WebCore.build/Release/WebCore.build/Objects-normal/x86_64/RenderVideo.o rendering/RenderVideo.cpp normal x86_64 c++ com.apple.compilers.llvm.clang.1_0.compiler
(1 failure)

Full output: http://webkit-queues.appspot.com/results/5076801827110912
Comment 5 Chris Dumez 2014-10-10 10:26:16 PDT
Created attachment 239635 [details]
Patch
Comment 6 WebKit Commit Bot 2014-10-10 11:02:39 PDT
Comment on attachment 239635 [details]
Patch

Clearing flags on attachment: 239635

Committed r174603: <http://trac.webkit.org/changeset/174603>
Comment 7 WebKit Commit Bot 2014-10-10 11:02:45 PDT
All reviewed patches have been landed.  Closing bug.