Bug 48185 – build-webkit: add support for --progress-tag switch

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[David Kilzer (:ddkilzer)]

build-webkit: add support for --progress-tag switch

[David Kilzer (:ddkilzer)]

Created attachment 71639 [details]
Patch v1

[David Kilzer (:ddkilzer)]

Committed r70413: <http://trac.webkit.org/changeset/70413>

[WebKit Review Bot]

http://trac.webkit.org/changeset/70413 might have broken Leopard Intel Debug (Tests)
The following tests are not passing:
editing/spelling/context-menu-suggestions.html
editing/spelling/spellcheck-attribute.html
editing/spelling/spelling-backspace-between-lines.html
editing/spelling/spelling-contenteditable.html
editing/spelling/spelling-textarea.html
platform/mac/accessibility/attributed-string-includes-misspelled-with-selection.html
platform/mac/accessibility/misspelled-attributed-string.html

[Csaba Osztrogonác]

(In reply to comment #2)
> Committed r70413: <http://trac.webkit.org/changeset/70413>

It broke Qt minimal build with this error message:
(Unfortunately buildbot was dead.)

In file included from ../../../WebCore/html/HTMLMeterElement.cpp:32:
../../../WebCore/rendering/RenderMeter.h:35: error: expected class-name before ‘{’ token
../../../WebCore/rendering/RenderMeter.h: In function ‘WebCore::RenderMeter* WebCore::toRenderMeter(WebCore::RenderObject*)’:
../../../WebCore/rendering/RenderMeter.h:64: error: invalid static_cast from type ‘WebCore::RenderObject*’ to type ‘WebCore::RenderMeter*’
../../../WebCore/html/HTMLMeterElement.cpp: In member function ‘virtual WebCore::RenderObject* WebCore::HTMLMeterElement::createRenderer(WebCore::RenderArena*, WebCore::RenderStyle*)’:
../../../WebCore/html/HTMLMeterElement.cpp:54: error: cannot convert ‘WebCore::RenderMeter*’ to ‘WebCore::RenderObject*’ in return
g

It was rolled out by http://trac.webkit.org/changeset/70423 .
I'll check it later.

[Csaba Osztrogonác]

Comment on attachment 71639 [details]
Patch v1

remove r+ from landed and rolled-out patch

[Csaba Osztrogonác]

Created attachment 71696 [details]
proposed buildfix

Before this patch:
ENABLE_METER_TAG=1
ENABLE_PROGRESS_TAG=1

After this patch:
ENABLE_METER_TAG=1
ENABLE_PROGRESS_TAG=0

There were build fail, because METER_TAG depends on PROGRESS_TAG:
RenderMeter class inherited from RenderIndicator in rendering/RenderMeter.h
But after this patch RenderIndicator.h wasn't included in RenderMeter.h
It is only included undirectly by RenderProgress.h if PROGRESS_TAG is enabled.
I don't think if METER_TAG needs PROGRESS_TAG, but RenderIndicator class.

[Csaba Osztrogonác]

(In reply to comment #6)
> Before this patch:
> After this patch:
I mean before/after the original patch.

[Csaba Osztrogonác]

I think the original bug dependency set by sheriffbot was correct:
This bug depends on Bug 48210 -  REGRESSION(r70413): It broke Qt minimal build (Requested by Ossy on #webkit). and b48210 blocks this bug.

[Csaba Osztrogonác]

Yael, you added HTMLMeterElement in http://trac.webkit.org/changeset/59541.
What do you think about my proposed fix ?

All, what do you think if we should add default value for ENABLE_METER_TAG,
same as for ENABLE_PROGRESS_TAG in http://trac.webkit.org/changeset/70413 ?

[David Kilzer (:ddkilzer)]

Comment on attachment 71696 [details]
proposed buildfix

r=me.  This is obviously correct because RenderMeter derives from RenderIndicator.

[David Kilzer (:ddkilzer)]

(In reply to comment #9)
> All, what do you think if we should add default value for ENABLE_METER_TAG,
> same as for ENABLE_PROGRESS_TAG in http://trac.webkit.org/changeset/70413 ?

That's fine.  I didn't even realize meter-tag was missing a switch on build-webkit, too.

[Yael]

(In reply to comment #9)
> Yael, you added HTMLMeterElement in http://trac.webkit.org/changeset/59541.
> What do you think about my proposed fix ?
> 
Thanks for taking a quick action, Ossy :-)
You could also remove #include "RemderProgress.h". RenderMeter does not really depend on RenderProgress.

> All, what do you think if we should add default value for ENABLE_METER_TAG,
> same as for ENABLE_PROGRESS_TAG in http://trac.webkit.org/changeset/70413 ?
Is anyone doing this already? If not, I will do that tomorrow.

[Csaba Osztrogonác]

(In reply to comment #10)
> (From update of attachment 71696 [details])
> r=me.  This is obviously correct because RenderMeter derives from RenderIndicator.

Landed in http://trac.webkit.org/changeset/70440.
Additionally I removed needless #include "RemderProgress.h".

Original patch relanded in http://trac.webkit.org/changeset/70441

[Csaba Osztrogonác]

(In reply to comment #12)
> > All, what do you think if we should add default value for ENABLE_METER_TAG,
> > same as for ENABLE_PROGRESS_TAG in http://trac.webkit.org/changeset/70413 ?
> Is anyone doing this already? If not, I will do that tomorrow.

new bug filed on it: https://bugs.webkit.org/show_bug.cgi?id=48224