Summary: | build-webkit: add support for --progress-tag switch | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
Component: | Tools / Tests | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | abarth, aroben, eric, ossy, webkit.review.bot, yael | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Other | ||||||||
OS: | OS X 10.5 | ||||||||
Bug Depends on: | 36961, 48184, 48210 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2010-10-23 06:39:50 PDT
Created attachment 71639 [details]
Patch v1
Committed r70413: <http://trac.webkit.org/changeset/70413> 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 (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. Comment on attachment 71639 [details]
Patch v1
remove r+ from landed and rolled-out patch
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.
(In reply to comment #6) > Before this patch: > After this patch: I mean before/after the original patch. 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. 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 ? Comment on attachment 71696 [details]
proposed buildfix
r=me. This is obviously correct because RenderMeter derives from RenderIndicator.
(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. (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. (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 (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 |