Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2264973002:
Adjust callers and networking delegates in extensions/ to modified APIs (Closed)
|
Created:
4 years, 4 months ago by maksims (do not use this acc) Modified:
4 years, 3 months ago CC:
chromium-reviews, chromium-apps-reviews_chromium.org, extensions-reviews_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@URLRequestRead Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdjust callers and networking delegates in extensions/ to modified APIs
Use modified Read and delegate methods from the following CL-
https://codereview.chromium.org/2262653003/
ExtensionWebRequestEventRouter::OnResponseStarted,
OnCompleted and OnErrorOccured are needed to be overloaded,
because some callers in chrome/ are using these old methods.
These methods will be completely removed in ~ 1 week once
https://codereview.chromium.org/2264903003/ is upstreamed
as well.
BUG=423484
Committed: https://crrev.com/450052de137a95b0a956bc77964ea97605de40af
Cr-Commit-Position: refs/heads/master@{#416585}
Patch Set 1 #
Total comments: 1
Messages
Total messages: 36 (19 generated)
The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Adjust callers and networking delegates in extensions/ to modified APIs BUG= ========== to ========== Adjust callers and networking delegates in extensions/ to modified APIs Use modified Read and delegate methods from the following CL- https://codereview.chromium.org/2262653003/ BUG=423484 ==========
maksim.sisov@intel.com changed reviewers: + finnur@chromium.org, jamescook@chromium.org
PTAL
Patchset #1 (id:1) has been deleted
CC mmenke@
Description was changed from ========== Adjust callers and networking delegates in extensions/ to modified APIs Use modified Read and delegate methods from the following CL- https://codereview.chromium.org/2262653003/ BUG=423484 ========== to ========== Adjust callers and networking delegates in extensions/ to modified APIs Use modified Read and delegate methods from the following CL- https://codereview.chromium.org/2262653003/ ExtensionWebRequestEventRouter::OnResponseStarted, OnCompleted and OnErrorOccured are needed to be overloaded, because some callers in chrome are using these old methods. These methods will be completely removed in ~ 1 week once https://codereview.chromium.org/2264903003/ is upstreamed as well. BUG=423484 ==========
Description was changed from ========== Adjust callers and networking delegates in extensions/ to modified APIs Use modified Read and delegate methods from the following CL- https://codereview.chromium.org/2262653003/ ExtensionWebRequestEventRouter::OnResponseStarted, OnCompleted and OnErrorOccured are needed to be overloaded, because some callers in chrome are using these old methods. These methods will be completely removed in ~ 1 week once https://codereview.chromium.org/2264903003/ is upstreamed as well. BUG=423484 ========== to ========== Adjust callers and networking delegates in extensions/ to modified APIs Use modified Read and delegate methods from the following CL- https://codereview.chromium.org/2262653003/ ExtensionWebRequestEventRouter::OnResponseStarted, OnCompleted and OnErrorOccured are needed to be overloaded, because some callers in chrome/ are using these old methods. These methods will be completely removed in ~ 1 week once https://codereview.chromium.org/2264903003/ is upstreamed as well. BUG=423484 ==========
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by maksim.sisov@intel.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Patchset #1 (id:20001) has been deleted
Rubberstamp extensions LGTM (when someone who knows networking better has green-lighted this).
The CQ bit was checked by maksim.sisov@intel.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Adjust callers and networking delegates in extensions/ to modified APIs Use modified Read and delegate methods from the following CL- https://codereview.chromium.org/2262653003/ ExtensionWebRequestEventRouter::OnResponseStarted, OnCompleted and OnErrorOccured are needed to be overloaded, because some callers in chrome/ are using these old methods. These methods will be completely removed in ~ 1 week once https://codereview.chromium.org/2264903003/ is upstreamed as well. BUG=423484 ========== to ========== Adjust callers and networking delegates in extensions/ to modified APIs Use modified Read and delegate methods from the following CL- https://codereview.chromium.org/2262653003/ ExtensionWebRequestEventRouter::OnResponseStarted, OnCompleted and OnErrorOccured are needed to be overloaded, because some callers in chrome/ are using these old methods. These methods will be completely removed in ~ 1 week once https://codereview.chromium.org/2264903003/ is upstreamed as well. BUG=423484 ==========
Message was sent while issue was closed.
Committed patchset #1 (id:40001)
Message was sent while issue was closed.
Description was changed from ========== Adjust callers and networking delegates in extensions/ to modified APIs Use modified Read and delegate methods from the following CL- https://codereview.chromium.org/2262653003/ ExtensionWebRequestEventRouter::OnResponseStarted, OnCompleted and OnErrorOccured are needed to be overloaded, because some callers in chrome/ are using these old methods. These methods will be completely removed in ~ 1 week once https://codereview.chromium.org/2264903003/ is upstreamed as well. BUG=423484 ========== to ========== Adjust callers and networking delegates in extensions/ to modified APIs Use modified Read and delegate methods from the following CL- https://codereview.chromium.org/2262653003/ ExtensionWebRequestEventRouter::OnResponseStarted, OnCompleted and OnErrorOccured are needed to be overloaded, because some callers in chrome/ are using these old methods. These methods will be completely removed in ~ 1 week once https://codereview.chromium.org/2264903003/ is upstreamed as well. BUG=423484 Committed: https://crrev.com/450052de137a95b0a956bc77964ea97605de40af Cr-Commit-Position: refs/heads/master@{#416585} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/450052de137a95b0a956bc77964ea97605de40af Cr-Commit-Position: refs/heads/master@{#416585}
Message was sent while issue was closed.
Umm... I asked for this not to be checked in until someone who knows networking better can review. Can you find someone to greenlight this code?
Message was sent while issue was closed.
On 2016/09/06 09:54:14, Finnur wrote: > Umm... I asked for this not to be checked in until someone who knows networking > better can review. > > Can you find someone to greenlight this code? Sorry, I missunderstood you.
Message was sent while issue was closed.
A revert of this CL (patchset #1 id:40001) has been created in https://codereview.chromium.org/2315673002/ by maksim.sisov@intel.com. The reason for reverting is: waiting for someone who knows networking code better.
Message was sent while issue was closed.
Description was changed from ========== Adjust callers and networking delegates in extensions/ to modified APIs Use modified Read and delegate methods from the following CL- https://codereview.chromium.org/2262653003/ ExtensionWebRequestEventRouter::OnResponseStarted, OnCompleted and OnErrorOccured are needed to be overloaded, because some callers in chrome/ are using these old methods. These methods will be completely removed in ~ 1 week once https://codereview.chromium.org/2264903003/ is upstreamed as well. BUG=423484 Committed: https://crrev.com/450052de137a95b0a956bc77964ea97605de40af Cr-Commit-Position: refs/heads/master@{#416585} ========== to ========== Adjust callers and networking delegates in extensions/ to modified APIs Use modified Read and delegate methods from the following CL- https://codereview.chromium.org/2262653003/ ExtensionWebRequestEventRouter::OnResponseStarted, OnCompleted and OnErrorOccured are needed to be overloaded, because some callers in chrome/ are using these old methods. These methods will be completely removed in ~ 1 week once https://codereview.chromium.org/2264903003/ is upstreamed as well. BUG=423484 Committed: https://crrev.com/450052de137a95b0a956bc77964ea97605de40af Cr-Commit-Position: refs/heads/master@{#416585} ==========
maksim.sisov@intel.com changed reviewers: + mmenke@chromium.org
Matt, even though it has already been upstreamed, would you please take a look into this? There was a misunderstanding between me and finnur@. Please check whether I should revert it or not.
https://codereview.chromium.org/2264973002/diff/40001/extensions/browser/api/... File extensions/browser/api/web_request/web_request_api.h (right): https://codereview.chromium.org/2264973002/diff/40001/extensions/browser/api/... extensions/browser/api/web_request/web_request_api.h:242: net::URLRequest* request); Why are we keeping both the old and new versions around? This CL is small enough that it seems like we should just remove the old ones in this CL.
On 2016/09/06 17:21:50, mmenke wrote: > https://codereview.chromium.org/2264973002/diff/40001/extensions/browser/api/... > File extensions/browser/api/web_request/web_request_api.h (right): > > https://codereview.chromium.org/2264973002/diff/40001/extensions/browser/api/... > extensions/browser/api/web_request/web_request_api.h:242: net::URLRequest* > request); > Why are we keeping both the old and new versions around? This CL is small > enough that it seems like we should just remove the old ones in this CL. Oh, and the code looks correct.
On 2016/09/06 17:21:50, mmenke wrote: > https://codereview.chromium.org/2264973002/diff/40001/extensions/browser/api/... > File extensions/browser/api/web_request/web_request_api.h (right): > > https://codereview.chromium.org/2264973002/diff/40001/extensions/browser/api/... > extensions/browser/api/web_request/web_request_api.h:242: net::URLRequest* > request); > Why are we keeping both the old and new versions around? This CL is small > enough that it seems like we should just remove the old ones in this CL. Otherwise I will have to combine this one with changes in chrome/ folder. I did not want to mix everything together. When there are too many reviewers, it gets hard to track what, when, who and etc.
Cronet LGTM
Oops...wrong CL.
On 2016/09/06 17:57:44, mmenke wrote: > Oops...wrong CL. That having been said... This does LGTM, though I would have suggested including that one part of the chrome CL as part of this one, to avoid having to do a followup.
extensions/shell LGTM |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
