Remove mojo::Array.

Remove mojo::Array.

BUG=674766
Review-Url: https://codereview.chromium.org/2607063002
Cr-Commit-Position: refs/heads/master@{#442540}
Committed: https://chromium.googlesource.com/chromium/src/+/0e91f591193bd8616a44a2c307f58ca11255c62c

[Sam McNally, 2016-12-29 04:30:00]

The CQ bit was checked by sammc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-29 04:30:07]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-29 04:34:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-12-29 04:34:01]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Sam McNally, 2016-12-29 06:40:50]

Patchset #3 (id:40001) has been deleted

[Sam McNally, 2016-12-29 06:40:54]

Patchset #2 (id:20001) has been deleted

[Sam McNally, 2016-12-29 06:41:02]

Patchset #1 (id:1) has been deleted

[Sam McNally, 2017-01-05 23:28:52]

The CQ bit was checked by sammc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-05 23:29:26]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sam McNally, 2017-01-06 01:23:55]

Patchset #4 (id:120001) has been deleted

[Sam McNally, 2017-01-06 01:24:14]

Patchset #3 (id:100001) has been deleted

[Sam McNally, 2017-01-06 01:24:17]

Patchset #2 (id:80001) has been deleted

[Sam McNally, 2017-01-06 01:24:21]

Patchset #1 (id:60001) has been deleted

[Sam McNally, 2017-01-06 01:42:04]

sammc@chromium.org changed reviewers:
+ yzshen@chromium.org

[commit-bot: I haz the power, 2017-01-06 02:27:26]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-06 02:27:27]

Dry run: This issue passed the CQ dry run.

[yzshen1, 2017-01-06 19:38:38]

LGTM with a few nits

https://codereview.chromium.org/2607063002/diff/140001/components/arc/test/fa...
File components/arc/test/fake_app_instance.cc (right):

https://codereview.chromium.org/2607063002/diff/140001/components/arc/test/fa...
components/arc/test/fake_app_instance.cc:116:
std::vector<uint8_t>(png_data_as_string->begin(),
It would be nice to avoid such conversion by using same type in the user code
whenever possible. But I guess that is the responsibility of the owner of the
user code.

https://codereview.chromium.org/2607063002/diff/140001/content/browser/websoc...
File content/browser/websockets/websocket_manager_unittest.cc (right):

https://codereview.chromium.org/2607063002/diff/140001/content/browser/websoc...
content/browser/websockets/websocket_manager_unittest.cc:140: true,
blink::mojom::WebSocketMessageType::TEXT, std::move(data));
nit: it doesn't need std::move here. the parameter type is const&

https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
File
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc
(right):

https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc:23:
explicit PacketIOBuffer(std::vector<uint8_t> array);
Does it make sense to use &&?

https://codereview.chromium.org/2607063002/diff/140001/services/ui/public/cpp...
File services/ui/public/cpp/in_flight_change.cc (right):

https://codereview.chromium.org/2607063002/diff/140001/services/ui/public/cpp...
services/ui/public/cpp/in_flight_change.cc:160: property_name_, revert_value_ ?
&*revert_value_ : nullptr);
nit: it seems easier to read: &revert_value_.value()

https://codereview.chromium.org/2607063002/diff/140001/services/ui/public/cpp...
File services/ui/public/cpp/window_tree_client.cc (right):

https://codereview.chromium.org/2607063002/diff/140001/services/ui/public/cpp...
services/ui/public/cpp/window_tree_client.cc:1061: new_data ? &*new_data :
nullptr);
nit: &new_data.value()?

[Sam McNally, 2017-01-08 23:54:02]

The CQ bit was checked by sammc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-08 23:54:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-08 23:56:57]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-08 23:56:58]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Sam McNally, 2017-01-08 23:58:28]

The CQ bit was checked by sammc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-08 23:58:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sam McNally, 2017-01-08 23:58:58]

Patchset #2 (id:160001) has been deleted

