[Bug 34228] Implement API for requesting channels and handling them yourself

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=34228

--- Comment #7 from Olli Salli <ollisal at gmail.com> 2011-03-03 06:27:04 PST ---
(In reply to comment #6)
> (In reply to comment #5)
> 
> > +    void channelReceived(const Tp::ChannelPtr &channel);
> > +
> > 
> > You need to relay at least the user action time so that whoever's using the API
> > can decide whether to display their window/tab for it when it's re-requested
> > and given to it again. Also, at least the hints from the ChannelRequest should
> > be given (a ready ChannelRequest will be given in the requestsSatisfied param
> > for every new request) so that they don't get lost.
> > 
> > wjt has proposed an interesting usecase for hints for requested text channels
> > in particular: you could use them to invoke the text chat UI, or whoever is
> > currently handling a text chat to a particular contact, for filling a
> > particular message to be sent but still leaving the user a way to edit it
> > before sending in the text UI. This could be used for e.g. sharing links: you'd
> > press "share link to X" from a context menu in your browser, and a text chat
> > window would pop up OR a previously open text chat window to that contact would
> > be focused, allowing you to describe the link you're sharing.
> > 
> > Note that while notifications on somebody else requesting the channel you're
> > currently handling through the Request and Handle API should give the hints to
> > you, it's fine for us to disallow passing any hints to the original request
> > made to the Request and Handle API: any successful request and handle will
> > result in the handling being done by the requester by definition, so the hints
> > would be just passing data uselessly from the requester back to it through the
> > CD.
> I am confused here, you first said "the hints of ChannelRequest should be given
> in channelReceived", but then you say that it does not make sense to pass hints
> as we are the caller anyway. I agree that hints should not be exposed, either
> in Account::create/ensureAndHandle and channelReceived. Is there something I
> missed here?

Read again with some thought please!

Why do we have HandledChannelNotifier in the first place? Because *somebody
else* can ensure the same channel, resulting in us being re-invoked. In that
case, we should receive the hints they passed in.

However, our *own* request shouldn't pass hints, as those would just at best
come back to us (where we already have them). So, channelReceived() wouldn't
have any hints for first emission, which should correspond to our request. Note
that the application doesn't see that emission anyway: that happens before the
PendingChannel is finished(). BUT it would have hints for any successive
emissions, if hints were passed to the request leading to the channel being
handled again.

Shortly put: We don't pass hints. But somebody else might, if they're using the
"ensure and forget" API rather than this request and handle API. We should get
their hints in that case.

> 
> > +    if (account != mAccount) {
> > +        emit error(TP_QT4_ERROR_SERVICE_CONFUSED,
> > +                QLatin1String("Account received is not the same as the account
> > which made the request"));
> > +        context->setFinishedWithError(TP_QT4_ERROR_SERVICE_CONFUSED,
> > +                QLatin1String("Account received is not the same as the account
> > which made the request"));
> > +        return;
> > +    }
> > +
> > 
> > I think you should set the context finished with error in this case. You're not
> > handling (incl. closing) the channel, although the CD assumes you are because
> > you return successfully.
> Well, check the code again, it's calling setFinishedWithError, so I don't get
> this.
> 

True, sorry about that - too quick reading :)

> >  ConnectionPtr PendingChannel::connection() const
> >  {
> > -    return ConnectionPtr(qobject_cast<Connection*>((Connection*)
> > object().data()));
> > +    return mPriv->connection;
> >  }
> > 
> > Please either make connection() return the current Connection of the Account
> > for a request made on an account (consistent, through a bit useless), or
> > document that it always returns NULL for requests made on accounts.
> We are always setting mPriv->connection = channel->connection() when we get the
> channel. It will work if either the channel was requested using Account or
> Connection.
> 

Well, that's strictly speaking true only once the operation is finished (when
we get the channel). When a Channel is requested directly from the Connection,
connection() will be non-NULL all the time - it'll be the Conn the request was
made on.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.