Bug 44483 – [chromium] fix the platform result fallback order on mac/win

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tony Chang]

[chromium] fix the platform result fallback order on mac/win

[Tony Chang]

Created attachment 65206 [details]
Patch

[Tony Chang]

The previous code happened to work on Linux because the fallback order on linux was hardcoded.  On Mac and Win, it's based on the port name.  To match existing behavior, we need to pass in null for port name.

[Tony Chang]

To clarify some more, the bug was that --platform 'google-chrome-win' wasn't including chromium-win-xp or chromium-win-vista.  This fixes it so the port name will still be chromium-win-{xp,vista}, even though we insert the google-chrome-win path in the baseline search path.

[Eric Seidel (no email)]

I'm still confused why None is required here.

[Tony Chang]

(In reply to comment #4)
> I'm still confused why None is required here.

None causes us to assign the default port name:
http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py#L47
http://trac.webkit.org/browser/trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py#L44

For windows, this is important because that's where -xp or -vista comes from.

[Eric Seidel (no email)]

Comment on attachment 65206 [details]
Patch

So these ports end up maskerading as win or mac or whatever?  Or I guess they get the chromium through inheriting from the Chromium port?

[Tony Chang]

(In reply to comment #6)
> (From update of attachment 65206 [details])
> So these ports end up maskerading as win or mac or whatever?  Or I guess they get the chromium through inheriting from the Chromium port?

Yes, these ports are identical to the chromium-win or chromium-mac ports.  We just want an extra layout test directory to check first for results.

[Eric Seidel (no email)]

Comment on attachment 65206 [details]
Patch

I'm still a bit confused, but I trust you.

[Dirk Pranke]

Comment on attachment 65206 [details]
Patch

Good catch. LGTM (although I'm not a reviewer).

[Tony Chang]

Comment on attachment 65206 [details]
Patch

Clearing flags on attachment: 65206

Committed r65944: <http://trac.webkit.org/changeset/65944>

[Tony Chang]

All reviewed patches have been landed.  Closing bug.