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

Issue 1194383003: Add a flag showing whether the current request was ignored by a handler (Closed)

Created:
5 years, 6 months ago by gsennton
Modified:
5 years, 5 months ago
CC:
chromium-reviews, extensions-reviews_chromium.org, David Trainor- moved to gerrit, nasko+codewatch_chromium.org, creis+watch_chromium.org, mlamouri+watch-content_chromium.org, jam, dzhioev+watch_chromium.org, avayvod+watch_chromium.org, cbentzel+watch_chromium.org, yurys, darin-cc_chromium.org, pfeldman, oshima+watch_chromium.org, devtools-reviews_chromium.org, mkwst+moarreviews-renderer_chromium.org, chromium-apps-reviews_chromium.org, stevenjb+watch_chromium.org, android-webview-reviews_chromium.org, davemoore+watch_chromium.org, mnaganov (inactive)
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Add a flag showing whether the current request was ignored by a handler When converting to using a resource throttle to call shouldOverrideUrlLoading in WebView we need to know whether the current request was ignored because of shouldOverrideUrlLoading or something else. This is because we need to know when to post OnPageFinished. This is an initial CL that simply passes the flag where it needs to be -- the actual move to using a resource throttle will be added in a follow-up CL to make that CL more easily reviewable/updatable. The flag is passed down from WebURLLoaderImpl::Context::OnCompletedRequest in content/child/web_url_loader_impl.cc all the way to AwWebContentsObserver.didFailLoad so that we can make the right decision there. A large part of the flag-passing is handled through adding a flag to WebURLError and ResourceError in Blink: https://codereview.chromium.org/1178273007/ TBR=jochen@chromium.org,fsamuel@chromium.org BUG=325351 Committed: https://crrev.com/6fbb38696709063437ffeccad69b34de401c55c6 Cr-Commit-Position: refs/heads/master@{#335974}

Patch Set 1 #

Patch Set 2 : Fix trybot failures (DidFailProvisionalLoad calls in unit tests) #

