Closed Bug 1115502 Opened 5 years ago Closed 5 years ago

Expose inBrowser information to TCPSocket

Categories

(Core :: Networking, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
blocking-b2g 2.2+
Tracking Status
firefox36 --- wontfix
firefox37 --- wontfix
firefox38 --- fixed
b2g-v2.2 --- fixed
b2g-master --- fixed

People

(Reporter: ethan, Assigned: ethan)

References

Details

Attachments

(1 file, 3 obsolete files)

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.
Blocks: 1070944
Assignee: nobody → ettseng
Status: NEW → ASSIGNED
Attached patch bug-1115502.patch (obsolete) — Splinter Review
TCPSocketParent and TCPServerSocketParent pass inBrowser to TCPSocket.js
in their open() and listen() interface, respectively.
Attached patch bug-1115502.patch (obsolete) — Splinter Review
Fix a typo in the previous patch.
Attachment #8541453 - Attachment is obsolete: true
Comment on attachment 8542058 [details] [diff] [review]
bug-1115502.patch

Hi Honza,

Could you help to review this patch?
Attachment #8542058 - Flags: review?(honzab.moz)
This is essential function for cost control and should be landed to 2.2.
feature-b2g: --- → 2.2?
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; ?
Attachment #8542058 - Flags: review?(honzab.moz) → review+
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.
Attached patch bug-1115502.patch (obsolete) — Splinter Review
Refresh comment "r=honzab".
Attachment #8542058 - Attachment is obsolete: true
Honza, thanks for your review! :)
Keywords: checkin-needed
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!
Flags: needinfo?(ettseng)
Keywords: checkin-needed
Rebase the patch (update the UUID of nsITCPSocketIntermediary).
Attachment #8546433 - Attachment is obsolete: true
Flags: needinfo?(ettseng)
Request to check in this patch to both central and 2.2.
Keywords: checkin-needed
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.
blocking-b2g: --- → 2.2+
feature-b2g: 2.2? → ---
https://hg.mozilla.org/mozilla-central/rev/b035d2c566ad
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
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
Attachment #8548699 - Flags: approval-mozilla-b2g37?
Attachment #8548699 - Flags: approval-mozilla-b2g37? → approval-mozilla-b2g37+
You need to log in before you can comment on or make changes to this bug.