Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
250953005:
Disable load event check on PrerenderPrint. (Closed)
|
Created:
6 years, 8 months ago by davidben Modified:
6 years, 7 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, Nate Chapin Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionDisable load event check on PrerenderPrint.
Causing problems with a pending change in Blink.
BUG=367271
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=266621
Patch Set 1 #
Messages
Total messages: 22 (0 generated)
mmenke: PTAL japhet: FYI
On 2014/04/25 19:28:26, David Benjamin wrote: > mmenke: PTAL > japhet: FYI Why don't we see the number of expected load events in this case?
On 2014/04/25 19:32:31, mmenke wrote: > On 2014/04/25 19:28:26, David Benjamin wrote: > > mmenke: PTAL > > japhet: FYI > > Why don't we see the number of expected load events in this case? We do right now. This guy will change that. https://codereview.chromium.org/252383010/ For prerender's purposes, I don't think we particularly care. It's probably better for us not to be sensitive to this sort of thing.
On 2014/04/25 19:50:15, David Benjamin wrote: > On 2014/04/25 19:32:31, mmenke wrote: > > On 2014/04/25 19:28:26, David Benjamin wrote: > > > mmenke: PTAL > > > japhet: FYI > > > > Why don't we see the number of expected load events in this case? > > We do right now. This guy will change that. > https://codereview.chromium.org/252383010/ > > For prerender's purposes, I don't think we particularly care. It's probably > better for us not to be sensitive to this sort of thing. Why does the test prerender not stop loading? Do we swap too early or what?
On 2014/04/25 19:54:43, mmenke wrote: > On 2014/04/25 19:50:15, David Benjamin wrote: > > On 2014/04/25 19:32:31, mmenke wrote: > > > On 2014/04/25 19:28:26, David Benjamin wrote: > > > > mmenke: PTAL > > > > japhet: FYI > > > > > > Why don't we see the number of expected load events in this case? > > > > We do right now. This guy will change that. > > https://codereview.chromium.org/252383010/ > > > > For prerender's purposes, I don't think we particularly care. It's probably > > better for us not to be sensitive to this sort of thing. > > Why does the test prerender not stop loading? Do we swap too early or what? The test page calls window.print() while the page is loading. Right now we get the ChromeViewHostMsg_CancelPrerenderForPrinting signal before the load event, so the prerender doesn't finish loading. This test is for an aborted prerender.
On 2014/04/25 19:58:37, David Benjamin wrote: > On 2014/04/25 19:54:43, mmenke wrote: > > On 2014/04/25 19:50:15, David Benjamin wrote: > > > On 2014/04/25 19:32:31, mmenke wrote: > > > > On 2014/04/25 19:28:26, David Benjamin wrote: > > > > > mmenke: PTAL > > > > > japhet: FYI > > > > > > > > Why don't we see the number of expected load events in this case? > > > > > > We do right now. This guy will change that. > > > https://codereview.chromium.org/252383010/ > > > > > > For prerender's purposes, I don't think we particularly care. It's probably > > > better for us not to be sensitive to this sort of thing. > > > > Why does the test prerender not stop loading? Do we swap too early or what? > > The test page calls window.print() while the page is loading. Right now we get > the ChromeViewHostMsg_CancelPrerenderForPrinting signal before the load event, > so the prerender doesn't finish loading. This test is for an aborted prerender. Ahh...makes sense. 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/250953005/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on linux_chromium_clang_dbg
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/250953005/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on android_dbg_triggered_tests
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/250953005/1
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: tryserver.chromium on android_dbg_triggered_tests
On 2014/04/28 16:39:27, I haz the power (commit-bot) wrote: > Try jobs failed on following builders: > tryserver.chromium on android_dbg_triggered_tests CQ seems to be getting confused about a previous android_dbg_triggered_tests run or something.
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/250953005/1
Message was sent while issue was closed.
Change committed as 266621 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
