ui/base/resource: Roll our own version of ReadBigEndian() function.

ui/base/resource: Remove dependency on net's big_endian implementation.

To remove this dependency we ended up moving big_endian* to base/, since besides ui/,
big_endian is also used by other top-level modules: chrome/utility/, cloud_print, media/cast

This way we removed one more net dependency from ui/base/.

BUG=299841
TEST=ui_unittests
R=tony@chromium.org,mark@chromium.org,hclam@chromium.org,vitalybuka@chromium.org
TBR=ben@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=253510

[tfarina, 2014-02-12 02:08:21]

https://codereview.chromium.org/145873006/diff/1/ui/base/resource/resource_bu...
File ui/base/resource/resource_bundle.cc (right):

https://codereview.chromium.org/145873006/diff/1/ui/base/resource/resource_bu...
ui/base/resource/resource_bundle.cc:764: uint32_t ReadBigEndian32(const void*
ptr) {
Where I should put this function?

https://codereview.chromium.org/145873006/diff/1/ui/base/resource/resource_bu...
ui/base/resource/resource_bundle.cc:764: uint32_t ReadBigEndian32(const void*
ptr) {
I copied this function from ppapi/tests/test_truetype_font.cc

https://codereview.chromium.org/145873006/diff/1/ui/base/resource/resource_bu...
ui/base/resource/resource_bundle.cc:764: uint32_t ReadBigEndian32(const void*
ptr) {
There is another one in content/browser/speech/chunked_byte_buffer.cc and one in
sql/test/test_helpers.cc

[tony, 2014-02-12 02:23:02]

https://codereview.chromium.org/145873006/diff/1/ui/base/resource/resource_bu...
File ui/base/resource/resource_bundle.cc (right):

https://codereview.chromium.org/145873006/diff/1/ui/base/resource/resource_bu...
ui/base/resource/resource_bundle.cc:764: uint32_t ReadBigEndian32(const void*
ptr) {
On 2014/02/12 02:08:22, tfarina wrote:
> Where I should put this function?

Probably somewhere in base since it's used in so many places.  I would ask one
of the base OWNERS if they think that is appropriate.  Maybe in base/numerics,
although that seems to be more for security sensitive code?

[tfarina, 2014-02-12 02:27:40]

On 2014/02/12 02:23:02, tony wrote:
>
https://codereview.chromium.org/145873006/diff/1/ui/base/resource/resource_bu...
> ui/base/resource/resource_bundle.cc:764: uint32_t ReadBigEndian32(const void*
> ptr) {
> On 2014/02/12 02:08:22, tfarina wrote:
> > Where I should put this function?
> 
> Probably somewhere in base since it's used in so many places.  I would ask one
> of the base OWNERS if they think that is appropriate.  Maybe in base/numerics,
> although that seems to be more for security sensitive code?

Among mark, darin, brett, willchan, albert and nico, who do you think would be
the best to ask this (based on the code and time availability)?

[tony, 2014-02-12 02:29:43]

On 2014/02/12 02:27:40, tfarina wrote:
> On 2014/02/12 02:23:02, tony wrote:
> >
>
https://codereview.chromium.org/145873006/diff/1/ui/base/resource/resource_bu...
> > ui/base/resource/resource_bundle.cc:764: uint32_t ReadBigEndian32(const
void*
> > ptr) {
> > On 2014/02/12 02:08:22, tfarina wrote:
> > > Where I should put this function?
> > 
> > Probably somewhere in base since it's used in so many places.  I would ask
one
> > of the base OWNERS if they think that is appropriate.  Maybe in
base/numerics,
> > although that seems to be more for security sensitive code?
> 
> Among mark, darin, brett, willchan, albert and nico, who do you think would be
> the best to ask this (based on the code and time availability)?

I would ask willchan because the code is used in net/.

[tfarina, 2014-02-12 02:40:50]

Will, Tony suggested I add you to review PS1.

We want to know your opinion if we can put ReadBigEndian() in base/.

Please, see our discussion and the code, for why we may need to put it there.

Thanks.

[tfarina, 2014-02-16 14:17:14]

Mark, I moved it to src/base.

[Mark Mentovai, 2014-02-18 17:14:30]

https://codereview.chromium.org/145873006/diff/140001/base/big_endian.h
File base/big_endian.h (right):

https://codereview.chromium.org/145873006/diff/140001/base/big_endian.h#newco...
base/big_endian.h:54: BigEndianReader(const void* buf, size_t len);
I would be more comfortable if buf were typed (const char*?). Same on line 80
(char*?). This might put a casting onus on callers—how many callers would need
to be changed if the interface changed to char*?

(Reading through some of the rest of this change, I see that some users of
BigEndianReader/Writer already cast to char*.)

[tfarina, 2014-02-23 03:45:00]

PTAL.

+Alpha for media/cast/ changes!

https://codereview.chromium.org/145873006/diff/140001/base/big_endian.h
File base/big_endian.h (right):

https://codereview.chromium.org/145873006/diff/140001/base/big_endian.h#newco...
base/big_endian.h:54: BigEndianReader(const void* buf, size_t len);
On 2014/02/18 17:14:30, Mark Mentovai wrote:
> I would be more comfortable if buf were typed (const char*?). Same on line 80
> (char*?). This might put a casting onus on callers—how many callers would need
> to be changed if the interface changed to char*?
> 
> (Reading through some of the rest of this change, I see that some users of
> BigEndianReader/Writer already cast to char*.)

Done.

[Mark Mentovai, 2014-02-23 05:21:05]

Check the Windows try bot. The other bots are satisfied.

[tfarina, 2014-02-23 14:56:41]

On 2014/02/23 05:21:05, Mark Mentovai wrote:
> Check the Windows try bot. The other bots are satisfied.

It was the targets I forgot to build (gcp20_device and gcp20_device_unittests).
Done. PTAL.

[Mark Mentovai, 2014-02-23 16:40:30]

LGTM

[Alpha Left Google, 2014-02-24 19:22:36]

media/cast LGTM if it compiles on all platforms. I noticed the removal of
reinterpret_cast from uint8* to unsigned char* so that might cause problems.

[Alpha Left Google, 2014-02-24 19:22:37]

media/cast LGTM if it compiles on all platforms. I noticed the removal of
reinterpret_cast from uint8* to unsigned char* so that might cause problems.

[tony, 2014-02-24 19:28:19]

ui/base/resource LGTM

[tfarina, 2014-02-24 19:32:19]

+Vitaly, for cloud_print/ OWNERS.

[tfarina, 2014-02-24 19:35:32]

I updated the CL description to actually reflect the code changes here.

[Alpha Left Google, 2014-02-24 20:07:29]

Sorry my bad. The reinterpret_cast is there. So LGTM.

[Vitaly Buka (NO REVIEWS), 2014-02-24 20:18:35]

lgtm

[tfarina, 2014-02-26 02:54:23]

Remove some unused big_endian.h includes.

https://codereview.chromium.org/145873006/diff/620001/cloud_print/gcp20/proto...
File cloud_print/gcp20/prototype/dns_sd_server.cc (right):

https://codereview.chromium.org/145873006/diff/620001/cloud_print/gcp20/proto...
cloud_print/gcp20/prototype/dns_sd_server.cc:10: #include "base/big_endian.h"
note to myself: remove, unused?

https://codereview.chromium.org/145873006/diff/620001/cloud_print/gcp20/proto...
cloud_print/gcp20/prototype/dns_sd_server.cc:10: #include "base/big_endian.h"
Done.

https://codereview.chromium.org/145873006/diff/620001/media/cast/transport/rt...
File media/cast/transport/rtp_sender/rtp_sender.cc (right):

https://codereview.chromium.org/145873006/diff/620001/media/cast/transport/rt...
media/cast/transport/rtp_sender/rtp_sender.cc:7: #include "base/big_endian.h"
note to myself: remove, unused?

https://codereview.chromium.org/145873006/diff/620001/media/cast/transport/rt...
media/cast/transport/rtp_sender/rtp_sender.cc:7: #include "base/big_endian.h"
Done.

https://codereview.chromium.org/145873006/diff/620001/net/dns/dns_transaction...
File net/dns/dns_transaction_unittest.cc (right):

https://codereview.chromium.org/145873006/diff/620001/net/dns/dns_transaction...
net/dns/dns_transaction_unittest.cc:7: #include "base/big_endian.h"
note to myself: remove, unused?

https://codereview.chromium.org/145873006/diff/620001/net/dns/dns_transaction...
net/dns/dns_transaction_unittest.cc:7: #include "base/big_endian.h"
Done.

https://codereview.chromium.org/145873006/diff/620001/net/dns/record_rdata.h
File net/dns/record_rdata.h (right):

https://codereview.chromium.org/145873006/diff/620001/net/dns/record_rdata.h#...
net/dns/record_rdata.h:12: #include "base/big_endian.h"
note to myself: remove, unused?

https://codereview.chromium.org/145873006/diff/620001/net/dns/record_rdata.h#...
net/dns/record_rdata.h:12: #include "base/big_endian.h"
Done.

[tfarina, 2014-02-26 03:10:05]

The CQ bit was checked by tfarina@chromium.org

[tfarina, 2014-02-26 03:10:39]

TBRing Ben for the rest!

[Paweł Hajdan Jr., 2014-02-26 04:39:41]

The CQ bit was unchecked by phajdan.jr@chromium.org

[commit-bot: I haz the power, 2014-02-26 04:45:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/145873006/800001

[Paweł Hajdan Jr., 2014-02-26 05:29:46]

The CQ bit was checked by phajdan.jr@chromium.org

[commit-bot: I haz the power, 2014-02-26 05:41:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/tfarina@chromium.org/145873006/800001

[commit-bot: I haz the power, 2014-02-26 18:15:01]

Change committed as 253510