chat: preview link unfurls before sending

go/chat/livelocation_test.go

@@ -81,7 +81,7 @@ func (m *mockUnfurler) Prefetch(ctx context.Context, uid gregor1.UID, convID cha
 }
 
 func (m *mockUnfurler) UnfurlAndSend(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID,
-	msg chat1.MessageUnboxed,
+	msg chat1.MessageUnboxed, suppress []string,
 ) {
 	require.True(m.t, msg.IsValid())
 	body := msg.Valid().MessageBody

go/chat/maps/livelocation.go

@@ -225,7 +225,7 @@ func (l *LiveLocationTracker) updateMapUnfurl(ctx context.Context, t *locationTr
 	unfurlDoneCh := make(chan struct{}, 10)
 	outboxID := storage.GetOutboxIDFromURL(body, t.convID, newMsg)
 	listenerID := l.G().NotifyRouter.AddListener(newUnfurlNotifyListener(l.G(), outboxID, unfurlDoneCh))
-	l.G().Unfurler.UnfurlAndSend(ctx, l.uid, t.convID, newMsg)
+	l.G().Unfurler.UnfurlAndSend(ctx, l.uid, t.convID, newMsg, nil)
 	select {
 	case <-unfurlDoneCh:
 	case <-time.After(time.Minute):

go/chat/sender.go

@@ -1337,9 +1337,10 @@ func (s *BlockingSender) Send(ctx context.Context, convID chat1.ConversationID,
 			chat1.ChatActivitySource_LOCAL)
 	}
 	if conv.GetTopicType() == chat1.TopicType_CHAT {
-		// Unfurl
+		// Unfurl. the suppressed urls travel with the message on its outbox record, so a
+		// send that waited offline still honours what the sender dismissed
 		go s.G().Unfurler.UnfurlAndSend(globals.BackgroundChatCtx(ctx, s.G()), boxed.ClientHeader.Sender,
-			convID, unboxedMsg)
+			convID, unboxedMsg, sendOpts.GetUnfurlSuppress())
 		// Start tracking any live location sends
 		if unboxedMsg.IsValid() && unboxedMsg.GetMessageType() == chat1.MessageType_TEXT &&
 			unboxedMsg.Valid().MessageBody.Text().LiveLocation != nil {

go/chat/server.go

@@ -1006,6 +1006,7 @@ func (h *Server) PostTextNonblock(ctx context.Context, arg chat1.PostTextNonbloc
 	}
 
 	var parg chat1.PostLocalNonblockArg
+	parg.UnfurlSuppress = arg.UnfurlSuppress
 	parg.SessionID = arg.SessionID
 	parg.ClientPrev = arg.ClientPrev
 	parg.ConversationID = arg.ConversationID
@@ -1198,14 +1199,18 @@ func (h *Server) PostLocalNonblock(ctx context.Context, arg chat1.PostLocalNonbl
 
 	// Create non block sender
 	var prepareOpts chat1.SenderPrepareOptions
+	var sendOpts chat1.SenderSendOptions
 	sender := NewBlockingSender(h.G(), h.boxer, h.remoteClient)
 	nonblockSender := NewNonblockingSender(h.G(), sender)
 	prepareOpts.ReplyTo = arg.ReplyTo
+	// rides the outbox record, so a message that waits offline still knows which urls the
+	// sender dismissed by the time it actually goes out
+	sendOpts.UnfurlSuppress = arg.UnfurlSuppress
 	if arg.Msg.ClientHeader.Conv.TopicType == chat1.TopicType_NONE {
 		arg.Msg.ClientHeader.Conv.TopicType = chat1.TopicType_CHAT
 	}
 	obid, _, err := nonblockSender.Send(ctx, arg.ConversationID, arg.Msg, arg.ClientPrev, arg.OutboxID,
-		nil, &prepareOpts)
+		&sendOpts, &prepareOpts)
 	if err != nil {
 		return res, fmt.Errorf("PostLocalNonblock: unable to send message: err: %s", err.Error())
 	}
@@ -1611,6 +1616,16 @@ func (h *Server) UpdateUnsentText(ctx context.Context, arg chat1.UpdateUnsentTex
 	return nil
 }
 
+func (h *Server) UnfurlPreviewLocal(ctx context.Context, arg chat1.UnfurlPreviewLocalArg) (res []chat1.UnfurlPreviewInfo, err error) {
+	ctx = globals.ChatCtx(ctx, h.G(), keybase1.TLFIdentifyBehavior_CHAT_GUI, nil, h.identNotifier)
+	defer h.Trace(ctx, &err, "UnfurlPreviewLocal")()
+	uid, err := utils.AssertLoggedInUID(ctx, h.G())
+	if err != nil {
+		return nil, err
+	}
+	return h.G().Unfurler.PreviewURLs(ctx, uid, arg.ConvID, arg.Text), nil
+}
+
 func (h *Server) UpdateTyping(ctx context.Context, arg chat1.UpdateTypingArg) (err error) {
 	var identBreaks []keybase1.TLFIdentifyFailure
 	ctx = globals.ChatCtx(ctx, h.G(), keybase1.TLFIdentifyBehavior_CHAT_GUI,
@@ -2666,7 +2681,10 @@ func (h *Server) ResolveUnfurlPrompt(ctx context.Context, arg chat1.ResolveUnfur
 		if len(msgs) != 1 {
 			return errors.New("message not found")
 		}
-		h.G().Unfurler.UnfurlAndSend(ctx, uid, arg.ConvID, msgs[0])
+		// no suppress list on this pass: the message is already sent, so its outbox record
+		// is gone. urls dismissed at send time were marked then, and UnfurlAndSend reads
+		// those markers, so a dismissal still holds here
+		h.G().Unfurler.UnfurlAndSend(ctx, uid, arg.ConvID, msgs[0], nil)
 		return nil
 	}
 	atyp, err := arg.Result.ActionType()

go/chat/types/interfaces.go

@@ -504,8 +504,10 @@ type WhitelistExemption interface {
 
 type Unfurler interface {
 	UnfurlAndSend(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID,
-		msg chat1.MessageUnboxed)
+		msg chat1.MessageUnboxed, suppress []string)
 	Prefetch(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, msgText string) int
+	PreviewURLs(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID,
+		text string) []chat1.UnfurlPreviewInfo
 	Status(ctx context.Context, outboxID chat1.OutboxID) (UnfurlerTaskStatus, *chat1.UnfurlResult, error)
 	Retry(ctx context.Context, outboxID chat1.OutboxID)
 	Complete(ctx context.Context, outboxID chat1.OutboxID)

go/chat/types/types.go

@@ -581,13 +581,19 @@ type DummyUnfurler struct{}
 var _ Unfurler = (*DummyUnfurler)(nil)
 
 func (d DummyUnfurler) UnfurlAndSend(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID,
-	msg chat1.MessageUnboxed) {
+	msg chat1.MessageUnboxed, suppress []string) {
 }
 
 func (d DummyUnfurler) Prefetch(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID, msgText string) int {
 	return 0
 }
 
+func (d DummyUnfurler) PreviewURLs(ctx context.Context, uid gregor1.UID, convID chat1.ConversationID,
+	text string,
+) []chat1.UnfurlPreviewInfo {
+	return nil
+}
+
 func (d DummyUnfurler) Status(ctx context.Context, outboxID chat1.OutboxID) (UnfurlerTaskStatus, *chat1.UnfurlResult, error) {
 	return UnfurlerTaskStatusFailed, nil, nil
 }

go/chat/unfurl/cache.go

@@ -21,28 +21,33 @@ type cacheItem struct {
 
 type unfurlCache struct {
 	sync.Mutex
-	cache *lru.Cache
-	clock clockwork.Clock
+	cache    *lru.Cache
+	clock    clockwork.Clock
+	lifetime time.Duration
 }
 
 func newUnfurlCache() *unfurlCache {
+	return newUnfurlCacheWithLifetime(defaultCacheLifetime)
+}
+
+func newUnfurlCacheWithLifetime(lifetime time.Duration) *unfurlCache {
 	cache, err := lru.New(defaultCacheSize)
 	if err != nil {
 		panic(err)
 	}
 	return &unfurlCache{
-		cache: cache,
-		clock: clockwork.NewRealClock(),
+		cache:    cache,
+		clock:    clockwork.NewRealClock(),
+		lifetime: lifetime,
 	}
 }
 
 func (c *unfurlCache) setClock(clock clockwork.Clock) {
 	c.clock = clock
 }
 
-// get determines if the item is in the cache and newer than 10
-// minutes. We don't want to cache this value indefinitely in case the page
-// content changes.
+// get determines if the item is in the cache and newer than the cache's lifetime. We
+// don't want to cache this value indefinitely in case the page content changes.
 func (c *unfurlCache) get(key string) (res cacheItem, ok bool) {
 	c.Lock()
 	defer c.Unlock()
@@ -55,7 +60,7 @@ func (c *unfurlCache) get(key string) (res cacheItem, ok bool) {
 	if !ok {
 		return res, false
 	}
-	valid := c.clock.Now().Sub(cacheItem.ctime.Time()) <= defaultCacheLifetime
+	valid := c.clock.Now().Sub(cacheItem.ctime.Time()) <= c.lifetime
 	if !valid {
 		c.cache.Remove(key)
 	}