Add MediaStream.active attribute

Add MediaStream.active attribute

This CL added MediaStream.active attribute, onactive and oninactive events according to the spec
http://w3c.github.io/mediacapture-main/getusermedia.html#widl-MediaStream-active

BUG=445084
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=189225

[jiajia.qin, 2014-12-25 07:35:07]

Please take a look. Thanks.

[tommi (sloooow) - chröme, 2015-01-03 16:13:53]

tommi@chromium.org changed reviewers:
+ perkj@chromium.org

[tommi (sloooow) - chröme, 2015-01-03 16:13:54]

Per - can you take a look?

[perkj_chrome, 2015-01-07 14:12:18]

Can you please add layout tests that shows this? 
Also, have you seen that the spec now talks about active/inactive MediaStreams
instead of readyState?

https://codereview.chromium.org/827673002/diff/1/Source/modules/mediastream/M...
File Source/modules/mediastream/MediaStream.cpp (right):

https://codereview.chromium.org/827673002/diff/1/Source/modules/mediastream/M...
Source/modules/mediastream/MediaStream.cpp:163: for
(MediaStreamTrackVector::iterator iter = m_audioTracks.begin(); iter !=
?

[jiajia.qin, 2015-01-12 10:49:18]

On 2015/01/07 14:12:18, perkj wrote:
> Can you please add layout tests that shows this? 
> Also, have you seen that the spec now talks about active/inactive MediaStreams
> instead of readyState?
I think you want to say the spec has renamed MediaStream.active to
MediaStream.ended :)
MediaStream.ended is a deprecated attribute. The patch set 2 added
MediaStream.active attribute
according to spec
http://w3c.github.io/mediacapture-main/getusermedia.html#widl-MediaStream-active
and still retained MediaStream.ended. Please take a look.