[commit-bot: I haz the power, 2017-01-09 00:01:27]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-09 00:01:28]

Dry run: Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)

[Sam McNally, 2017-01-09 00:12:45]

The CQ bit was checked by sammc@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-09 00:13:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-09 03:07:16]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-09 03:07:17]

Dry run: This issue passed the CQ dry run.

[Sam McNally, 2017-01-09 23:15:14]

sammc@chromium.org changed reviewers:
+ dcheng@chromium.org, jam@chromium.org

[Sam McNally, 2017-01-09 23:15:15]

+jam for everything outside //mojo

+dcheng for type converters and struct traits

https://codereview.chromium.org/2607063002/diff/140001/content/browser/websoc...
File content/browser/websockets/websocket_manager_unittest.cc (right):

https://codereview.chromium.org/2607063002/diff/140001/content/browser/websoc...
content/browser/websockets/websocket_manager_unittest.cc:140: true,
blink::mojom::WebSocketMessageType::TEXT, std::move(data));
On 2017/01/06 19:38:37, yzshen1 wrote:
> nit: it doesn't need std::move here. the parameter type is const&

Done.

https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
File
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc
(right):

https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc:23:
explicit PacketIOBuffer(std::vector<uint8_t> array);
On 2017/01/06 19:38:38, yzshen1 wrote:
> Does it make sense to use &&?

Yes, but the style guide doesn't allow it.

https://codereview.chromium.org/2607063002/diff/140001/services/ui/public/cpp...
File services/ui/public/cpp/in_flight_change.cc (right):

https://codereview.chromium.org/2607063002/diff/140001/services/ui/public/cpp...
services/ui/public/cpp/in_flight_change.cc:160: property_name_, revert_value_ ?
&*revert_value_ : nullptr);
On 2017/01/06 19:38:38, yzshen1 wrote:
> nit: it seems easier to read: &revert_value_.value()

Done.

https://codereview.chromium.org/2607063002/diff/140001/services/ui/public/cpp...
File services/ui/public/cpp/window_tree_client.cc (right):

https://codereview.chromium.org/2607063002/diff/140001/services/ui/public/cpp...
services/ui/public/cpp/window_tree_client.cc:1061: new_data ? &*new_data :
nullptr);
On 2017/01/06 19:38:38, yzshen1 wrote:
> nit: &new_data.value()?

Done.

[yzshen1, 2017-01-09 23:20:25]

LGTM with one nit

https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
File
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc
(right):

https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc:23:
explicit PacketIOBuffer(std::vector<uint8_t> array);
On 2017/01/09 23:15:15, Sam McNally wrote:
> On 2017/01/06 19:38:38, yzshen1 wrote:
> > Does it make sense to use &&?
> 
> Yes, but the style guide doesn't allow it.

I feel that this falls in the "move constructor" category:
"Only use these to define move constructors and move assignment operators, and
for perfect forwarding." from "C++11 use in Chromium"

[Sam McNally, 2017-01-09 23:30:11]

https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
File
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc
(right):

https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc:23:
explicit PacketIOBuffer(std::vector<uint8_t> array);
On 2017/01/09 23:20:24, yzshen1 wrote:
> On 2017/01/09 23:15:15, Sam McNally wrote:
> > On 2017/01/06 19:38:38, yzshen1 wrote:
> > > Does it make sense to use &&?
> > 
> > Yes, but the style guide doesn't allow it.
> 
> I feel that this falls in the "move constructor" category:
> "Only use these to define move constructors and move assignment operators, and
> for perfect forwarding." from "C++11 use in Chromium"

A move constructor requires the input type to be the same as the type being
constructed.

[yzshen1, 2017-01-09 23:39:36]

