Bug 137845 – Use is<>() / downcast<>() for all remaining RenderObject subclasses

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Chris Dumez]

Use is<>() / downcast<>() for all remaining RenderObject subclasses.

[Chris Dumez]

Created attachment 240058 [details]
Patch

[Chris Dumez]

Created attachment 240066 [details]
Patch

[Chris Dumez]

Finally, all the RenderObjects are ported. Getting really close to killing TYPE_CASTS_BASE() macro.

[Darin Adler]

Comment on attachment 240066 [details]
Patch

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

> Source/WebCore/html/HTMLEmbedElement.cpp:63
> +        do {
>              n = n->parentNode();
> -        while (n && !n->hasTagName(objectTag));
> +        } while (n && !is<HTMLObjectElement>(*n));

Why did you add the braces? Also surprised you didn’t rename the “n” variable.

[Chris Dumez]

Comment on attachment 240066 [details]
Patch

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

>> Source/WebCore/html/HTMLEmbedElement.cpp:63
>> +        } while (n && !is<HTMLObjectElement>(*n));
> 
> Why did you add the braces? Also surprised you didn’t rename the “n” variable.

The style checker complained. Was the style script wrong? Are we supposed to omit them in this case?

I'll rename the variable.

[Chris Dumez]

Created attachment 240094 [details]
Patch

[Darin Adler]

Comment on attachment 240066 [details]
Patch

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

>>> Source/WebCore/html/HTMLEmbedElement.cpp:63
>>> +        } while (n && !is<HTMLObjectElement>(*n));
>> 
>> Why did you add the braces? Also surprised you didn’t rename the “n” variable.
> 
> The style checker complained. Was the style script wrong? Are we supposed to omit them in this case?
> 
> I'll rename the variable.

I’m not sure. The coding style document doesn’t say one way or another. I am kind of annoyed at the style script making rules that we didn’t write down first, but I suppose there’s a lot of that.

Since I personally prefer braces even for one line if statements, I suppose it would be good to have our strange if rule be only for if statements, and not for looping constructs.

[WebKit Commit Bot]

Attachment 240094 [details] did not pass style-queue:


ERROR: Source/WebCore/html/HTMLEmbedElement.cpp:63:  Semicolon defining empty statement for this loop. Use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 63 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Chris Dumez]

Created attachment 240095 [details]
Patch

[Chris Dumez]

(In reply to comment #7)
> Comment on attachment 240066 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=240066&action=review
> 
> >>> Source/WebCore/html/HTMLEmbedElement.cpp:63
> >>> +        } while (n && !is<HTMLObjectElement>(*n));
> >> 
> >> Why did you add the braces? Also surprised you didn’t rename the “n” variable.
> > 
> > The style checker complained. Was the style script wrong? Are we supposed to omit them in this case?
> > 
> > I'll rename the variable.
> 
> I’m not sure. The coding style document doesn’t say one way or another. I am
> kind of annoyed at the style script making rules that we didn’t write down
> first, but I suppose there’s a lot of that.
> 
> Since I personally prefer braces even for one line if statements, I suppose
> it would be good to have our strange if rule be only for if statements, and
> not for looping constructs.

Ok, I also prefer the braces in this particular case. I left the braces in but renamed the n variable to node.

[WebKit Commit Bot]

Comment on attachment 240095 [details]
Patch

Clearing flags on attachment: 240095

Committed r174875: <http://trac.webkit.org/changeset/174875>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.