Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(348)

Issue 1427243003: Revert of Let RenderFrameImpl set navigationStart based on CommonNavigationParams (Closed)

Created:
5 years, 1 month ago by nasko
Modified:
5 years, 1 month ago
CC:
blink-reviews, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, gavinp+loader_chromium.org, jam, Nate Chapin, loading-reviews_chromium.org, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, tyoshino+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@navigation_start_common_params
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Let RenderFrameImpl set navigationStart based on CommonNavigationParams (patchset #2 id:20001 of https://codereview.chromium.org/1423453007/ ) Reason for revert: Test fails on the bots: http://build.chromium.org/p/chromium.win/builders/Win7%20Tests%20%28dbg%29%281%29/builds/43015 RenderViewImplTest.NavigationStartOverride (run #1): [ RUN ] RenderViewImplTest.NavigationStartOverride [3788:2828:1105/152333:16934314:WARNING:resource_bundle.cc(305)] locale_file_path.empty() for locale c:uild\slave\win_builder__dbg_uild\src\content\renderer\render_view_browsertest.cc(2282): error: Expected: (early_nav_reported_start) < (before_navigation), actual: 2015-11-05 23:23:33.730 UTC vs 2015-11-05 23:23:33.728 UTC [ FAILED ] RenderViewImplTest.NavigationStartOverride (663 ms) Original issue's description: > Let RenderFrameImpl set navigationStart based on CommonNavigationParams > > This entails setting the timestamp before we set it in blink, so DocumentLoadTiming had to be modified to allow for that. We set the value in didCreateDataSource. > > didCreateDataSource was also refactored slightly so that same-page navigations don't need to call it anymore. > > This is 2/3 of the effort to add a navigation_start timestamp to DidStartProvisional load. The full CL we're breaking up is here: > https://codereview.chromium.org/1425823002/ > > BUG=548335 > > Committed: https://crrev.com/5f61ecb107fd7118621a99fe03b094f37ce04394 > Cr-Commit-Position: refs/heads/master@{#358144} TBR=jochen@chromium.org,clamy@chromium.org,csharrison@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=548335 Committed: https://crrev.com/ef4aafa5cad8305289dc041fd148158205e0a7df Cr-Commit-Position: refs/heads/master@{#358192}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+46 lines, -80 lines) Patch
M content/renderer/render_frame_impl.h View 1 chunk +0 lines, -4 lines 0 comments Download
M content/renderer/render_frame_impl.cc View 9 chunks +44 lines, -66 lines 0 comments Download
M third_party/WebKit/Source/core/loader/DocumentLoadTiming.cpp View 2 chunks +2 lines, -10 lines 0 comments Download

Messages

Total messages: 4 (0 generated)
nasko
Created Revert of Let RenderFrameImpl set navigationStart based on CommonNavigationParams
5 years, 1 month ago (2015-11-06 00:04:14 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1427243003/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1427243003/1
5 years, 1 month ago (2015-11-06 00:08:21 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 1 month ago (2015-11-06 00:11:36 UTC) #3
commit-bot: I haz the power
5 years, 1 month ago (2015-11-06 00:12:42 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/ef4aafa5cad8305289dc041fd148158205e0a7df
Cr-Commit-Position: refs/heads/master@{#358192}

Powered by Google App Engine
This is Rietveld 408576698