On 2017/01/09 23:30:11, Sam McNally wrote:
>
https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
> File
>
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc
> (right):
> 
>
https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
>
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc:23:
> explicit PacketIOBuffer(std::vector<uint8_t> array);
> On 2017/01/09 23:20:24, yzshen1 wrote:
> > On 2017/01/09 23:15:15, Sam McNally wrote:
> > > On 2017/01/06 19:38:38, yzshen1 wrote:
> > > > Does it make sense to use &&?
> > > 
> > > Yes, but the style guide doesn't allow it.
> > 
> > I feel that this falls in the "move constructor" category:
> > "Only use these to define move constructors and move assignment operators,
and
> > for perfect forwarding." from "C++11 use in Chromium"
> 
> A move constructor requires the input type to be the same as the type being
> constructed.

Yeah, but there are quite a lot of places where we use a different type for a
"conversion". For example:
https://codesearch.chromium.org/chromium/src/base/optional.h?rcl=0&l=140

I feel that it is better than pass-by-value in this case.

[yzshen1, 2017-01-09 23:40:08]

On 2017/01/09 23:39:36, yzshen1 wrote:
> On 2017/01/09 23:30:11, Sam McNally wrote:
> >
>
https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
> > File
> >
>
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc
> > (right):
> > 
> >
>
https://codereview.chromium.org/2607063002/diff/140001/extensions/browser/api...
> >
>
extensions/browser/api/display_source/wifi_display/wifi_display_media_service_impl.cc:23:
> > explicit PacketIOBuffer(std::vector<uint8_t> array);
> > On 2017/01/09 23:20:24, yzshen1 wrote:
> > > On 2017/01/09 23:15:15, Sam McNally wrote:
> > > > On 2017/01/06 19:38:38, yzshen1 wrote:
> > > > > Does it make sense to use &&?
> > > > 
> > > > Yes, but the style guide doesn't allow it.
> > > 
> > > I feel that this falls in the "move constructor" category:
> > > "Only use these to define move constructors and move assignment operators,
> and
> > > for perfect forwarding." from "C++11 use in Chromium"
> > 
> > A move constructor requires the input type to be the same as the type being
> > constructed.
> 
> Yeah, but there are quite a lot of places where we use a different type for a
> "conversion". For example:
> https://codesearch.chromium.org/chromium/src/base/optional.h?rcl=0&l=140
> 
> I feel that it is better than pass-by-value in this case.

^^ And please consider this a non-blocking comment.

[dcheng, 2017-01-10 05:58:09]

type converters / struct traits lgtm

https://codereview.chromium.org/2607063002/diff/200001/mojo/public/cpp/bindin...
File mojo/public/cpp/bindings/type_converter.h (right):

https://codereview.chromium.org/2607063002/diff/200001/mojo/public/cpp/bindin...
mojo/public/cpp/bindings/type_converter.h:90: struct
TypeConverter<std::vector<T>, Container> {
I hope we can kill this someday.

[jam, 2017-01-10 07:00:23]

lgtm

in the future for changes like this please break up owners review per top level
directory

[Sam McNally, 2017-01-10 07:22:46]

The CQ bit was checked by sammc@chromium.org

[commit-bot: I haz the power, 2017-01-10 07:23:01]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-10 09:42:53]

CQ is committing da patch.

Bot data: {"patchset_id": 200001, "attempt_start_ts": 1484032966018200,
"parent_rev": "5b457a448b5f4c2e4fb620f39a495e35fef486c5", "commit_rev":
"0e91f591193bd8616a44a2c307f58ca11255c62c"}

[commit-bot: I haz the power, 2017-01-10 09:43:23]

Description was changed from

==========
Remove mojo::Array.

BUG=674766
==========

to

==========
Remove mojo::Array.

BUG=674766

Review-Url: https://codereview.chromium.org/2607063002
Cr-Commit-Position: refs/heads/master@{#442540}
Committed:
https://chromium.googlesource.com/chromium/src/+/0e91f591193bd8616a44a2c307f5...
==========

[commit-bot: I haz the power, 2017-01-10 09:43:25]

Committed patchset #3 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/0e91f591193bd8616a44a2c307f5...