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)
Core
Networking
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: u408661, Assigned: jaypoulz)
Details
Attachments
(1 file, 2 obsolete files)
4.02 KB,
patch
|
u408661
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8430896 -
Flags: review?(hurley)
Assignee | ||
Comment 2•10 years ago
|
||
Here is the try build:
https://tbpl.mozilla.org/?tree=Try&rev=77a9170f7da3
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)
Assignee | ||
Comment 4•10 years ago
|
||
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)
Assignee | ||
Comment 6•10 years ago
|
||
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)
Assignee | ||
Comment 7•10 years ago
|
||
Here's a link to the new try build.
https://tbpl.mozilla.org/?tree=Try&rev=19053be91785
Attachment #8430941 -
Flags: review?(hurley) → review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 8•10 years ago
|
||
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
Comment 9•10 years ago
|
||
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.
Description
•