Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
334553002:
Unflake PrerenderOmniboxBrowserTest.PrerenderOmniboxAbandon. (Closed)
|
Created:
6 years, 6 months ago by davidben Modified:
6 years, 6 months ago Reviewers:
mmenke CC:
chromium-reviews, tburkard+watch_chromium.org, gavinp+prer_chromium.org, cbentzel+watch_chromium.org, dominich+watch_chromium.org, davidben+watch_chromium.org Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionUnflake PrerenderOmniboxBrowserTest.PrerenderOmniboxAbandon.
The AutocompleteActionPredictor cancels prerenders in its destructor, so the
cancel and app terminating signals are likely racing. Disable the final status
check as it's not relevant to this test.
BUG=368721
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276796
Patch Set 1 #
Messages
Total messages: 17 (0 generated)
LGTM, though the way the test started the second prerender does seem a little weird.
On 2014/06/11 23:02:21, mmenke wrote: > LGTM, though the way the test started the second prerender does seem a little > weird. Second prerender?
On 2014/06/11 23:40:30, David Benjamin wrote: > On 2014/06/11 23:02:21, mmenke wrote: > > LGTM, though the way the test started the second prerender does seem a little > > weird. > > Second prerender? Erm...yes. One's trigger via putting junk in the omnibox, and then another's triggered by StartOmniboxPrerender...Or does the omnibox not do a real prerender in the test? If not...why the heck are we using a different URL when calling StartOmniboxPrerender?
On 2014/06/11 23:47:43, mmenke wrote: > On 2014/06/11 23:40:30, David Benjamin wrote: > > On 2014/06/11 23:02:21, mmenke wrote: > > > LGTM, though the way the test started the second prerender does seem a > little > > > weird. > > > > Second prerender? > > Erm...yes. One's trigger via putting junk in the omnibox, and then another's > triggered by StartOmniboxPrerender...Or does the omnibox not do a real prerender > in the test? If not...why the heck are we using a different URL when calling > StartOmniboxPrerender? Oh...I guess we're just starting one prerender and navigating to another URL through a rather bizarre method. I had thought there was a second prerender was to force the prerender triggered by the omnibox to be abandoned, but I guess there is no prerender triggered by the cruft we stick in the omnibox. I was wondering about the abandoned comment...
On 2014/06/11 23:47:43, mmenke wrote: > On 2014/06/11 23:40:30, David Benjamin wrote: > > On 2014/06/11 23:02:21, mmenke wrote: > > > LGTM, though the way the test started the second prerender does seem a > little > > > weird. > > > > Second prerender? > > Erm...yes. One's trigger via putting junk in the omnibox, and then another's > triggered by StartOmniboxPrerender...Or does the omnibox not do a real prerender > in the test? If not...why the heck are we using a different URL when calling > StartOmniboxPrerender? Oh. It's prerendering one URL via omnibox, but navigating to another. If they matched, we'd swap, and the goal is to test that, should there be a miss, the prerender is marked as abandoned so it doesn't last that long.
On 2014/06/11 23:50:56, mmenke wrote: > On 2014/06/11 23:47:43, mmenke wrote: > > On 2014/06/11 23:40:30, David Benjamin wrote: > > > On 2014/06/11 23:02:21, mmenke wrote: > > > > LGTM, though the way the test started the second prerender does seem a > > little > > > > weird. > > > > > > Second prerender? > > > > Erm...yes. One's trigger via putting junk in the omnibox, and then another's > > triggered by StartOmniboxPrerender...Or does the omnibox not do a real > prerender > > in the test? If not...why the heck are we using a different URL when calling > > StartOmniboxPrerender? > > Oh...I guess we're just starting one prerender and navigating to another URL > through a rather bizarre method. I had thought there was a second prerender was > to force the prerender triggered by the omnibox to be abandoned, but I guess > there is no prerender triggered by the cruft we stick in the omnibox. I was > wondering about the abandoned comment... Hrm...Guess this is actually the normal path, I had just it was handled by some sort of callback, rather than telling some random per-profile thing to get to it.
On 2014/06/11 23:59:12, mmenke wrote: > On 2014/06/11 23:50:56, mmenke wrote: > > On 2014/06/11 23:47:43, mmenke wrote: > > > On 2014/06/11 23:40:30, David Benjamin wrote: > > > > On 2014/06/11 23:02:21, mmenke wrote: > > > > > LGTM, though the way the test started the second prerender does seem a > > > little > > > > > weird. > > > > > > > > Second prerender? > > > > > > Erm...yes. One's trigger via putting junk in the omnibox, and then > another's > > > triggered by StartOmniboxPrerender...Or does the omnibox not do a real > > prerender > > > in the test? If not...why the heck are we using a different URL when > calling > > > StartOmniboxPrerender? > > > > Oh...I guess we're just starting one prerender and navigating to another URL > > through a rather bizarre method. I had thought there was a second prerender > was > > to force the prerender triggered by the omnibox to be abandoned, but I guess > > there is no prerender triggered by the cruft we stick in the omnibox. I was > > wondering about the abandoned comment... > > Hrm...Guess this is actually the normal path, I had just it was handled by some > sort of callback, rather than telling some random per-profile thing to get to > it. Anyhow, now that I understand the test, still LGTM.
The CQ bit was checked by davidben@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/davidben@chromium.org/334553002/1
On 2014/06/11 23:50:56, mmenke wrote: > On 2014/06/11 23:47:43, mmenke wrote: > > On 2014/06/11 23:40:30, David Benjamin wrote: > > > On 2014/06/11 23:02:21, mmenke wrote: > > > > LGTM, though the way the test started the second prerender does seem a > > little > > > > weird. > > > > > > Second prerender? > > > > Erm...yes. One's trigger via putting junk in the omnibox, and then another's > > triggered by StartOmniboxPrerender...Or does the omnibox not do a real > prerender > > in the test? If not...why the heck are we using a different URL when calling > > StartOmniboxPrerender? > > Oh...I guess we're just starting one prerender and navigating to another URL > through a rather bizarre method. I had thought there was a second prerender was > to force the prerender triggered by the omnibox to be abandoned, but I guess > there is no prerender triggered by the cruft we stick in the omnibox. I was > wondering about the abandoned comment... Sorry about that rather irate sounding email.
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: android_dbg_triggered_tests on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_dbg_triggered...) ios_dbg_simulator on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_dbg_simulator/bui...) ios_rel_device on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device/builds...) ios_rel_device_ninja on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/ios_rel_device_ninja/...) mac_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...) mac_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...) win_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...) win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...) win_chromium_x64_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...) mac_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/mac_gpu/builds/15805) win_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18936)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_gpu on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu/builds/18964)
The CQ bit was checked by davidben@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/davidben@chromium.org/334553002/1
Message was sent while issue was closed.
Change committed as 276796 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
