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

Issue 1258593002: Reland "Add ExecuteJavaScriptForTest and make all tests use it" (Closed)

Created:
5 years, 5 months ago by jochen (gone - plz use gerrit)
Modified:
5 years, 4 months ago
Reviewers:
Mike West, jam, Dan Beam
CC:
aboxhall+watch_chromium.org, android-webview-reviews_chromium.org, asanka, benjhayden+dwatch_chromium.org, cbentzel+watch_chromium.org, chromium-apps-reviews_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, davemoore+watch_chromium.org, davidben+watch_chromium.org, devtools-reviews_chromium.org, dmazzoni+watch_chromium.org, dtseng+watch_chromium.org, dzhioev+watch_chromium.org, eroman, estade+watch_chromium.org, extensions-reviews_chromium.org, gavinp+prer_chromium.org, gcasto+watchlist_chromium.org, grt+watch_chromium.org, jam, je_julie, mkwst+watchlist-passwords_chromium.org, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, mlamouri+watch-geolocation_chromium.org, mmenke, Michael van Ouwerkerk, nasko+codewatch_chromium.org, nektar+watch_chromium.org, oshima+watch_chromium.org, pfeldman, plundblad+watch_chromium.org, rouslan+autofillwatch_chromium.org, stevenjb+watch_chromium.org, tburkard+watch_chromium.org, tfarina, Lei Zhang, tommycli, vabr+watchlist_chromium.org, yurys, yuzo+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Reland "Add ExecuteJavaScriptForTest and make all tests use it" Original issue's description: > Additionally, restrict the URLs that ExecuteJavaScript can be invoked on > to chrome-controlled URLs. > > R=jam@chromium.org > BUG=507809 > > Review URL: https://codereview.chromium.org/1123783002 > > Cr-Commit-Position: refs/heads/master@{#340231} R=mkwst@chromium.org TBR=jam@chromium.org BUG=507809 CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel Committed: https://chromium.googlesource.com/chromium/src/+/14ea977ff2e300fd131d3ca8ff6f5311f5fd1df8

Patch Set 1 #

Patch Set 2 : fix #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+305 lines, -150 lines) Patch
M android_webview/browser/aw_browser_main_parts.cc View 2 chunks +3 lines, -0 lines 0 comments Download
M android_webview/java/src/org/chromium/android_webview/AwContents.java View 1 chunk +16 lines, -0 lines 0 comments Download
M android_webview/javatests/src/org/chromium/android_webview/test/AwContentsTest.java View 1 chunk +1 line, -1 line 0 comments Download
M android_webview/javatests/src/org/chromium/android_webview/test/AwJavaBridgeTest.java View 1 chunk +1 line, -1 line 0 comments Download
M android_webview/javatests/src/org/chromium/android_webview/test/AwTestBase.java View 1 chunk +1 line, -1 line 0 comments Download
M android_webview/javatests/src/org/chromium/android_webview/test/GeolocationTest.java View 5 chunks +5 lines, -5 lines 0 comments Download
M android_webview/javatests/src/org/chromium/android_webview/test/util/JSUtils.java View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/JavaScriptEvalChromeTest.java View 2 chunks +4 lines, -4 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/ModalDialogTest.java View 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/android/javatests/src/org/chromium/chrome/browser/TabsTest.java View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/browser/apps/custom_launcher_page_browsertest_views.cc View 5 chunks +6 lines, -6 lines 0 comments Download
M chrome/browser/chromeos/login/lock/screen_locker_tester.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/download/download_browsertest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/errorpage_browsertest.cc View 7 chunks +8 lines, -8 lines 0 comments Download
M chrome/browser/extensions/alert_apitest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/media_galleries/media_galleries_watch_apitest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/extensions/api/web_navigation/web_navigation_apitest.cc View 2 chunks +4 lines, -2 lines 0 comments Download
M chrome/browser/geolocation/geolocation_browsertest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/mouseleave_browsertest.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/prerender/prerender_browsertest.cc View 3 chunks +7 lines, -4 lines 0 comments Download
M chrome/browser/safe_browsing/safe_browsing_blocking_page_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/task_manager/task_manager_browsertest.cc View 1 chunk +2 lines, -1 line 0 comments Download
M chrome/browser/ui/blocked_content/popup_blocker_browsertest.cc View 2 chunks +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/browser_browsertest.cc View 7 chunks +7 lines, -7 lines 0 comments Download
M chrome/browser/ui/views/select_file_dialog_extension_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/chromeos/login/gaia_screen_handler.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M chrome/browser/ui/webui/net_internals/net_internals_ui_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/ui/webui/web_ui_test_handler.cc View 2 chunks +5 lines, -4 lines 0 comments Download
M chrome/renderer/autofill/autofill_renderer_browsertest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/renderer/autofill/form_autocomplete_browsertest.cc View 5 chunks +8 lines, -7 lines 0 comments Download
M chrome/renderer/autofill/form_autofill_browsertest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/renderer/autofill/page_click_tracker_browsertest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/renderer/autofill/password_autofill_agent_browsertest.cc View 2 chunks +2 lines, -2 lines 0 comments Download
M chrome/renderer/autofill/password_generation_agent_browsertest.cc View 3 chunks +5 lines, -4 lines 0 comments Download
M components/dom_distiller/content/browser/distiller_page_web_contents_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/dom_distiller/content/browser/test/dom_distiller_js_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M components/printing/test/print_web_view_helper_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/accessibility/accessibility_win_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/accessibility/dump_accessibility_events_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/browser/frame_host/render_frame_host_impl.h View 2 chunks +7 lines, -0 lines 0 comments Download
M content/browser/frame_host/render_frame_host_impl.cc View 6 chunks +40 lines, -0 lines 1 comment Download
M content/browser/web_contents/web_contents_android.h View 1 chunk +4 lines, -0 lines 0 comments Download
M content/browser/web_contents/web_contents_android.cc View 1 chunk +33 lines, -0 lines 0 comments Download
M content/public/android/java/src/org/chromium/content/browser/webcontents/WebContentsImpl.java View 2 chunks +8 lines, -1 line 0 comments Download
M content/public/android/java/src/org/chromium/content_public/browser/WebContents.java View 1 chunk +16 lines, -1 line 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/ContentViewLocationTest.java View 4 chunks +4 lines, -4 lines 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/JavaBridgeBareboneTest.java View 1 chunk +1 line, -1 line 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/JavaBridgeChildFrameTest.java View 1 chunk +1 line, -1 line 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/NavigationTest.java View 2 chunks +4 lines, -2 lines 0 comments Download
M content/public/android/javatests/src/org/chromium/content/browser/TestsJavaScriptEvalTest.java View 1 chunk +1 line, -1 line 0 comments Download
M content/public/browser/render_frame_host.h View 2 chunks +14 lines, -2 lines 0 comments Download
M content/public/test/android/javatests/src/org/chromium/content/browser/test/util/JavaScriptUtils.java View 2 chunks +2 lines, -2 lines 0 comments Download
M content/public/test/android/javatests/src/org/chromium/content/browser/test/util/TestCallbackHelperContainer.java View 2 chunks +3 lines, -3 lines 0 comments Download
M content/public/test/render_view_test.h View 1 chunk +1 line, -1 line 0 comments Download
M content/public/test/render_view_test.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/public/test/test_utils.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/accessibility/renderer_accessibility_browsertest.cc View 3 chunks +6 lines, -6 lines 0 comments Download
M content/renderer/devtools/v8_sampling_profiler_browsertest.cc View 1 chunk +1 line, -1 line 0 comments Download
M content/renderer/render_view_browsertest.cc View 15 chunks +23 lines, -21 lines 0 comments Download
M content/renderer/render_view_browsertest_mac.mm View 3 chunks +5 lines, -5 lines 0 comments Download
M content/shell/browser/shell_devtools_frontend.cc View 1 4 chunks +4 lines, -4 lines 0 comments Download
M extensions/browser/api/system_storage/system_storage_eject_apitest.cc View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 7 (1 generated)
jochen (gone - plz use gerrit)
ptal patch was reverted because devtools in content_shell runs on file:// or http:// instead of ...
5 years, 5 months ago (2015-07-24 11:10:09 UTC) #1
Mike West
lgtm
5 years, 5 months ago (2015-07-24 11:37:15 UTC) #2
jochen (gone - plz use gerrit)
Committed patchset #2 (id:20001) manually as 14ea977ff2e300fd131d3ca8ff6f5311f5fd1df8 (tree was closed).
5 years, 5 months ago (2015-07-24 12:06:06 UTC) #3
Dan Beam
https://codereview.chromium.org/1258593002/diff/20001/content/browser/frame_host/render_frame_host_impl.cc File content/browser/frame_host/render_frame_host_impl.cc (right): https://codereview.chromium.org/1258593002/diff/20001/content/browser/frame_host/render_frame_host_impl.cc#newcode284 content/browser/frame_host/render_frame_host_impl.cc:284: CHECK(CanExecuteJavaScript()); getting triggered when opening chrome://downloads (shortly after starting ...
5 years, 4 months ago (2015-07-27 20:39:47 UTC) #5
jochen (gone - plz use gerrit)
On 2015/07/27 at 20:39:47, dbeam wrote: > https://codereview.chromium.org/1258593002/diff/20001/content/browser/frame_host/render_frame_host_impl.cc > File content/browser/frame_host/render_frame_host_impl.cc (right): > > https://codereview.chromium.org/1258593002/diff/20001/content/browser/frame_host/render_frame_host_impl.cc#newcode284 ...
5 years, 4 months ago (2015-07-27 20:41:43 UTC) #6
Dan Beam
5 years, 4 months ago (2015-07-27 20:46:18 UTC) #7
Message was sent while issue was closed.
On 2015/07/27 20:41:43, jochen wrote:
> On 2015/07/27 at 20:39:47, dbeam wrote:
> >
>
https://codereview.chromium.org/1258593002/diff/20001/content/browser/frame_h...
> > File content/browser/frame_host/render_frame_host_impl.cc (right):
> > 
> >
>
https://codereview.chromium.org/1258593002/diff/20001/content/browser/frame_h...
> > content/browser/frame_host/render_frame_host_impl.cc:284:
> CHECK(CanExecuteJavaScript());
> > getting triggered when opening chrome://downloads (shortly after starting
> chrome)
> 
> can you file a bug with a stacktrace please?

yeah, working on that right now (found in release build, no symbols, rebuilding
debug).  hopefully will have a fix as well.

The source of the issue is this is probably getting called before it should:
https://code.google.com/p/chromium/codesearch#chromium/src/chrome/browser/ui/...

Powered by Google App Engine
This is Rietveld 408576698