Shape Detection: add SaveSameObject idl to SameObject annotations

Shape Detection: add SaveSameObject idl to SameObject annotations

This CL adds SaveSameObject to the SameObject annotation in idls,
following up to comments [1,2].

[1] https://codereview.chromium.org/2859413002/#msg27
[2] https://codereview.chromium.org/2859413002/#msg28


BUG=718693
TBR=haraken@chromium.org

Review-Url: https://codereview.chromium.org/2873453004
Cr-Commit-Position: refs/heads/master@{#471476}
Committed: https://chromium.googlesource.com/chromium/src/+/fc059e074702f77cd815822eb93e6ee954a12bc5

[mcasas, 2017-05-09 17:28:21]

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

[mcasas, 2017-05-09 17:28:51]

mcasas@chromium.org changed reviewers:
+ yukishiino@chromium.org

[mcasas, 2017-05-09 17:28:51]

yukishiino@ PTAL

(haraken@ FYI)

[commit-bot: I haz the power, 2017-05-09 17:29: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, 2017-05-09 17:45:59]

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

[commit-bot: I haz the power, 2017-05-09 17:46:00]

Dry run: Try jobs failed on following builders:
  cast_shell_android on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...)

[mcasas, 2017-05-09 20:59:06]

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

[commit-bot: I haz the power, 2017-05-09 20:59:54]

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-05-09 21:48:46]

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

[commit-bot: I haz the power, 2017-05-09 21:48:46]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[haraken, 2017-05-09 22:30:43]

Can we add tests?

[mcasas, 2017-05-10 00:57:53]

Patchset #3 (id:40001) has been deleted

[mcasas, 2017-05-10 00:58:13]

ptal

[Yuki, 2017-05-10 06:37:37]

https://codereview.chromium.org/2873453004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/DetectedBarcode.idl
(right):

https://codereview.chromium.org/2873453004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/DetectedBarcode.idl:12:
[SameObject, SaveSameObject] readonly attribute DOMString rawValue;
Please double-check on your side, but IIUC, we don't need [SaveSameObject] here
because DOMString is not an Object, you cannot distinguish two strings with the
same content (if the content is the same, then two strings are identical).

https://codereview.chromium.org/2873453004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/DetectedBarcode.idl:13:
[SameObject, SaveSameObject] readonly attribute DOMRect boundingBox;
Please double-check on your side, but we shouldn't need [SaveSameObject] here,
too, as long as you're returning the same blink::DOMRect* from
DetectedBarcode::boundingBox().

We apologize this confusing situation, but you only need [SaveSameObject] for
|cornerPoints| that returns a FrozenArray, because corner_points_ of type
HeapVector<Point2D> is not a ScriptWrappable and we cannot associate a wrapper
to it.

https://codereview.chromium.org/2873453004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/DetectedFace.idl (right):

https://codereview.chromium.org/2873453004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/DetectedFace.idl:12:
[SameObject, SaveSameObject] readonly attribute DOMRect boundingBox;
Ditto.  We shouldn't need [SaveSameObject] here, but please test it.

https://codereview.chromium.org/2873453004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/DetectedText.idl (right):

https://codereview.chromium.org/2873453004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/DetectedText.idl:12:
[SameObject, SaveSameObject] readonly attribute DOMRect boundingBox;
As same, we shouldn't need [SaveSameObject] for both of |rawValue| of string
type and |boundingBox| that is a ScriptWrappable.

[mcasas, 2017-05-10 17:35:21]

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

[commit-bot: I haz the power, 2017-05-10 17:36:49]

Dry run: CQ is trying da patch.

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

[mcasas, 2017-05-10 19:00:45]

ptal

https://codereview.chromium.org/2873453004/diff/60001/third_party/WebKit/Sour...
File third_party/WebKit/Source/modules/shapedetection/DetectedBarcode.idl
(right):

https://codereview.chromium.org/2873453004/diff/60001/third_party/WebKit/Sour...
third_party/WebKit/Source/modules/shapedetection/DetectedBarcode.idl:13:
[SameObject, SaveSameObject] readonly attribute DOMRect boundingBox;
On 2017/05/10 06:37:37, Yuki wrote:
> Please double-check on your side, but we shouldn't need [SaveSameObject] here,
> too, as long as you're returning the same blink::DOMRect* from
> DetectedBarcode::boundingBox().
> 
> We apologize this confusing situation, but you only need [SaveSameObject] for
> |cornerPoints| that returns a FrozenArray, because corner_points_ of type
> HeapVector<Point2D> is not a ScriptWrappable and we cannot associate a wrapper
> to it.

OIC! Apologies for the misunderstanding; next Ps will
only keep [SaveSameObject] for the FrozenArray<>s.

[commit-bot: I haz the power, 2017-05-10 19:53:47]

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

[commit-bot: I haz the power, 2017-05-10 19:53:48]

Dry run: Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Yuki, 2017-05-11 08:11:45]

LGTM.

[mcasas, 2017-05-11 16:13:22]

On 2017/05/11 08:11:45, Yuki wrote:
> LGTM.

Thanks! haraken@ does this lgty?

[mcasas, 2017-05-12 19:02:15]

Description was changed from

==========
Shape Detection: add SaveSameObject idl to SameObject annotations

This CL adds SaveSameObject to the SameObject annotation in idls,
following up to comments [1,2].

[1] https://codereview.chromium.org/2859413002/#msg27
[2] https://codereview.chromium.org/2859413002/#msg28


BUG=718693
==========

to

==========
Shape Detection: add SaveSameObject idl to SameObject annotations

This CL adds SaveSameObject to the SameObject annotation in idls,
following up to comments [1,2].

[1] https://codereview.chromium.org/2859413002/#msg27
[2] https://codereview.chromium.org/2859413002/#msg28


BUG=718693
TBR=haraken@chromium.org
==========

[mcasas, 2017-05-12 19:02:19]

The CQ bit was checked by mcasas@chromium.org

[commit-bot: I haz the power, 2017-05-12 19:04:49]

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-05-12 22:59:51]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494615739477280,
"parent_rev": "7534c70bd82e16b47f06290f1355eec795e3c2fb", "commit_rev":
"fc059e074702f77cd815822eb93e6ee954a12bc5"}

[commit-bot: I haz the power, 2017-05-12 22:59:58]

Description was changed from

==========
Shape Detection: add SaveSameObject idl to SameObject annotations

This CL adds SaveSameObject to the SameObject annotation in idls,
following up to comments [1,2].

[1] https://codereview.chromium.org/2859413002/#msg27
[2] https://codereview.chromium.org/2859413002/#msg28


BUG=718693
TBR=haraken@chromium.org
==========

to

==========
Shape Detection: add SaveSameObject idl to SameObject annotations

This CL adds SaveSameObject to the SameObject annotation in idls,
following up to comments [1,2].

[1] https://codereview.chromium.org/2859413002/#msg27
[2] https://codereview.chromium.org/2859413002/#msg28


BUG=718693
TBR=haraken@chromium.org

Review-Url: https://codereview.chromium.org/2873453004
Cr-Commit-Position: refs/heads/master@{#471476}
Committed:
https://chromium.googlesource.com/chromium/src/+/fc059e074702f77cd815822eb93e...
==========

[commit-bot: I haz the power, 2017-05-12 22:59:59]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/fc059e074702f77cd815822eb93e...