Closed Bug 1016617 Opened 10 years ago Closed 10 years ago

seer should not allow subresources/subhosts to have a higher load count than their parent

Categories

(Core :: Networking, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: u408661, Assigned: jaypoulz)

Details

Attachments

(1 file, 2 obsolete files)

Right now it's conceivable that, if a page (say www.example.com/index.html) loads multiple resources from the same origin (say www.example2.com/style.css and www.example2.com/script.js), then www.example2.com will say it has been loaded twice, while www.example.com (which caused all the loads of www.example2.com) was only loaded once. We should limit the hits for a suborigin/subresource to the number of times the parent was loaded.
Attached patch bug-1016617-fix.patch (obsolete) — Splinter Review
Attachment #8430896 - Flags: review?(hurley)
Comment on attachment 8430896 [details] [diff] [review] bug-1016617-fix.patch Review of attachment 8430896 [details] [diff] [review]: ----------------------------------------------------------------- Just a couple little issues, nothing functional. Fix up the issues, and upload a new patch, and we should be good to go. ::: netwerk/base/src/Seer.cpp @@ +1939,5 @@ > } > mozStorageStatementScoper scope(stmt); > > + // Make sure the hit count can't be more that its parent > + int32_t hitCount = ((info.hitCount + 1) > parentCount) ?\ You don't need the \ here, C++ will keep scanning until it finds a ; @@ +1946,3 @@ > nsresult rv = stmt->BindInt32ByName(NS_LITERAL_CSTRING("hit_count"), > + hitCount); > + nit: you added trailing whitespace on this empty line. Get rid of it, please.
Attachment #8430896 - Flags: review?(hurley)
Attached patch bug-1016617-fix.patch (obsolete) — Splinter Review
Now with less whitespace. White is saddened.
Attachment #8430896 - Attachment is obsolete: true
Attachment #8430917 - Flags: review?(hurley)
Comment on attachment 8430917 [details] [diff] [review] bug-1016617-fix.patch Review of attachment 8430917 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, this round's my fault :) ::: netwerk/base/src/Seer.cpp @@ +1940,5 @@ > mozStorageStatementScoper scope(stmt); > > + // Make sure the hit count can't be more that its parent > + int32_t hitCount = ((info.hitCount + 1) > parentCount) ? > + parentCount : (info.hitCount + 1); Sorry, I don't know why I didn't think of this last time. Just use std::min here, so int32_t hitCount = std::min(info.hitCount + 1, parentCount); instead of reinventing the wheel. This is the last change I'm gonna request from you, I swear :)
Attachment #8430917 - Flags: review?(hurley)
Accidentally gave the code an std. Will add a new try server link for this build.
Attachment #8430917 - Attachment is obsolete: true
Attachment #8430941 - Flags: review?(hurley)
Here's a link to the new try build. https://tbpl.mozilla.org/?tree=Try&rev=19053be91785
Attachment #8430941 - Flags: review?(hurley) → review+
Keywords: checkin-needed
Pushed with some minor tweaks to the commit message. Thanks for the patch! https://hg.mozilla.org/integration/mozilla-inbound/rev/974979fa171a
Keywords: checkin-needed
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: