Bug 113294 - REGRESSION(r145279): Build fails while linking when MEDIA_STREAM is enabled
Summary: REGRESSION(r145279): Build fails while linking when MEDIA_STREAM is enabled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Regression
Depends on:
Blocks: 111729
  Show dependency treegraph
 
Reported: 2013-03-26 04:21 PDT by Carlos Garcia Campos
Modified: 2013-04-05 02:23 PDT (History)
10 users (show)

See Also:


Attachments
Patch (9.26 KB, patch)
2013-03-26 04:25 PDT, Carlos Garcia Campos
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Try to fix qt build (9.26 KB, patch)
2013-03-26 11:55 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2013-03-26 04:21:40 PDT
Due to undefined references to WebCore::JSRTCStatsResponse::nameGetter and WebCore::JSRTCStatsResponse::canGetItemsForName. In r145279 NamedGetter extended attribute is added to RTCStatsResponse.idl which makes the code generator to add prototypes for JSRTCStatsResponse::nameGetter and JSRTCStatsResponse::canGetItemsForName to the generated JSRTCStatsResponse header, but they are not implemented anywhere. We need to provide a SRTCStatsResponseCustom file with the implementation of both functions.
Comment 1 Carlos Garcia Campos 2013-03-26 04:25:31 PDT
Created attachment 195056 [details]
Patch
Comment 2 Early Warning System Bot 2013-03-26 04:30:50 PDT
Comment on attachment 195056 [details]
Patch

Attachment 195056 [details] did not pass qt-wk2-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17197985
Comment 3 Early Warning System Bot 2013-03-26 04:31:36 PDT
Comment on attachment 195056 [details]
Patch

Attachment 195056 [details] did not pass qt-ews (qt):
Output: http://webkit-commit-queue.appspot.com/results/17316108
Comment 4 Carlos Garcia Campos 2013-03-26 04:34:41 PDT
(In reply to comment #2)
> (From update of attachment 195056 [details])
> Attachment 195056 [details] did not pass qt-wk2-ews (qt):
> Output: http://webkit-commit-queue.appspot.com/results/17197985

It seems JSRTCStatsResponse.h is not generated when media stream is disabled, so I guess I just need to move the include inside the #if ENABLE(MEDIA_STREAM) block
Comment 5 Carlos Garcia Campos 2013-03-26 04:47:10 PDT
(In reply to comment #4)
> (In reply to comment #2)
> > (From update of attachment 195056 [details] [details])
> > Attachment 195056 [details] [details] did not pass qt-wk2-ews (qt):
> > Output: http://webkit-commit-queue.appspot.com/results/17197985
> 
> It seems JSRTCStatsResponse.h is not generated when media stream is disabled, so I guess I just need to move the include inside the #if ENABLE(MEDIA_STREAM) block

Could someone from qt confirm this? Maybe I should not include the JSRTCStatsResponseCustom.cpp file to the compilation for qt.
Comment 6 Harald Alvestrand 2013-03-26 05:55:09 PDT
Patch seems clean to me, but seems to address a symptom, not the cause.

Does this mean that the [NamedGetter] modifier can only be used with custom bindings for JSC?

If so, that seems like a problem with the implementation of NamedGetter.

Are there test cases one could build for this, so that it gets detected earlier (either add NamedGetter to the test set for the JSC compiler, or add a target that compiles MediaStream with JSC and runs on the bots)?
Comment 7 Carlos Garcia Campos 2013-03-26 06:22:39 PDT
(In reply to comment #6)
> Patch seems clean to me, but seems to address a symptom, not the cause.
> 
> Does this mean that the [NamedGetter] modifier can only be used with custom bindings for JSC?

I seems so, see all other JS*Custom files in bindings/js/
Comment 8 Carlos Garcia Campos 2013-03-26 11:55:39 PDT
Created attachment 195131 [details]
Try to fix qt build
Comment 9 Xan Lopez 2013-04-05 01:59:28 PDT
Comment on attachment 195131 [details]
Try to fix qt build

Since this is a build fix I think it makes sense to push it now. I agree that possibly the bindings generator should cope with this somehow (but I'm not an expert, so perhaps there are good reasons for this), but we can fix it after this patch lands. Also a BlackBerry EWS/buildbot is planned, so we should be able to catch this stuff in the near future.
Comment 10 Carlos Garcia Campos 2013-04-05 02:02:06 PDT
Comment on attachment 195131 [details]
Try to fix qt build

Thanks!
Comment 11 WebKit Commit Bot 2013-04-05 02:23:51 PDT
Comment on attachment 195131 [details]
Try to fix qt build

Clearing flags on attachment: 195131

Committed r147731: <http://trac.webkit.org/changeset/147731>
Comment 12 WebKit Commit Bot 2013-04-05 02:23:54 PDT
All reviewed patches have been landed.  Closing bug.