1275541 - Fix the name conflict of macro GetCurrentTime() in WinBas.h and MediaStream::GetCurrentTime()

Status: RESOLVED FIXED

(Core :: Audio/Video: MediaStreamGraph, defect, P1)

Description

[Tzuhao Kuo [:kaku]]

Follow bug 1235301 comment 73.

In the try of bug 1235301, Window does not compile which is caused by the name conflict of macro GetCurrentTime() in WinBas.h and MediaStream::GetCurrentTime().

The failed try job is: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0fa6727ad16c&selectedJob=20335318

Comment 1

[Tzuhao Kuo [:kaku]]

Attachment: MozReview Request: Bug 1275541 - undefine GetCurrentTime; r?jesup

Review commit: https://reviewboard.mozilla.org/r/55078/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/55078/

Comment 2

[Paul Adenot (:padenot)]

Comment on attachment 8756298 [details]
MozReview Request: Bug 1275541 - undefine GetCurrentTime; r?jesup

https://reviewboard.mozilla.org/r/55078/#review51732

Stealing the review, this is trivial and we've got the same fix in other locations of the code.

Comment 3

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8756298 [details]
MozReview Request: Bug 1275541 - undefine GetCurrentTime; r?jesup

https://reviewboard.mozilla.org/r/55078/#review51734

Comment 4

[Tzuhao Kuo [:kaku]]

Try1: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4800b1d6e4ed
Try2: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8c55de20682d
I don't know why tests do not run in Win8 in the first try so I have a second try and all look good.

Thanks for the reviews!

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ebb8ec09a40a

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ebb8ec09a40a

Comment 7

[Tzuhao Kuo [:kaku]]

Personally I think putting the "#ifndef GetCurrentTime  #define GetCurrentTime  #endif" everywhere is somewhat annoying and it's not even obvious where to put this period of code since it depends on the include order.

Does bug 834505 a good solution to get rid of all the fix? or how about we just rename the MediaStream::GetCurrentTime()?