>
https://codereview.chromium.org/827673002/diff/1/Source/modules/mediastream/M...
> File Source/modules/mediastream/MediaStream.cpp (right):
> 
>
https://codereview.chromium.org/827673002/diff/1/Source/modules/mediastream/M...
> Source/modules/mediastream/MediaStream.cpp:163: for
> (MediaStreamTrackVector::iterator iter = m_audioTracks.begin(); iter !=
> ?
Sorry, I don't understand this comment's meaning.

[perkj_chrome, 2015-01-12 11:22:18]

perkj@chromium.org changed reviewers:
+ hta@chromium.org

[perkj_chrome, 2015-01-12 11:22:19]

Thanks for doing this. I don't know blink that well so we need another blink
reviewer before it can be landed.

Can you please update the description? I also think that MediaStream.inactive
can toogle from inactive to active if a live track is added to the MediaStream.


Hta, can you also please take a look? Does my comments match with your
understanding of MediaStream.inactive ?

https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
File Source/modules/mediastream/MediaStream.cpp (right):

https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:122: m_descriptor->setEnded();
should setEnded be removed? 

If I understand the spec correctly a MediaStream can now toggle between inactive
and active depending on the track members readyState and what tracks it
contains. So inactive is not the same as ended?

https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:167: for
(MediaStreamTrackVector::iterator iter = m_audioTracks.begin(); iter !=
it looks like this this line is not complete? At least not in the code review
system.

https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:193: if (!active()) {
Adding a track if the state is inactive should toogle the state to active? You
can create an empty MS that should this be inactive. Later adding a track should
change the state to active.

https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
File Source/modules/mediastream/MediaStream.idl (right):

https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.idl:58: attribute EventHandler onended;
mark onended as deprecated ?

[jiajia.qin, 2015-01-13 06:20:51]

On 2015/01/12 11:22:19, perkj wrote:
> Thanks for doing this. I don't know blink that well so we need another blink
> reviewer before it can be landed.
> 
> Can you please update the description?
Done

> I also think that MediaStream.inactive
> can toogle from inactive to active if a live track is added to the
MediaStream.
Agree. I add this logic into MediaStream::addTrack. I removed the ended
checking. So, in current
implementation, even the mediastream.active is false, it still can addTrack. If
the added track
is live, it will trigger an 'active' event.  MediaStream::removeTrack is
similar.
 
> Hta, can you also please take a look? Does my comments match with your
> understanding of MediaStream.inactive ?
Wait for Hta's reply.

>
https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
> File Source/modules/mediastream/MediaStream.cpp (right):
> 
>
https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
> Source/modules/mediastream/MediaStream.cpp:122: m_descriptor->setEnded();
> should setEnded be removed? 
> If I understand the spec correctly a MediaStream can now toggle between
inactive
> and active depending on the track members readyState and what tracks it
> contains. So inactive is not the same as ended?
I'm not sure about it. I thought ended and inactive were same. But, in patch set
3, I removed setEnded part temporarily.

>
https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
> Source/modules/mediastream/MediaStream.cpp:167: for
> (MediaStreamTrackVector::iterator iter = m_audioTracks.begin(); iter !=
> it looks like this this line is not complete? At least not in the code review
> system.
Sorry, it's my fault. I made mistake when extracting this CL to submit. Now it
should be correct :)

>
https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
> Source/modules/mediastream/MediaStream.cpp:193: if (!active()) {
> Adding a track if the state is inactive should toogle the state to active? You
> can create an empty MS that should this be inactive. Later adding a track
should
> change the state to active.
Done

>
https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
> File Source/modules/mediastream/MediaStream.idl (right):
> 
>
https://codereview.chromium.org/827673002/diff/20001/Source/modules/mediastre...
> Source/modules/mediastream/MediaStream.idl:58: attribute EventHandler onended;
> mark onended as deprecated ?
Done

[perkj_chrome, 2015-01-13 08:24:31]

https://codereview.chromium.org/827673002/diff/20001/Source/platform/mediastr...
File Source/platform/mediastream/MediaStreamCenter.cpp (right):

https://codereview.chromium.org/827673002/diff/20001/Source/platform/mediastr...
Source/platform/mediastream/MediaStreamCenter.cpp:144: stream->setActive(false);
Shouldn't you keep setEnded here? MediaStream.Stop is also deprecated but why
not leave this unchanged. 

Alternatively, remove MediaStreamDescriptor.setEnded all together and just let
MediaStream.ended mirror MediaStream.active.

https://codereview.chromium.org/827673002/diff/40001/LayoutTests/fast/mediast...
File LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html (right):

https://codereview.chromium.org/827673002/diff/40001/LayoutTests/fast/mediast...
LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html:146:
getUserMedia({audio:true, video:true}, gotStream3);
shouldBeFalse('stream1.active') ?

https://codereview.chromium.org/827673002/diff/40001/Source/modules/mediastre...
File Source/modules/mediastream/MediaStream.cpp (right):

https://codereview.chromium.org/827673002/diff/40001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:211:
MediaStreamCenter::instance().didAddMediaStreamTrack(m_descriptor.get(),
track->component());
please let MediaStreamCenter::instance().didAddMediaStreamTrac be the last thing
in this function.

https://codereview.chromium.org/827673002/diff/40001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:246: if (active() &&
emptyOrOnlyEndedTracks()) {
dito. Please call  MediaStreamCenter::instance().didRemoveMediaStreamTrack last
in the function.

https://codereview.chromium.org/827673002/diff/40001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:317: m_descriptor->setActive(false);
is this necessary?

https://codereview.chromium.org/827673002/diff/40001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:346:
scheduleDispatchEvent(MediaStreamTrackEvent::create(EventTypeNames::addtrack,
false, false, track));
You should check and active state here as well. dito for removeRemoteTrack.
These are used when a remote track is added or removed using a PeerConnection.
There is a fast test of this in RTCPeerConnection-remotestreams.html.

[jiajia.qin, 2015-01-13 08:52:45]

https://codereview.chromium.org/827673002/diff/20001/Source/platform/mediastr...
File Source/platform/mediastream/MediaStreamCenter.cpp (right):

https://codereview.chromium.org/827673002/diff/20001/Source/platform/mediastr...
Source/platform/mediastream/MediaStreamCenter.cpp:144: stream->setActive(false);
On 2015/01/13 08:24:31, perkj wrote:
> Shouldn't you keep setEnded here? MediaStream.Stop is also deprecated but why
> not leave this unchanged. 
Agree. I will leave this unchanged.
  
> Alternatively, remove MediaStreamDescriptor.setEnded all together and just let
> MediaStream.ended mirror MediaStream.active. 
Does it mean that you think mediastream.ended is same with mediastream.inactive?
Then, in any case(including the constructor function) where mediastream only
contains empty or ended tracks, we need to call setEnded too?

[perkj_chrome, 2015-01-13 09:01:31]

I meant that we could delete the method MediaStreamDescriptor.setEnded and
MediaStreamDescription.ended() and let MediaStream.ended() return
MediaStreamDescription.active() until the MediaStream.ended attribute is
removed.

But it probably better to just leave everything related to the ended
attribute and ended event unchanged for a while and just introduce
MediaStream.active /inactive in paralell. When it has been used in a stable
Chrome release we can remove MediaStream.ended.

On Tue, Jan 13, 2015 at 9:52 AM, <jiajia.qin@intel.com> wrote:

>
> https://codereview.chromium.org/827673002/diff/20001/
> Source/platform/mediastream/MediaStreamCenter.cpp
> File Source/platform/mediastream/MediaStreamCenter.cpp (right):
>
> https://codereview.chromium.org/827673002/diff/20001/
> Source/platform/mediastream/MediaStreamCenter.cpp#newcode144
> Source/platform/mediastream/MediaStreamCenter.cpp:144:
> stream->setActive(false);
> On 2015/01/13 08:24:31, perkj wrote:
>
>> Shouldn't you keep setEnded here? MediaStream.Stop is also deprecated
>>
> but why
>
>> not leave this unchanged.
>>
> Agree. I will leave this unchanged.
>
>  Alternatively, remove MediaStreamDescriptor.setEnded all together and
>>
> just let
>
>> MediaStream.ended mirror MediaStream.active.
>>
> Does it mean that you think mediastream.ended is same with
> mediastream.inactive? Then, in any case(including the constructor
> function) where mediastream only contains empty or ended tracks, we need
> to call setEnded too?
>
> https://codereview.chromium.org/827673002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[jiajia.qin, 2015-01-13 10:42:30]

https://codereview.chromium.org/827673002/diff/40001/LayoutTests/fast/mediast...
File LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html (right):

https://codereview.chromium.org/827673002/diff/40001/LayoutTests/fast/mediast...
LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html:146:
getUserMedia({audio:true, video:true}, gotStream3);
On 2015/01/13 08:24:31, perkj wrote:
> shouldBeFalse('stream1.active') ?

Done.

https://codereview.chromium.org/827673002/diff/40001/Source/modules/mediastre...
File Source/modules/mediastream/MediaStream.cpp (right):

https://codereview.chromium.org/827673002/diff/40001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:211:
MediaStreamCenter::instance().didAddMediaStreamTrack(m_descriptor.get(),
track->component());
On 2015/01/13 08:24:31, perkj wrote:
> please let MediaStreamCenter::instance().didAddMediaStreamTrac be the last
thing
> in this function. 

Done.

https://codereview.chromium.org/827673002/diff/40001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:246: if (active() &&
emptyOrOnlyEndedTracks()) {
On 2015/01/13 08:24:31, perkj wrote:
> dito. Please call  MediaStreamCenter::instance().didRemoveMediaStreamTrack
last
> in the function.

Done.

https://codereview.chromium.org/827673002/diff/40001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:317: m_descriptor->setActive(false);
On 2015/01/13 08:24:31, perkj wrote:
> is this necessary?

Done.

https://codereview.chromium.org/827673002/diff/40001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:346:
scheduleDispatchEvent(MediaStreamTrackEvent::create(EventTypeNames::addtrack,
false, false, track));
On 2015/01/13 08:24:31, perkj wrote:
> You should check and active state here as well. dito for removeRemoteTrack.
> These are used when a remote track is added or removed using a PeerConnection.
> There is a fast test of this in RTCPeerConnection-remotestreams.html.

Done.

[hta - Chromium, 2015-01-14 14:12:02]

My advice: Touch less.

In particular, don't let ending touch the active state, and vice versa.

Apart from that, the code LGTM, but doing less needs to be done. (Also suggest
testing more - that's a constant....)

One point (m_stopped) may be important.

https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
File LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html (right):

https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html:53:
testPassed('Stream1 active.');
Please verify the value of stream3.active in both cases.

https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html:59: }
This file has been written with the last callback that is expected to be called
first, and the first one last. Personally, I find that confusing, but please be
consistent with it - so place onStreamInactive at the top.

https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
File LayoutTests/fast/mediastream/MediaStream-stop.html (right):

https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
LayoutTests/fast/mediastream/MediaStream-stop.html:8: description("Test cloning
mediastreams oninactive.");
While you're at it, can you fix this to be a grammatical sentence?

https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
LayoutTests/fast/mediastream/MediaStream-stop.html:27: testPassed('Stream
inactive.');
Also check that active is false here.

https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
LayoutTests/fast/mediastream/MediaStream-stop.html:35: cloned_stream.oninactive
= onStreamInactive;
Also check that active is true here.

https://codereview.chromium.org/827673002/diff/80001/Source/modules/mediastre...
File Source/modules/mediastream/MediaStream.cpp (left):

https://codereview.chromium.org/827673002/diff/80001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:312: return;
Until we delete the ended code altogether, keep this code.

https://codereview.chromium.org/827673002/diff/80001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:332: return;
m_stopped is not relevant to either active or ended. Please don't touch it
without expert advice (I think it's a DOM thing).

https://codereview.chromium.org/827673002/diff/80001/Source/modules/mediastre...
File Source/modules/mediastream/MediaStream.cpp (right):

https://codereview.chromium.org/827673002/diff/80001/Source/modules/mediastre...
Source/modules/mediastream/MediaStream.cpp:306: if (!active())
Wrong test. Ended streams (as previously defined) could not later become
non-ended. Inactive streams can become active.

Suggest not touching the "active" concept when ending a stream.

[jiajia.qin, 2015-01-15 11:31:56]

On 2015/01/14 14:12:02, hta - Chromium wrote:
> My advice: Touch less.
> 
> In particular, don't let ending touch the active state, and vice versa.
> 
> Apart from that, the code LGTM, but doing less needs to be done. (Also suggest
> testing more - that's a constant....)
> 
> One point (m_stopped) may be important.
> 
>
https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
> File LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html (right):
> 
>
https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
> LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html:53:
> testPassed('Stream1 active.');
> Please verify the value of stream3.active in both cases.

Done

>
https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
> LayoutTests/fast/mediastream/MediaStream-add-remove-tracks.html:59: }
> This file has been written with the last callback that is expected to be
called
> first, and the first one last. Personally, I find that confusing, but please
be
> consistent with it - so place onStreamInactive at the top.

Done

>
https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
> File LayoutTests/fast/mediastream/MediaStream-stop.html (right):
> 
>
https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
> LayoutTests/fast/mediastream/MediaStream-stop.html:8: description("Test
cloning
> mediastreams oninactive.");
> While you're at it, can you fix this to be a grammatical sentence?

Done

>
https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
> LayoutTests/fast/mediastream/MediaStream-stop.html:27: testPassed('Stream
> inactive.');
> Also check that active is false here.

Done

>
https://codereview.chromium.org/827673002/diff/80001/LayoutTests/fast/mediast...
> LayoutTests/fast/mediastream/MediaStream-stop.html:35:
cloned_stream.oninactive
> = onStreamInactive;
> Also check that active is true here.

Done

>
https://codereview.chromium.org/827673002/diff/80001/Source/modules/mediastre...
> File Source/modules/mediastream/MediaStream.cpp (left):
> 
>
https://codereview.chromium.org/827673002/diff/80001/Source/modules/mediastre...
> Source/modules/mediastream/MediaStream.cpp:312: return;
> Until we delete the ended code altogether, keep this code.

Done

>
https://codereview.chromium.org/827673002/diff/80001/Source/modules/mediastre...
> Source/modules/mediastream/MediaStream.cpp:332: return;
> m_stopped is not relevant to either active or ended. Please don't touch it
> without expert advice (I think it's a DOM thing).

Done

>
https://codereview.chromium.org/827673002/diff/80001/Source/modules/mediastre...
> File Source/modules/mediastream/MediaStream.cpp (right):
> 
>
https://codereview.chromium.org/827673002/diff/80001/Source/modules/mediastre...
> Source/modules/mediastream/MediaStream.cpp:306: if (!active())
> Wrong test. Ended streams (as previously defined) could not later become
> non-ended. Inactive streams can become active.
> 
> Suggest not touching the "active" concept when ending a stream.

Done

[hta - Chromium, 2015-01-15 12:07:47]

lgtm

[perkj_chrome, 2015-01-15 13:52:39]

lgtm

No you only need an owner to review it too.... Thanks for you patient.

[perkj_chrome, 2015-01-16 08:12:17]

tommi - do you want to/dare to or can reassign the review to jochen?

[tommi (sloooow) - chröme, 2015-01-17 09:45:51]

The CQ bit was checked by tommi@chromium.org

[tommi (sloooow) - chröme, 2015-01-17 09:45:52]

lgtm

[commit-bot: I haz the power, 2015-01-17 09:46:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/827673002/100001

[commit-bot: I haz the power, 2015-01-17 09:46:34]

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

[commit-bot: I haz the power, 2015-01-17 09:46:35]

Failed to apply patch for Source/platform/mediastream/MediaStreamDescriptor.h:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file Source/platform/mediastream/MediaStreamDescriptor.h
  Hunk #2 FAILED at 97.
  1 out of 2 hunks FAILED -- saving rejects to file
Source/platform/mediastream/MediaStreamDescriptor.h.rej

Patch:       Source/platform/mediastream/MediaStreamDescriptor.h
Index: Source/platform/mediastream/MediaStreamDescriptor.h
diff --git a/Source/platform/mediastream/MediaStreamDescriptor.h
b/Source/platform/mediastream/MediaStreamDescriptor.h
index
d4c05d5d82359c1c2702def0b8bae6e4c9edfb36..f3658f66a7e8f8e1e0e332c1601f6865f4e02a49
100644
--- a/Source/platform/mediastream/MediaStreamDescriptor.h
+++ b/Source/platform/mediastream/MediaStreamDescriptor.h
@@ -80,6 +80,9 @@ public:
     void addRemoteTrack(MediaStreamComponent*);
     void removeRemoteTrack(MediaStreamComponent*);
 
+    bool active() const { return m_active; }
+    void setActive(bool active) { m_active = active; }
+
     bool ended() const { return m_ended; }
     void setEnded() { m_ended = true; }
 
@@ -94,6 +97,7 @@ private:
     String m_id;
     Vector<RefPtr<MediaStreamComponent> > m_audioComponents;
     Vector<RefPtr<MediaStreamComponent> > m_videoComponents;
+    bool m_active;
     bool m_ended;
 
     OwnPtr<ExtraData> m_extraData;

[jiajia.qin, 2015-01-19 01:24:33]

jiajia.qin@intel.com changed reviewers:
+ hta - chromium. jochen@chromium.org
- hta@chromium.org

[jiajia.qin, 2015-01-19 01:24:33]

Add jochen to review 'Source/platform/mediastream' part.

[jiajia.qin, 2015-01-19 01:33:19]

jiajia.qin@intel.com changed reviewers:
+ hta@chromium.org, jochen@chromium.org
- hta - chromium. jochen@chromium.org

[jiajia.qin, 2015-01-19 01:38:22]

Add jochen to review 'Source/platform/mediastream' part.

[jiajia.qin, 2015-01-19 02:11:45]

The CQ bit was checked by jiajia.qin@intel.com

[commit-bot: I haz the power, 2015-01-19 02:12:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/827673002/120001

[commit-bot: I haz the power, 2015-01-19 02:20:10]

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

[commit-bot: I haz the power, 2015-01-19 02:20:11]

Try jobs failed on following builders:
  blink_presubmit on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/blink_presubmit/builds/2...)

[jochen (gone - plz use gerrit), 2015-01-19 12:57:05]

do you have a link to the intent to ship thread? It's a web exposed change,
right?

[blink-reviews, 2015-01-19 13:06:30]

Meta: Do we need an intent to ship every time we change something to
conform to a new rev of the W3C WebRTC spec? (that will be a lot of ItS).

I don't think we have any intent-to-ship thread on this one.



On Mon, Jan 19, 2015 at 1:57 PM, <jochen@chromium.org> wrote:

> do you have a link to the intent to ship thread? It's a web exposed change,
> right?
>
> https://codereview.chromium.org/827673002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

[jiajia.qin, 2015-01-20 04:47:01]

On 2015/01/19 12:57:05, jochen (slow) wrote:
> do you have a link to the intent to ship thread? It's a web exposed change,
> right?

Hi, jochen. I don't have a link about this one. Yes, it's a web exposed change.
But the change
is trivial. Is it still needed to send the intent to ship?

[jochen (gone - plz use gerrit), 2015-01-20 19:17:31]

On 2015/01/20 at 04:47:01, jiajia.qin wrote:
> On 2015/01/19 12:57:05, jochen (slow) wrote:
> > do you have a link to the intent to ship thread? It's a web exposed change,
> > right?
> 
> Hi, jochen. I don't have a link about this one. Yes, it's a web exposed
change. But the change
> is trivial. Is it still needed to send the intent to ship?

All web exposed changes should have an intent to ship thread. If it's trivial,
it should be a no-brainer to get the three LGTMs.

WebRTC changes went through that processes as well in the past (although
admittedly not all...)

[jiajia.qin, 2015-01-26 09:00:36]

On 2015/01/20 19:17:31, jochen (slow) wrote:
> On 2015/01/20 at 04:47:01, jiajia.qin wrote:
> > On 2015/01/19 12:57:05, jochen (slow) wrote:
> > > do you have a link to the intent to ship thread? It's a web exposed
change,
> > > right?
> > 
> > Hi, jochen. I don't have a link about this one. Yes, it's a web exposed
> change. But the change
> > is trivial. Is it still needed to send the intent to ship?
> 
> All web exposed changes should have an intent to ship thread. If it's trivial,
> it should be a no-brainer to get the three LGTMs.
> 
> WebRTC changes went through that processes as well in the past (although
> admittedly not all...)
Have sent it. Please see
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/m4jiqG67Mvo

[jiajia.qin, 2015-01-29 07:08:53]

jochen, the intent to implement has got 3 'lgtm'. Is it ok for you to land this
CL?

[jochen (gone - plz use gerrit), 2015-01-29 15:16:18]

lgtm!

[jiajia.qin, 2015-01-30 02:12:38]

The CQ bit was checked by jiajia.qin@intel.com

[commit-bot: I haz the power, 2015-01-30 02:13:22]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/827673002/120001

[commit-bot: I haz the power, 2015-01-30 02:17:27]

Committed patchset #7 (id:120001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=189225