Closed Bug 1426898 Opened 2 years ago Closed 2 years ago

Stop including Char16.h everywhere

Categories

(Core :: MFBT, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox59 --- fixed

People

(Reporter: emk, Assigned: emk)

Details

Attachments

(2 files)

Because we do not need the char16_t emulation anymore.
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Attachment #8938695 - Flags: review?(jwalden+bmo)
Comment on attachment 8938695 [details]
Bug 1426898 - Stop including Char16.h everywhere.

https://reviewboard.mozilla.org/r/209284/#review215086

::: ipc/glue/FileDescriptor.h:17
(Diff revision 1)
>  #include "mozilla/UniquePtr.h"
>  
>  #ifdef XP_WIN
>  // Need the HANDLE typedef.
>  #include <winnt.h>
> +#include <cstdint>

Why is this addition necessary?  I don't see anything in this file that uses anything from this header at a glance.

::: media/gmp-clearkey/0.1/WMFUtils.h:29
(Diff revision 1)
>  #include <mfapi.h>
>  #include <mferror.h>
>  #include <mfobjects.h>
>  #include <mftransform.h>
>  #include <wmcodecdsp.h>
> +#include "mozilla/Attributes.h"

I would have guessed this file was imported, but I see MOZ_IMPLICITs in here, so clearly not.  (Or at least we've decided to make changes atop it, and there's no sign of a clean import-script anywhere in sight.)

::: xpcom/string/nsTDependentSubstring.cpp:8
(Diff revision 1)
>  /* This Source Code Form is subject to the terms of the Mozilla Public
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
> +#include "nsTDependentSubstring.h"
> +

I assume this addition isn't strictly necessary for this patch, just a driveby improvement?  If this really is necessary for some reason, I'd like to know why, just so I'm clear why we're doing this.
Attachment #8938695 - Flags: review?(jwalden+bmo) → review+
Comment on attachment 8938695 [details]
Bug 1426898 - Stop including Char16.h everywhere.

https://reviewboard.mozilla.org/r/209284/#review215086

> Why is this addition necessary?  I don't see anything in this file that uses anything from this header at a glance.

It is needed for std::intptr_t at line 57.

> I assume this addition isn't strictly necessary for this patch, just a driveby improvement?  If this really is necessary for some reason, I'd like to know why, just so I'm clear why we're doing this.

Yes, "mozilla/Char16.h" is sufficient for the purpose of this patch.
Attachment #8938694 - Flags: review?(rjesup)
Comment on attachment 8938694 [details]
Bug 1426898 - Fix a bug of a local patch for libyuv.

https://reviewboard.mozilla.org/r/209282/#review215096
Attachment #8938694 - Flags: review?(rjesup) → review+
Pushed by VYV03354@nifty.ne.jp:
https://hg.mozilla.org/integration/autoland/rev/6e072dd8d48f
Fix a bug of a local patch for libyuv. r=jesup
https://hg.mozilla.org/integration/autoland/rev/e9c5541886e8
Stop including Char16.h everywhere. r=Waldo
https://hg.mozilla.org/mozilla-central/rev/6e072dd8d48f
https://hg.mozilla.org/mozilla-central/rev/e9c5541886e8
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Comment on attachment 8938694 [details]
Bug 1426898 - Fix a bug of a local patch for libyuv.

https://reviewboard.mozilla.org/r/209282/#review215116
Attachment #8938694 - Flags: review?(sotaro.ikeda.g) → review+
You need to log in before you can comment on or make changes to this bug.