Commit d1faf713 by tonihei Committed by kim-vde

Use Clock to create Handler in ListenerSet.

This ensures the Handler is governed by this clock.

PiperOrigin-RevId: 353011555
parent f2057156
...@@ -35,6 +35,7 @@ import com.google.android.exoplayer2.trackselection.TrackSelection; ...@@ -35,6 +35,7 @@ import com.google.android.exoplayer2.trackselection.TrackSelection;
import com.google.android.exoplayer2.trackselection.TrackSelectionArray; import com.google.android.exoplayer2.trackselection.TrackSelectionArray;
import com.google.android.exoplayer2.trackselection.TrackSelector; import com.google.android.exoplayer2.trackselection.TrackSelector;
import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Assertions;
import com.google.android.exoplayer2.util.Clock;
import com.google.android.exoplayer2.util.ListenerSet; import com.google.android.exoplayer2.util.ListenerSet;
import com.google.android.exoplayer2.util.Log; import com.google.android.exoplayer2.util.Log;
import com.google.android.exoplayer2.util.MimeTypes; import com.google.android.exoplayer2.util.MimeTypes;
...@@ -138,6 +139,7 @@ public final class CastPlayer extends BasePlayer { ...@@ -138,6 +139,7 @@ public final class CastPlayer extends BasePlayer {
listeners = listeners =
new ListenerSet<>( new ListenerSet<>(
Looper.getMainLooper(), Looper.getMainLooper(),
Clock.DEFAULT,
Player.Events::new, Player.Events::new,
(listener, eventFlags) -> listener.onEvents(/* player= */ this, eventFlags)); (listener, eventFlags) -> listener.onEvents(/* player= */ this, eventFlags));
......
...@@ -21,6 +21,7 @@ import com.google.android.exoplayer2.Player; ...@@ -21,6 +21,7 @@ import com.google.android.exoplayer2.Player;
import com.google.android.exoplayer2.Timeline; import com.google.android.exoplayer2.Timeline;
import com.google.android.exoplayer2.testutil.StubExoPlayer; import com.google.android.exoplayer2.testutil.StubExoPlayer;
import com.google.android.exoplayer2.trackselection.TrackSelectionArray; import com.google.android.exoplayer2.trackselection.TrackSelectionArray;
import com.google.android.exoplayer2.util.Clock;
import com.google.android.exoplayer2.util.ListenerSet; import com.google.android.exoplayer2.util.ListenerSet;
/** A fake player for testing content/ad playback. */ /** A fake player for testing content/ad playback. */
...@@ -43,6 +44,7 @@ import com.google.android.exoplayer2.util.ListenerSet; ...@@ -43,6 +44,7 @@ import com.google.android.exoplayer2.util.ListenerSet;
listeners = listeners =
new ListenerSet<>( new ListenerSet<>(
Looper.getMainLooper(), Looper.getMainLooper(),
Clock.DEFAULT,
Player.Events::new, Player.Events::new,
(listener, eventFlags) -> listener.onEvents(/* player= */ this, eventFlags)); (listener, eventFlags) -> listener.onEvents(/* player= */ this, eventFlags));
period = new Timeline.Period(); period = new Timeline.Period();
......
...@@ -154,6 +154,7 @@ import java.util.List; ...@@ -154,6 +154,7 @@ import java.util.List;
listeners = listeners =
new ListenerSet<>( new ListenerSet<>(
applicationLooper, applicationLooper,
clock,
Player.Events::new, Player.Events::new,
(listener, eventFlags) -> listener.onEvents(playerForListeners, eventFlags)); (listener, eventFlags) -> listener.onEvents(playerForListeners, eventFlags));
mediaSourceHolderSnapshots = new ArrayList<>(); mediaSourceHolderSnapshots = new ArrayList<>();
......
...@@ -91,6 +91,7 @@ public class AnalyticsCollector ...@@ -91,6 +91,7 @@ public class AnalyticsCollector
listeners = listeners =
new ListenerSet<>( new ListenerSet<>(
Util.getCurrentOrMainLooper(), Util.getCurrentOrMainLooper(),
clock,
AnalyticsListener.Events::new, AnalyticsListener.Events::new,
(listener, eventFlags) -> {}); (listener, eventFlags) -> {});
period = new Period(); period = new Period();
......
...@@ -26,39 +26,42 @@ import androidx.annotation.Nullable; ...@@ -26,39 +26,42 @@ import androidx.annotation.Nullable;
*/ */
public interface HandlerWrapper { public interface HandlerWrapper {
/** @see Handler#getLooper() */ /** See {@link Handler#getLooper()}. */
Looper getLooper(); Looper getLooper();
/** @see Handler#obtainMessage(int) */ /** See {@link Handler#hasMessages(int)}. */
boolean hasMessages(int what);
/** See {@link Handler#obtainMessage(int)}. */
Message obtainMessage(int what); Message obtainMessage(int what);
/** @see Handler#obtainMessage(int, Object) */ /** See {@link Handler#obtainMessage(int, Object)}. */
Message obtainMessage(int what, @Nullable Object obj); Message obtainMessage(int what, @Nullable Object obj);
/** @see Handler#obtainMessage(int, int, int) */ /** See {@link Handler#obtainMessage(int, int, int)}. */
Message obtainMessage(int what, int arg1, int arg2); Message obtainMessage(int what, int arg1, int arg2);
/** @see Handler#obtainMessage(int, int, int, Object) */ /** See {@link Handler#obtainMessage(int, int, int, Object)}. */
Message obtainMessage(int what, int arg1, int arg2, @Nullable Object obj); Message obtainMessage(int what, int arg1, int arg2, @Nullable Object obj);
/** @see Handler#sendEmptyMessage(int) */ /** See {@link Handler#sendEmptyMessage(int)}. */
boolean sendEmptyMessage(int what); boolean sendEmptyMessage(int what);
/** @see Handler#sendEmptyMessageDelayed(int, long) */ /** See {@link Handler#sendEmptyMessageDelayed(int, long)}. */
boolean sendEmptyMessageDelayed(int what, int delayMs); boolean sendEmptyMessageDelayed(int what, int delayMs);
/** @see Handler#sendEmptyMessageAtTime(int, long) */ /** See {@link Handler#sendEmptyMessageAtTime(int, long)}. */
boolean sendEmptyMessageAtTime(int what, long uptimeMs); boolean sendEmptyMessageAtTime(int what, long uptimeMs);
/** @see Handler#removeMessages(int) */ /** See {@link Handler#removeMessages(int)}. */
void removeMessages(int what); void removeMessages(int what);
/** @see Handler#removeCallbacksAndMessages(Object) */ /** See {@link Handler#removeCallbacksAndMessages(Object)}. */
void removeCallbacksAndMessages(@Nullable Object token); void removeCallbacksAndMessages(@Nullable Object token);
/** @see Handler#post(Runnable) */ /** See {@link Handler#post(Runnable)}. */
boolean post(Runnable runnable); boolean post(Runnable runnable);
/** @see Handler#postDelayed(Runnable, long) */ /** See {@link Handler#postDelayed(Runnable, long)}. */
boolean postDelayed(Runnable runnable, long delayMs); boolean postDelayed(Runnable runnable, long delayMs);
} }
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
*/ */
package com.google.android.exoplayer2.util; package com.google.android.exoplayer2.util;
import android.os.Handler;
import android.os.Looper; import android.os.Looper;
import android.os.Message; import android.os.Message;
import androidx.annotation.CheckResult; import androidx.annotation.CheckResult;
...@@ -72,7 +71,8 @@ public final class ListenerSet<T, E extends MutableFlags> { ...@@ -72,7 +71,8 @@ public final class ListenerSet<T, E extends MutableFlags> {
private static final int MSG_ITERATION_FINISHED = 0; private static final int MSG_ITERATION_FINISHED = 0;
private static final int MSG_LAZY_RELEASE = 1; private static final int MSG_LAZY_RELEASE = 1;
private final Handler handler; private final Clock clock;
private final HandlerWrapper handler;
private final Supplier<E> eventFlagsSupplier; private final Supplier<E> eventFlagsSupplier;
private final IterationFinishedEvent<T, E> iterationFinishedEvent; private final IterationFinishedEvent<T, E> iterationFinishedEvent;
private final CopyOnWriteArraySet<ListenerHolder<T, E>> listeners; private final CopyOnWriteArraySet<ListenerHolder<T, E>> listeners;
...@@ -86,6 +86,7 @@ public final class ListenerSet<T, E extends MutableFlags> { ...@@ -86,6 +86,7 @@ public final class ListenerSet<T, E extends MutableFlags> {
* *
* @param looper A {@link Looper} used to call listeners on. The same {@link Looper} must be used * @param looper A {@link Looper} used to call listeners on. The same {@link Looper} must be used
* to call all other methods of this class. * to call all other methods of this class.
* @param clock A {@link Clock}.
* @param eventFlagsSupplier A {@link Supplier} for new instances of {@link E the event flags * @param eventFlagsSupplier A {@link Supplier} for new instances of {@link E the event flags
* type}. * type}.
* @param iterationFinishedEvent An {@link IterationFinishedEvent} sent when all other events sent * @param iterationFinishedEvent An {@link IterationFinishedEvent} sent when all other events sent
...@@ -93,11 +94,13 @@ public final class ListenerSet<T, E extends MutableFlags> { ...@@ -93,11 +94,13 @@ public final class ListenerSet<T, E extends MutableFlags> {
*/ */
public ListenerSet( public ListenerSet(
Looper looper, Looper looper,
Clock clock,
Supplier<E> eventFlagsSupplier, Supplier<E> eventFlagsSupplier,
IterationFinishedEvent<T, E> iterationFinishedEvent) { IterationFinishedEvent<T, E> iterationFinishedEvent) {
this( this(
/* listeners= */ new CopyOnWriteArraySet<>(), /* listeners= */ new CopyOnWriteArraySet<>(),
looper, looper,
clock,
eventFlagsSupplier, eventFlagsSupplier,
iterationFinishedEvent); iterationFinishedEvent);
} }
...@@ -105,8 +108,10 @@ public final class ListenerSet<T, E extends MutableFlags> { ...@@ -105,8 +108,10 @@ public final class ListenerSet<T, E extends MutableFlags> {
private ListenerSet( private ListenerSet(
CopyOnWriteArraySet<ListenerHolder<T, E>> listeners, CopyOnWriteArraySet<ListenerHolder<T, E>> listeners,
Looper looper, Looper looper,
Clock clock,
Supplier<E> eventFlagsSupplier, Supplier<E> eventFlagsSupplier,
IterationFinishedEvent<T, E> iterationFinishedEvent) { IterationFinishedEvent<T, E> iterationFinishedEvent) {
this.clock = clock;
this.listeners = listeners; this.listeners = listeners;
this.eventFlagsSupplier = eventFlagsSupplier; this.eventFlagsSupplier = eventFlagsSupplier;
this.iterationFinishedEvent = iterationFinishedEvent; this.iterationFinishedEvent = iterationFinishedEvent;
...@@ -114,7 +119,7 @@ public final class ListenerSet<T, E extends MutableFlags> { ...@@ -114,7 +119,7 @@ public final class ListenerSet<T, E extends MutableFlags> {
queuedEvents = new ArrayDeque<>(); queuedEvents = new ArrayDeque<>();
// It's safe to use "this" because we don't send a message before exiting the constructor. // It's safe to use "this" because we don't send a message before exiting the constructor.
@SuppressWarnings("methodref.receiver.bound.invalid") @SuppressWarnings("methodref.receiver.bound.invalid")
Handler handler = Util.createHandler(looper, this::handleMessage); HandlerWrapper handler = clock.createHandler(looper, this::handleMessage);
this.handler = handler; this.handler = handler;
} }
...@@ -129,7 +134,7 @@ public final class ListenerSet<T, E extends MutableFlags> { ...@@ -129,7 +134,7 @@ public final class ListenerSet<T, E extends MutableFlags> {
@CheckResult @CheckResult
public ListenerSet<T, E> copy( public ListenerSet<T, E> copy(
Looper looper, IterationFinishedEvent<T, E> iterationFinishedEvent) { Looper looper, IterationFinishedEvent<T, E> iterationFinishedEvent) {
return new ListenerSet<>(listeners, looper, eventFlagsSupplier, iterationFinishedEvent); return new ListenerSet<>(listeners, looper, clock, eventFlagsSupplier, iterationFinishedEvent);
} }
/** /**
......
...@@ -34,6 +34,11 @@ import androidx.annotation.Nullable; ...@@ -34,6 +34,11 @@ import androidx.annotation.Nullable;
} }
@Override @Override
public boolean hasMessages(int what) {
return handler.hasMessages(what);
}
@Override
public Message obtainMessage(int what) { public Message obtainMessage(int what) {
return handler.obtainMessage(what); return handler.obtainMessage(what);
} }
......
...@@ -43,7 +43,8 @@ public class ListenerSetTest { ...@@ -43,7 +43,8 @@ public class ListenerSetTest {
@Test @Test
public void queueEvent_withoutFlush_sendsNoEvents() { public void queueEvent_withoutFlush_sendsNoEvents() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
TestListener listener = mock(TestListener.class); TestListener listener = mock(TestListener.class);
listenerSet.add(listener); listenerSet.add(listener);
...@@ -57,7 +58,8 @@ public class ListenerSetTest { ...@@ -57,7 +58,8 @@ public class ListenerSetTest {
@Test @Test
public void flushEvents_sendsPreviouslyQueuedEventsToAllListeners() { public void flushEvents_sendsPreviouslyQueuedEventsToAllListeners() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
TestListener listener1 = mock(TestListener.class); TestListener listener1 = mock(TestListener.class);
TestListener listener2 = mock(TestListener.class); TestListener listener2 = mock(TestListener.class);
listenerSet.add(listener1); listenerSet.add(listener1);
...@@ -81,7 +83,8 @@ public class ListenerSetTest { ...@@ -81,7 +83,8 @@ public class ListenerSetTest {
@Test @Test
public void flushEvents_recursive_sendsEventsInCorrectOrder() { public void flushEvents_recursive_sendsEventsInCorrectOrder() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
// Listener1 sends callback3 recursively when receiving callback1. // Listener1 sends callback3 recursively when receiving callback1.
TestListener listener1 = TestListener listener1 =
spy( spy(
...@@ -114,7 +117,8 @@ public class ListenerSetTest { ...@@ -114,7 +117,8 @@ public class ListenerSetTest {
public void public void
flushEvents_withMultipleMessageQueueIterations_sendsIterationFinishedEventPerIteration() { flushEvents_withMultipleMessageQueueIterations_sendsIterationFinishedEventPerIteration() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
// Listener1 sends callback1 recursively when receiving callback3. // Listener1 sends callback1 recursively when receiving callback3.
TestListener listener1 = TestListener listener1 =
spy( spy(
...@@ -170,7 +174,8 @@ public class ListenerSetTest { ...@@ -170,7 +174,8 @@ public class ListenerSetTest {
@Test @Test
public void flushEvents_calledFromIterationFinishedCallback_restartsIterationFinishedEvents() { public void flushEvents_calledFromIterationFinishedCallback_restartsIterationFinishedEvents() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
// Listener2 sends callback1 recursively when receiving the iteration finished event. // Listener2 sends callback1 recursively when receiving the iteration finished event.
TestListener listener2 = TestListener listener2 =
spy( spy(
...@@ -212,7 +217,8 @@ public class ListenerSetTest { ...@@ -212,7 +217,8 @@ public class ListenerSetTest {
@Test @Test
public void flushEvents_withUnsetEventFlag_doesNotThrow() { public void flushEvents_withUnsetEventFlag_doesNotThrow() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
listenerSet.queueEvent(/* eventFlag= */ C.INDEX_UNSET, TestListener::callback1); listenerSet.queueEvent(/* eventFlag= */ C.INDEX_UNSET, TestListener::callback1);
listenerSet.flushEvents(); listenerSet.flushEvents();
...@@ -224,7 +230,8 @@ public class ListenerSetTest { ...@@ -224,7 +230,8 @@ public class ListenerSetTest {
@Test @Test
public void add_withRecursion_onlyReceivesUpdatesForFutureEvents() { public void add_withRecursion_onlyReceivesUpdatesForFutureEvents() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
TestListener listener2 = mock(TestListener.class); TestListener listener2 = mock(TestListener.class);
// Listener1 adds listener2 recursively. // Listener1 adds listener2 recursively.
TestListener listener1 = TestListener listener1 =
...@@ -256,7 +263,8 @@ public class ListenerSetTest { ...@@ -256,7 +263,8 @@ public class ListenerSetTest {
@Test @Test
public void add_withQueueing_onlyReceivesUpdatesForFutureEvents() { public void add_withQueueing_onlyReceivesUpdatesForFutureEvents() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
TestListener listener1 = mock(TestListener.class); TestListener listener1 = mock(TestListener.class);
TestListener listener2 = mock(TestListener.class); TestListener listener2 = mock(TestListener.class);
...@@ -281,7 +289,8 @@ public class ListenerSetTest { ...@@ -281,7 +289,8 @@ public class ListenerSetTest {
@Test @Test
public void remove_withRecursion_stopsReceivingEventsImmediately() { public void remove_withRecursion_stopsReceivingEventsImmediately() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
TestListener listener2 = mock(TestListener.class); TestListener listener2 = mock(TestListener.class);
// Listener1 removes listener2 recursively. // Listener1 removes listener2 recursively.
TestListener listener1 = TestListener listener1 =
...@@ -309,7 +318,8 @@ public class ListenerSetTest { ...@@ -309,7 +318,8 @@ public class ListenerSetTest {
@Test @Test
public void remove_withQueueing_stopsReceivingEventsImmediately() { public void remove_withQueueing_stopsReceivingEventsImmediately() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
TestListener listener1 = mock(TestListener.class); TestListener listener1 = mock(TestListener.class);
TestListener listener2 = mock(TestListener.class); TestListener listener2 = mock(TestListener.class);
listenerSet.add(listener1); listenerSet.add(listener1);
...@@ -330,7 +340,8 @@ public class ListenerSetTest { ...@@ -330,7 +340,8 @@ public class ListenerSetTest {
@Test @Test
public void release_stopsForwardingEventsImmediately() { public void release_stopsForwardingEventsImmediately() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
TestListener listener2 = mock(TestListener.class); TestListener listener2 = mock(TestListener.class);
// Listener1 releases the set from within the callback. // Listener1 releases the set from within the callback.
TestListener listener1 = TestListener listener1 =
...@@ -357,7 +368,8 @@ public class ListenerSetTest { ...@@ -357,7 +368,8 @@ public class ListenerSetTest {
@Test @Test
public void release_preventsRegisteringNewListeners() { public void release_preventsRegisteringNewListeners() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
TestListener listener = mock(TestListener.class); TestListener listener = mock(TestListener.class);
listenerSet.release(); listenerSet.release();
...@@ -370,7 +382,8 @@ public class ListenerSetTest { ...@@ -370,7 +382,8 @@ public class ListenerSetTest {
@Test @Test
public void lazyRelease_stopsForwardingEventsFromNewHandlerMessagesAndCallsReleaseCallback() { public void lazyRelease_stopsForwardingEventsFromNewHandlerMessagesAndCallsReleaseCallback() {
ListenerSet<TestListener, Flags> listenerSet = ListenerSet<TestListener, Flags> listenerSet =
new ListenerSet<>(Looper.myLooper(), Flags::new, TestListener::iterationFinished); new ListenerSet<>(
Looper.myLooper(), Clock.DEFAULT, Flags::new, TestListener::iterationFinished);
TestListener listener = mock(TestListener.class); TestListener listener = mock(TestListener.class);
listenerSet.add(listener); listenerSet.add(listener);
......
...@@ -148,6 +148,16 @@ public class FakeClock implements Clock { ...@@ -148,6 +148,16 @@ public class FakeClock implements Clock {
return true; return true;
} }
private synchronized boolean hasPendingMessage(ClockHandler handler, int what) {
for (int i = 0; i < handlerMessages.size(); i++) {
HandlerMessageData message = handlerMessages.get(i);
if (message.handler.equals(handler) && message.message == what) {
return true;
}
}
return handler.handler.hasMessages(what);
}
/** Message data saved to send messages or execute runnables at a later time on a Handler. */ /** Message data saved to send messages or execute runnables at a later time on a Handler. */
private static final class HandlerMessageData { private static final class HandlerMessageData {
...@@ -199,6 +209,11 @@ public class FakeClock implements Clock { ...@@ -199,6 +209,11 @@ public class FakeClock implements Clock {
} }
@Override @Override
public boolean hasMessages(int what) {
return hasPendingMessage(/* handler= */ this, what);
}
@Override
public Message obtainMessage(int what) { public Message obtainMessage(int what) {
return handler.obtainMessage(what); return handler.obtainMessage(what);
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment