Bug 40312 - [chromium] Track state for whether a ResourceRequest is fetched via a proxy
Summary: [chromium] Track state for whether a ResourceRequest is fetched via a proxy
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Mike Belshe
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-06-08 10:52 PDT by Mike Belshe
Modified: 2010-08-10 22:49 PDT (History)
5 users (show)

See Also:


Attachments
Simple patch - chromium specific. (4.50 KB, patch)
2010-06-08 17:59 PDT, Mike Belshe
levin: review+
Details | Formatted Diff | Diff
Update the reviewer field . (5.26 KB, patch)
2010-06-09 11:58 PDT, Mike Belshe
mbelshe: commit-queue-
Details | Formatted Diff | Diff
One more try, without tagalong fixes. (4.50 KB, patch)
2010-06-09 12:00 PDT, Mike Belshe
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Belshe 2010-06-08 10:52:08 PDT
Track whether a page is fetched via a proxy.
Comment 1 Mike Belshe 2010-06-08 17:59:07 PDT
Created attachment 58204 [details]
Simple patch - chromium specific.
Comment 2 David Levin 2010-06-08 18:08:20 PDT
Not sure if it matters but these fields will not be copied across threads (see PassOwnPtr<CrossThreadResourceRequestData> copyData() const;). This is called when workers do resource requests.

Other: The reviewer typically isn't filled in until someone has r+'ed it.
Comment 3 Mike Belshe 2010-06-08 18:32:00 PDT
Yes, CrossThreadResourceRequestData can't deal with platform specific flags.  So this is no change.

re the Reviewer:  I know.  I take a crap-shoot at the reviewer.  If Dimitry reviews it, then I don't need to regenerate a patch.  A better fix would be to get rid of ChangeLogs, but I think that is an issue for the Office of Redundancy to tackle...
Comment 4 David Levin 2010-06-08 19:50:03 PDT
(In reply to comment #3)
> Yes, CrossThreadResourceRequestData can't deal with platform specific flags.  So this is no change.

Well, it doesn't at present. I suppose it should be modified to handle platform specific flags if they matter (when it is called).

Specifically, this method is called when workers do resource requests. Do these fields matter for that case?
Comment 5 Mike Belshe 2010-06-09 10:24:01 PDT
(In reply to comment #4)
> Specifically, this method is called when workers do resource requests. Do these fields matter for that case?

We're trying to track at a page-load-level whether a page is "proxied or not".  This already has room for error, because some resources on a page could be proxied, and others may not.  We use the base-page as the trigger for how to label the page load.  Since we're only looking at page loads, not resources loaded by XHR, workers, etc, it doesn't matter for workers.
Comment 6 David Levin 2010-06-09 10:35:28 PDT
(In reply to comment #5)
> (In reply to comment #4)
> > Specifically, this method is called when workers do resource requests. Do these fields matter for that case?
> 
> We're trying to track at a page-load-level whether a page is "proxied or not".  This already has room for error, because some resources on a page could be proxied, and others may not.  We use the base-page as the trigger for how to label the page load.  Since we're only looking at page loads, not resources loaded by XHR, workers, etc, it doesn't matter for workers.

Thanks!
Comment 7 Mike Belshe 2010-06-09 11:58:56 PDT
Created attachment 58269 [details]
Update the reviewer field .
Comment 8 Mike Belshe 2010-06-09 12:00:21 PDT
Created attachment 58271 [details]
One more try, without tagalong fixes.
Comment 9 WebKit Commit Bot 2010-06-10 06:56:25 PDT
Comment on attachment 58271 [details]
One more try, without tagalong fixes.

Clearing flags on attachment: 58271

Committed r60955: <http://trac.webkit.org/changeset/60955>
Comment 10 Noam Rosenthal 2010-07-29 09:34:01 PDT
*** Bug 43011 has been marked as a duplicate of this bug. ***
Comment 11 Adam Barth 2010-08-10 22:49:55 PDT
This patch claims to have been landed.