Unified diffs Side-by-side diffs Delta from patch set Stats (+194 lines, -100 lines) Patch
M android_webview/java/src/org/chromium/android_webview/AwWebContentsObserver.java View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/Tab.java View 1 chunk +1 line, -1 line 0 comments Download
M chrome/android/java/src/org/chromium/chrome/browser/tab/BackgroundContentViewHelper.java View 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/android/java_staging/src/org/chromium/chrome/browser/tab/ChromeTab.java View 1 chunk +2 lines, -2 lines 0 comments Download
M chrome/browser/after_startup_task_utils.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/captive_portal/captive_portal_tab_helper.h View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/captive_portal/captive_portal_tab_helper.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/captive_portal/captive_portal_tab_helper_unittest.cc View 1 10 chunks +23 lines, -14 lines 0 comments Download
M chrome/browser/chromeos/first_run/drive_first_run_controller.cc View 3 chunks +8 lines, -4 lines 0 comments Download
M chrome/browser/chromeos/login/ui/webui_login_view.h View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/chromeos/login/ui/webui_login_view.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/errorpage_browsertest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/identity/web_auth_flow.h View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/identity/web_auth_flow.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/extensions/api/web_navigation/web_navigation_api.h View 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/extensions/api/web_navigation/web_navigation_api.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/net/net_error_tab_helper.h View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/net/net_error_tab_helper.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/net/net_error_tab_helper_unittest.cc View 1 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ssl/ssl_browser_tests.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/omnibox/omnibox_navigation_observer.h View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/omnibox/omnibox_navigation_observer.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/chromeos/mobile_setup_ui.h View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/webui/chromeos/mobile_setup_ui.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/test/base/in_process_browser_test_browsertest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M components/dom_distiller/content/distiller_page_web_contents.h View 1 chunk +2 lines, -1 line 0 comments Download
M components/dom_distiller/content/distiller_page_web_contents.cc View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/android/web_contents_observer_proxy.h View 2 chunks +6 lines, -3 lines 0 comments Download
M content/browser/android/web_contents_observer_proxy.cc View 3 chunks +10 lines, -6 lines 0 comments Download
M content/browser/devtools/render_frame_devtools_agent_host.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/devtools/render_frame_devtools_agent_host.cc View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/frame_host/navigation_controller_impl_browsertest.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M content/browser/frame_host/navigator.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/frame_host/navigator_delegate.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/frame_host/navigator_impl.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/frame_host/navigator_impl.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/frame_host/render_frame_host_impl.cc View 1 chunk +4 lines, -2 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 2 chunks +6 lines, -3 lines 0 comments Download
M content/browser/web_contents/web_contents_impl_unittest.cc View 2 chunks +3 lines, -2 lines 0 comments Download
M content/child/web_url_loader_impl.cc View 1 chunk +2 lines, -1 line 0 comments Download
M content/child/web_url_request_util.h View 1 chunk +6 lines, -0 lines 0 comments Download
M content/child/web_url_request_util.cc View 1 chunk +10 lines, -0 lines 0 comments Download
M content/common/frame_messages.h View 2 chunks +6 lines, -2 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java View 1 chunk +1 line, -1 line 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsObserverProxy.java View 1 chunk +3 lines, -3 lines 0 comments Download
M content/public/android/java/src/org/chromium/content_public/browser/WebContentsObserver.java View 1 chunk +1 line, -1 line 0 comments Download
M content/public/browser/web_contents_observer.h View 2 chunks +4 lines, -2 lines 0 comments Download
M content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestWebContentsObserver.java View 1 chunk +3 lines, -2 lines 0 comments Download
M content/public/test/test_navigation_observer.cc View 1 chunk +2 lines, -1 line 0 comments Download
M content/renderer/render_frame_impl.cc View 2 chunks +3 lines, -1 line 0 comments Download
M content/test/test_web_contents.h View 1 chunk +2 lines, -1 line 0 comments Download
M content/test/test_web_contents.cc View 1 chunk +3 lines, -2 lines 0 comments Download
M content/test/web_contents_observer_sanity_checker.h View 2 chunks +4 lines, -2 lines 0 comments Download
M content/test/web_contents_observer_sanity_checker.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M extensions/browser/guest_view/web_view/web_view_guest.h View 1 chunk +2 lines, -1 line 0 comments Download
M extensions/browser/guest_view/web_view/web_view_guest.cc View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 24 (9 generated)
gsennton
Hi, so this is a monstrosity... the only files where the flag is not only ...
5 years, 6 months ago (2015-06-22 19:29:57 UTC) #2
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1194383003/1
5 years, 6 months ago (2015-06-22 20:44:00 UTC) #4
commit-bot: I haz the power
Dry run: Try jobs failed on following builders: mac_chromium_rel_ng on tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_ng/builds/79657)
5 years, 6 months ago (2015-06-22 21:34:28 UTC) #6
sgurun-gerrit only
On 2015/06/22 21:34:28, commit-bot: I haz the power wrote: > Dry run: Try jobs failed ...
5 years, 6 months ago (2015-06-22 21:35:49 UTC) #7
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1194383003/20001
5 years, 6 months ago (2015-06-23 09:44:43 UTC) #9
commit-bot: I haz the power
Dry run: No L-G-T-M from a valid reviewer yet. Only full committers are accepted. Even ...
5 years, 6 months ago (2015-06-23 09:44:47 UTC) #11
jam
lgtm
5 years, 6 months ago (2015-06-23 15:28:20 UTC) #12
sgurun-gerrit only
On 2015/06/23 15:28:20, jam wrote: > lgtm lgtm, you may want to check if this ...
5 years, 6 months ago (2015-06-23 15:34:59 UTC) #13
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1194383003/20001
5 years, 6 months ago (2015-06-23 16:00:39 UTC) #15
commit-bot: I haz the power
Exceeded global retry quota
5 years, 6 months ago (2015-06-23 16:55:09 UTC) #17
gsennton
On 2015/06/23 16:55:09, commit-bot: I haz the power wrote: > Exceeded global retry quota AFAICT ...
5 years, 6 months ago (2015-06-23 17:04:58 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1194383003/20001
5 years, 6 months ago (2015-06-24 16:57:16 UTC) #20
commit-bot: I haz the power
Committed patchset #2 (id:20001)
5 years, 6 months ago (2015-06-24 19:24:28 UTC) #21
commit-bot: I haz the power
Patchset 2 (id:??) landed as https://crrev.com/6fbb38696709063437ffeccad69b34de401c55c6 Cr-Commit-Position: refs/heads/master@{#335974}
5 years, 6 months ago (2015-06-24 19:25:21 UTC) #22
gsennton
5 years, 5 months ago (2015-07-06 10:33:47 UTC) #24
Message was sent while issue was closed.
Realized that I hadn't added the TBRs as reviewers..
jochen@ for chrome/
and
fsamuel@ for extensions/browser/guest_view/

Powered by Google App Engine
This is Rietveld 408576698