Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2867283002:
Wait for network quiescence in loading.mobile pages (Closed)
|
Created:
3 years, 7 months ago by Kunihiko Sakamoto Modified:
3 years, 7 months ago CC:
chromium-reviews, telemetry-reviews_chromium.org, rnephew (Reviews Here), charliea (OOO until 10-5) Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionWait for network quiescence in loading.mobile pages
Some pages in loading.mobile use location.href to redirect to their
mobile site. WaitForDocumentReadyStateToBeComplete() does not wait
such second navigation, so this patch changes PageCyclerStory to
wait for network quiescence.
Note to perf sheriffs: This may affect loading.mobile benchmark, as this
will fix a bug where metrics for some navigations were not recorded.
BUG=716433
Review-Url: https://codereview.chromium.org/2867283002
Cr-Commit-Position: refs/heads/master@{#470805}
Committed: https://chromium.googlesource.com/chromium/src/+/0c42bb9692ce450bee3e9db90982faba46349f3d
Patch Set 1 #
Messages
Total messages: 25 (16 generated)
The CQ bit was checked by ksakamoto@chromium.org 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: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
The CQ bit was checked by ksakamoto@chromium.org 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.
Description was changed from ========== Wait for network quiescence in loading.mobile pages BUG=716433 ========== to ========== Wait for network quiescence in loading.mobile pages Some pages in loading.mobile use location.href to redirect to their mobile site. WaitForDocumentReadyStateToBeComplete() does not wait such second navigation, so this patch changes PageCyclerStory to wait for network quiescence. BUG=716433 ==========
ksakamoto@chromium.org changed reviewers: + kouhei@chromium.org, nednguyen@google.com
lgtm
On 2017/05/09 12:52:13, nednguyen wrote: > lgtm Though can you add a note to the description to let sheriffs know that this may cause some perf regression to loading.mobile?
lgtm
Description was changed from ========== Wait for network quiescence in loading.mobile pages Some pages in loading.mobile use location.href to redirect to their mobile site. WaitForDocumentReadyStateToBeComplete() does not wait such second navigation, so this patch changes PageCyclerStory to wait for network quiescence. BUG=716433 ========== to ========== Wait for network quiescence in loading.mobile pages Some pages in loading.mobile use location.href to redirect to their mobile site. WaitForDocumentReadyStateToBeComplete() does not wait such second navigation, so this patch changes PageCyclerStory to wait for network quiescence. Note to perf sheriffs: This may affect loading.mobile benchmark, as this will fix a bug where metrics for some navigations were not recorded. BUG=716433 ==========
On 2017/05/09 12:52:42, nednguyen wrote: > On 2017/05/09 12:52:13, nednguyen wrote: > > lgtm > > Though can you add a note to the description to let sheriffs know that this may > cause some perf regression to loading.mobile? Done.
The CQ bit was checked by ksakamoto@chromium.org
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
Try jobs failed on following builders: chromium_presubmit on master.tryserver.chromium.linux (JOB_TIMED_OUT, build has not started yet; builder either lacks capacity or does not exist (misspelled?))
The CQ bit was checked by ksakamoto@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 1, "attempt_start_ts": 1494464656965960, "parent_rev":
"d52c912abb8accf430d7ba7e7d368b2638a4c8fa", "commit_rev":
"0c42bb9692ce450bee3e9db90982faba46349f3d"}
Message was sent while issue was closed.
Description was changed from ========== Wait for network quiescence in loading.mobile pages Some pages in loading.mobile use location.href to redirect to their mobile site. WaitForDocumentReadyStateToBeComplete() does not wait such second navigation, so this patch changes PageCyclerStory to wait for network quiescence. Note to perf sheriffs: This may affect loading.mobile benchmark, as this will fix a bug where metrics for some navigations were not recorded. BUG=716433 ========== to ========== Wait for network quiescence in loading.mobile pages Some pages in loading.mobile use location.href to redirect to their mobile site. WaitForDocumentReadyStateToBeComplete() does not wait such second navigation, so this patch changes PageCyclerStory to wait for network quiescence. Note to perf sheriffs: This may affect loading.mobile benchmark, as this will fix a bug where metrics for some navigations were not recorded. BUG=716433 Review-Url: https://codereview.chromium.org/2867283002 Cr-Commit-Position: refs/heads/master@{#470805} Committed: https://chromium.googlesource.com/chromium/src/+/0c42bb9692ce450bee3e9db90982... ==========
Message was sent while issue was closed.
Committed patchset #1 (id:1) as https://chromium.googlesource.com/chromium/src/+/0c42bb9692ce450bee3e9db90982... |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
