1292057 - [Presentation WebAPI] add complete NSPR log for core service and webidl implementation

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Comment 1

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Attachment: Bug 1292057 - add NSPR log for Presentation API.

Review commit: https://reviewboard.mozilla.org/r/71578/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/71578/

Comment 2

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8781076 [details]
Bug 1292057 - add NSPR log for Presentation API.

@kershaw, please let me know if any other log you think is useful.

Comment 3

[Kershaw Chang [:kershaw]]

Comment on attachment 8781076 [details]
Bug 1292057 - add NSPR log for Presentation API.

https://reviewboard.mozilla.org/r/71576/#review69290

::: dom/presentation/PresentationConnection.cpp:354
(Diff revision 1)
>  }
>  
>  NS_IMETHODIMP
>  PresentationConnection::NotifyMessage(const nsAString& aSessionId,
>                                        const nsACString& aData)
>  {

Adding a log here could be helpful.

::: dom/presentation/PresentationConnection.cpp:382
(Diff revision 1)
>  }
>  
>  NS_IMETHODIMP
>  PresentationConnection::NotifyReplaced()
>  {
>    return NotifyStateChange(mId,

Also here.

::: dom/presentation/PresentationService.cpp:305
(Diff revision 1)
>    }
>  }
>  
>  nsresult
>  PresentationService::HandleDeviceChange()
>  {

Not sure if we are insterested in device change. If yes, you might want to add a log here.

::: dom/presentation/PresentationService.cpp:486
(Diff revision 1)
>  }
>  
>  nsresult
>  PresentationService::HandleReconnectRequest(nsIPresentationSessionRequest* aRequest)
>  {
>    nsCOMPtr<nsIPresentationControlChannel> ctrlChannel;

Add here to log reconnecting at receiver side.

::: dom/presentation/PresentationService.cpp:985
(Diff revision 1)
>  PresentationService::UntrackSessionInfo(const nsAString& aSessionId,
>                                          uint8_t aRole)
>  {
>    MOZ_ASSERT(aRole == nsIPresentationService::ROLE_CONTROLLER ||
>               aRole == nsIPresentationService::ROLE_RECEIVER);
>    // Remove the session info.

Also add a log here.

::: dom/presentation/PresentationSessionInfo.cpp:902
(Diff revision 1)
>  // nsIServerSocketListener
>  NS_IMETHODIMP
>  PresentationControllingInfo::OnSocketAccepted(nsIServerSocket* aServerSocket,
>                                              nsISocketTransport* aTransport)
>  {
>    int32_t port;

Add a log here could be helpful.

::: dom/presentation/PresentationSessionInfo.cpp:925
(Diff revision 1)
>  
>  NS_IMETHODIMP
>  PresentationControllingInfo::OnStopListening(nsIServerSocket* aServerSocket,
>                                             nsresult aStatus)
>  {
>    MOZ_ASSERT(NS_IsMainThread());

Also here.

::: dom/presentation/PresentationSessionInfo.cpp:1222
(Diff revision 1)
>            false;
>  }
>  
>  nsresult
>  PresentationPresentingInfo::NotifyResponderReady()
>  {

Add a log here.

Comment 4

[Kershaw Chang [:kershaw]]

Comment on attachment 8781076 [details]
Bug 1292057 - add NSPR log for Presentation API.

https://reviewboard.mozilla.org/r/71578/#review69296

Overall looks good to me.
I just want to add more logs in callback functions.

Comment 5

[Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)]

Comment on attachment 8781076 [details]
Bug 1292057 - add NSPR log for Presentation API.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/71578/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f63971df6d31
add NSPR log for Presentation API. r=kershaw

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f63971df6d31