|
|
Created:
7 years, 7 months ago by kouhei (in TOK) Modified:
7 years, 6 months ago CC:
chromium-reviews, cbentzel+watch_chromium.org, kenjibaheux, haraken, tburkard, odean, Alexei Svitkine (slow) Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
DescriptionEnable pre-connect via {mouse,gesture}-event triggers to limited users controlled by Finch.
BUG=240959
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=203120
Patch Set 1 #
Total comments: 5
Patch Set 2 : fix style / add doc #
Total comments: 2
Patch Set 3 : no early ret #Messages
Total messages: 17 (0 generated)
jam, mathp: Would you take a look?
Finch trial code lgtm https://codereview.chromium.org/15725006/diff/1/chrome/renderer/net/prescient... File chrome/renderer/net/prescient_networking_dispatcher.cc (right): https://codereview.chromium.org/15725006/diff/1/chrome/renderer/net/prescient... chrome/renderer/net/prescient_networking_dispatcher.cc:21: Document the function? https://codereview.chromium.org/15725006/diff/1/chrome/renderer/net/prescient... chrome/renderer/net/prescient_networking_dispatcher.cc:41: } nit: // end namespace
https://codereview.chromium.org/15725006/diff/1/chrome/renderer/net/prescient... File chrome/renderer/net/prescient_networking_dispatcher.cc (right): https://codereview.chromium.org/15725006/diff/1/chrome/renderer/net/prescient... chrome/renderer/net/prescient_networking_dispatcher.cc:41: } On 2013/05/27 12:34:53, Mathieu Perreault wrote: > nit: // end namespace or rather // namespace, sorry
mathp: Thanks for review! jam: Would you take a look? https://codereview.chromium.org/15725006/diff/1/chrome/renderer/net/prescient... File chrome/renderer/net/prescient_networking_dispatcher.cc (right): https://codereview.chromium.org/15725006/diff/1/chrome/renderer/net/prescient... chrome/renderer/net/prescient_networking_dispatcher.cc:21: On 2013/05/27 12:34:53, Mathieu Perreault wrote: > Document the function? Done. https://codereview.chromium.org/15725006/diff/1/chrome/renderer/net/prescient... chrome/renderer/net/prescient_networking_dispatcher.cc:41: } On 2013/05/27 12:50:48, Mathieu Perreault wrote: > On 2013/05/27 12:34:53, Mathieu Perreault wrote: > > nit: // end namespace > > or rather // namespace, sorry Done.
On 2013/05/28 03:09:00, kouhei wrote: > mathp: Thanks for review! > > jam: Would you take a look? please see the comment in chrome/OWNERS
thakis: Would you review this patch?
lgtm; optional nit below https://codereview.chromium.org/15725006/diff/6001/chrome/renderer/net/presci... File chrome/renderer/net/prescient_networking_dispatcher.cc (right): https://codereview.chromium.org/15725006/diff/6001/chrome/renderer/net/presci... chrome/renderer/net/prescient_networking_dispatcher.cc:56: content::RenderThread::Get()->Send(new ChromeViewHostMsg_Preconnect(url)); If it fits in 80 cols, write if (isPrec…) RenderThread...->Send(…) instead of the early return (unless you plan on adding more code down here)
Thanks for quick review! https://codereview.chromium.org/15725006/diff/6001/chrome/renderer/net/presci... File chrome/renderer/net/prescient_networking_dispatcher.cc (right): https://codereview.chromium.org/15725006/diff/6001/chrome/renderer/net/presci... chrome/renderer/net/prescient_networking_dispatcher.cc:56: content::RenderThread::Get()->Send(new ChromeViewHostMsg_Preconnect(url)); On 2013/05/30 00:26:23, Nico wrote: > If it fits in 80 cols, write > > if (isPrec…) > RenderThread...->Send(…) > > instead of the early return (unless you plan on adding more code down here) Done.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kouhei@chromium.org/15725006/13001
Sorry for I got bad news for ya. Compile failed with a clobber build on linux_aura. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura... Your code is likely broken or HEAD is junk. Please ensure your code is not broken then alert the build sheriffs. Look at the try server FAQ for more details.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kouhei@chromium.org/15725006/13001
Sorry for I got bad news for ya. Compile failed with a clobber build on linux_chromeos. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro... Your code is likely broken or HEAD is junk. Please ensure your code is not broken then alert the build sheriffs. Look at the try server FAQ for more details.
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kouhei@chromium.org/15725006/13001
Retried try job too often on linux_chromeos for step(s) browser_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/kouhei@chromium.org/15725006/13001
Message was sent while issue was closed.
Change committed as 203120
Message was sent while issue was closed.
On 2013/05/30 12:18:10, I haz the power (commit-bot) wrote: > Change committed as 203120 It turned out that this patch was incomplete. Trying to fix the bug in http://crrev.com/17749008 |