1115502 - Expose inBrowser information to TCPSocket

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

Bug 1070944 is trying to separate network statistics accounted from app and
browser iframe elements.
The idea is to provide an additional parameter inBrowser (true/false) when
saving statistics through NetworkStatsServiceProxy.

Since TCP socket saves statistics in TCPSocket.js, we have to expose inBrowser
information to TCPSocket.js.

Comment 1

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

Attachment: bug-1115502.patch

TCPSocketParent and TCPServerSocketParent pass inBrowser to TCPSocket.js
in their open() and listen() interface, respectively.

Comment 2

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

Attachment: bug-1115502.patch

Fix a typo in the previous patch.

Comment 3

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

Comment on attachment 8542058 [details] [diff] [review]
bug-1115502.patch

Hi Honza,

Could you help to review this patch?

Comment 4

[Vincent Chang[:vchang][changyihsin]]

This is essential function for cost control and should be landed to 2.2.

Comment 5

[Honza Bambas (:mayhemer)]

Comment on attachment 8542058 [details] [diff] [review]
bug-1115502.patch

Review of attachment 8542058 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/network/TCPSocketParent.h
@@ +62,5 @@
>                          const uint32_t& aTrackingNumber) MOZ_OVERRIDE;
>    virtual bool RecvRequestDelete() MOZ_OVERRIDE;
>    virtual nsresult OfflineNotification(nsISupports *) MOZ_OVERRIDE;
>    virtual uint32_t GetAppId() MOZ_OVERRIDE;
> +  bool GetInBrowser();

MOZ_OVERRIDE; ?

Comment 6

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

Comment on attachment 8542058 [details] [diff] [review]
bug-1115502.patch

Review of attachment 8542058 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/network/TCPSocketParent.h
@@ +62,5 @@
>                          const uint32_t& aTrackingNumber) MOZ_OVERRIDE;
>    virtual bool RecvRequestDelete() MOZ_OVERRIDE;
>    virtual nsresult OfflineNotification(nsISupports *) MOZ_OVERRIDE;
>    virtual uint32_t GetAppId() MOZ_OVERRIDE;
> +  bool GetInBrowser();

No. GetInBrowser() is not declared in any parent class of TCPSocketParent.

Comment 7

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

Attachment: bug-1115502.patch

Refresh comment "r=honzab".

Comment 8

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

The result of Treeherder:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=38ed7c7f89da

Comment 9

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

Honza, thanks for your review! :)

Comment 10

[Carsten Book [:Tomcat]]

sorry, this failed to apply:

applying bug-1115502.patch
patching file dom/network/interfaces/nsITCPSocketParent.idl
Hunk #1 FAILED at 59
1 out of 1 hunks FAILED -- saving rejects to file dom/network/interfaces/nsITCPSocketParent.idl.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
errors during apply, please fix and refresh bug-1115502.patch

could you take a look? Thanks!

Comment 11

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

Attachment: bug-1115502.patch

Rebase the patch (update the UUID of nsITCPSocketIntermediary).

Comment 12

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

The result of Treeherder:
3.0: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d8584b2d0632
2.2: https://treeherder.mozilla.org/#/jobs?repo=try&revision=3b3a7c1d76e2

Comment 13

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

Request to check in this patch to both central and 2.2.

Comment 14

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

triage: this is must-have for not miscalculating data usage between browser and system.
We had this defect since 2.0, and should get it fixed in 2.2 at least.

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b035d2c566ad

Comment 16

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

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

Comment 17

[Ethan Tseng [:ethan] (PTO until Feb 26, 2021)]

Comment on attachment 8548699 [details] [diff] [review]
bug-1115502.patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 1070944 (Separation of browsing
                                          data from system data)
User impact if declined: Incorrect usage in cost control app
Testing completed: On m-c
Risk to taking this patch (and alternatives if risky): None
String or UUID changes made by this patch: nsITCPSocketInternal,
                                           nsITCPSocketIntermediary

Comment 18

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/048ce32c92b9