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

Issue 972313002: Make <webview> use out-of-process iframe architecture. (Closed)

Created:
5 years, 9 months ago by lazyboy
Modified:
5 years, 4 months ago
CC:
dcheng, alexmos, chromium-apps-reviews_chromium.org, chromium-reviews, creis+watch_chromium.org, darin-cc_chromium.org, extensions-reviews_chromium.org, grt+watch_chromium.org, jam, lfg, michaeln, mkwst+moarreviews-renderer_chromium.org, mlamouri+watch-content_chromium.org, nasko+codewatch_chromium.org, site-isolation-reviews_chromium.org
Base URL:
ssh://saopaulo.wat/mnt/dev/shared/src@testoopif2z-better-chrome
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Make <webview> use out-of-process iframe architecture. This is behind --site-per-process flag. This is the first step to make <webview> --site-per-process compatible, only basic features work: rendering, input, navigation, executeScript and postMessage. This CL makes <webview> BrowserPlugin free, so <webview> is not a plugin to blink anymore, rather it's similar to iframe. Therefore, this CL makes <webview> work without BrowserPlugin or BrowserPluginGuest/BrowserPluginEmbedder. We still need to use the old browser/ counterparts (BPG and BPE) because we use it for 1) Guest related decision making in WebContents, so WebContentsImpl::browser_plugin_guest() is still used and useful, 2) Initializing and attaching logic inside content/, e.g. BPG::InitInternal(), BPG::OnWillAttachComplete. In subsequent CLs, these dependencies will be either removed if not required, or extracted to a separate class. TBR=jochen@chromium.org for testing/buildbot/chromium.fyi.json BUG=330264 Committed: https://crrev.com/6ec48b2a6529b753f28340fd6741908086224bee Cr-Commit-Position: refs/heads/master@{#336565}

Patch Set 1 #

Patch Set 2 : Revert unused changes from previous attempt + more cleanup. #

Total comments: 45

Patch Set 3 : Sync (not good for review) #

Patch Set 4 : remove pseudo_parent_ #

Patch Set 5 : move proxy creation stuff to RFHM from WC #

Patch Set 6 : some review comments addressed #

Total comments: 28

Patch Set 7 : sync @tott #

Patch Set 8 : Address comments #

Patch Set 9 : sync. #

Patch Set 10 : Sync again. #

Patch Set 11 : Make <webview> work without --site-per-process as well #

Total comments: 90

Patch Set 12 : sync only #

Patch Set 13 : Address content/ comment from Charlie (minus RWHVChildF) + git cl format #

Patch Set 14 : not using NPAPI bindings anymore, yay! #

Total comments: 12

Patch Set 15 : address comments/nits #

Patch Set 16 : Sync. #

Patch Set 17 : Sync again #

Patch Set 18 : rebase again #

Patch Set 19 : sync again #

Patch Set 20 : add basic postMessage test #

Total comments: 28

Patch Set 21 : sync #

Patch Set 22 : fix setting RWVHChildFrame + first pass of components/extensions cleanup #

Patch Set 23 : remove setContentWindow #

Patch Set 24 : rm GetGuestByInstanceIDHack and revert some unwanted changes, WebViewTest.* pass w/o --site-per-pro… #

Patch Set 25 : rename msg and move host->GetView()->Show() #

Patch Set 26 : remove test from this CL #

Patch Set 27 : rename changes #

Total comments: 65

Patch Set 28 : Sync #

Patch Set 29 : Fix Keyboard input #

Patch Set 30 : address all comments from Nasko and Charlie, minus is_loading #

Total comments: 31

Patch Set 31 : address content/ nits/comments + git cl format #

Patch Set 32 : Sync after components/ refactor #

Total comments: 15

Patch Set 33 : sync @tott #

Patch Set 34 : address comments from creis@ #

Total comments: 2

Patch Set 35 : fix RFPH::OnDispatch, remove dispatcher.cc changes, use new container, rename ACK IPC #

Total comments: 2

Patch Set 36 : add a todo #

Patch Set 37 : Rebase after swapped out changes major rework with RFP #

Total comments: 12

Patch Set 38 : address comments from nasko@ #

Total comments: 40

Patch Set 39 : sync first. #

Patch Set 40 : address comments from nasko@ + git cl format #

Total comments: 34

Patch Set 41 : Fix postMessage crash, we should be getting those new failing tests to pass now #

Total comments: 1

Patch Set 42 : sync #

Total comments: 6

Patch Set 43 : address comments from Charlie #

Patch Set 44 : Move iframe*guestview to components/ #

Total comments: 14

Patch Set 45 : Address comments move destruction callback to GuestViewContainer #

Total comments: 12

Patch Set 46 : address nits #

Patch Set 47 : sync + update chromium.fyi.json #

Patch Set 48 : guest_wc->can be null #

Patch Set 49 : sync + remove WebViewTest.IndexedDBIsolation as it is not passing on linux_site_isolation #

Patch Set 50 : sync #

Unified diffs Side-by-side diffs Delta from patch set Stats (+764 lines, -72 lines) Patch
M components/guest_view.gypi View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 1 chunk +5 lines, -1 line 0 comments Download
M components/guest_view/browser/guest_view_base.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 3 chunks +10 lines, -0 lines 0 comments Download
M components/guest_view/browser/guest_view_message_filter.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 2 chunks +7 lines, -0 lines 0 comments Download
M components/guest_view/browser/guest_view_message_filter.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 2 chunks +45 lines, -0 lines 0 comments Download
M components/guest_view/common/guest_view_messages.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 2 chunks +14 lines, -0 lines 0 comments Download
M components/guest_view/renderer/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 1 chunk +4 lines, -0 lines 0 comments Download
M components/guest_view/renderer/DEPS View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 1 chunk +1 line, -0 lines 0 comments Download
M components/guest_view/renderer/guest_view_container.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 5 chunks +15 lines, -4 lines 0 comments Download
M components/guest_view/renderer/guest_view_container.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 5 chunks +36 lines, -2 lines 0 comments Download
A components/guest_view/renderer/iframe_guest_view_container.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 1 chunk +30 lines, -0 lines 0 comments Download
A components/guest_view/renderer/iframe_guest_view_container.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 1 chunk +38 lines, -0 lines 0 comments Download
A components/guest_view/renderer/iframe_guest_view_request.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 1 chunk +41 lines, -0 lines 0 comments Download
A components/guest_view/renderer/iframe_guest_view_request.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 1 chunk +46 lines, -0 lines 0 comments Download
M content/browser/browser_plugin/browser_plugin_guest.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 1 chunk +3 lines, -0 lines 0 comments Download
M content/browser/browser_plugin/browser_plugin_guest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 7 chunks +43 lines, -12 lines 0 comments Download
M content/browser/frame_host/cross_process_frame_connector.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 3 chunks +16 lines, -1 line 0 comments Download
M content/browser/frame_host/frame_tree_node.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 1 chunk +2 lines, -0 lines 0 comments Download
M content/browser/frame_host/render_frame_host_manager.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 3 chunks +41 lines, -0 lines 0 comments Download
M content/browser/frame_host/render_frame_host_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 3 chunks +75 lines, -0 lines 0 comments Download
M content/browser/frame_host/render_frame_proxy_host.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 3 chunks +23 lines, -9 lines 0 comments Download
M content/browser/frame_host/render_widget_host_view_child_frame.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 2 chunks +16 lines, -0 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 4 chunks +41 lines, -0 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 8 chunks +95 lines, -2 lines 0 comments Download
M content/public/browser/web_contents.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 1 chunk +6 lines, -0 lines 0 comments Download
M extensions/renderer/guest_view/extensions_guest_view_container.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 2 chunks +0 lines, -5 lines 0 comments Download
M extensions/renderer/guest_view/extensions_guest_view_container.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 2 chunks +0 lines, -28 lines 0 comments Download
M extensions/renderer/guest_view/guest_view_internal_custom_bindings.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 1 chunk +7 lines, -0 lines 0 comments Download
M extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 5 chunks +98 lines, -4 lines 0 comments Download
M extensions/renderer/resources/guest_view/guest_view_iframe.js View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 1 chunk +4 lines, -2 lines 0 comments Download
M testing/buildbot/chromium.fyi.json View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 63 (13 generated)
Fady Samuel
https://chromiumcodereview.appspot.com/972313002/diff/20001/content/browser/browser_plugin/browser_plugin_guest.cc File content/browser/browser_plugin/browser_plugin_guest.cc (right): https://chromiumcodereview.appspot.com/972313002/diff/20001/content/browser/browser_plugin/browser_plugin_guest.cc#newcode5 content/browser/browser_plugin/browser_plugin_guest.cc:5: // Note that all IPC sent out from this ...
5 years, 9 months ago (2015-03-04 20:44:47 UTC) #3
Fady Samuel
https://chromiumcodereview.appspot.com/972313002/diff/20001/content/browser/browser_plugin/browser_plugin_guest.cc File content/browser/browser_plugin/browser_plugin_guest.cc (right): https://chromiumcodereview.appspot.com/972313002/diff/20001/content/browser/browser_plugin/browser_plugin_guest.cc#newcode5 content/browser/browser_plugin/browser_plugin_guest.cc:5: // Note that all IPC sent out from this ...
5 years, 9 months ago (2015-03-04 20:44:48 UTC) #4
Fady Samuel
More comments. Generally I'm OK if the new code paths starts out with broken functionality ...
5 years, 9 months ago (2015-03-04 22:21:06 UTC) #5
Charlie Reis
Thanks for putting this (and the accompanying doc) together. I'm sure I'm still missing a ...
5 years, 9 months ago (2015-03-10 04:09:45 UTC) #7
lazyboy
Patch Set #6 is updated with the recent changes. I've updated the doc to reflect ...
5 years, 8 months ago (2015-04-01 21:47:58 UTC) #9
Charlie Reis
Sorry for the delay; this got buried among other tasks. I've left some comments as ...
5 years, 8 months ago (2015-04-08 23:42:12 UTC) #10
lazyboy
(Comments addressed in Patch Set #8) I've left some comments as I went through the ...
5 years, 8 months ago (2015-04-14 01:38:04 UTC) #11
lazyboy
Hi Charlie, I've updated the CL to get <webview> working in both site-per-process and non ...
5 years, 7 months ago (2015-04-28 17:17:52 UTC) #12
Fady Samuel
I like where this is going. We need to avoid using NPAPI though. I hope ...
5 years, 7 months ago (2015-04-28 17:56:11 UTC) #13
Charlie Reis
Thanks for getting it to work both with --site-per-process and without. Some feedback and things ...
5 years, 7 months ago (2015-04-30 23:06:48 UTC) #15
lazyboy
I've address content/ comments for now (except one comment in render_widget_host_view_child_frame.cc) Major changes: 1) use ...
5 years, 7 months ago (2015-05-05 07:28:15 UTC) #16
nasko
Small wave of mostly nits, since I reviewed only code in content/. https://codereview.chromium.org/972313002/diff/280001/content/browser/frame_host/render_frame_host_manager.cc File content/browser/frame_host/render_frame_host_manager.cc ...
5 years, 7 months ago (2015-05-05 21:34:50 UTC) #17
lazyboy
https://chromiumcodereview.appspot.com/972313002/diff/280001/content/browser/frame_host/render_frame_host_manager.cc File content/browser/frame_host/render_frame_host_manager.cc (right): https://chromiumcodereview.appspot.com/972313002/diff/280001/content/browser/frame_host/render_frame_host_manager.cc#newcode1631 content/browser/frame_host/render_frame_host_manager.cc:1631: DCHECK(base::CommandLine::ForCurrentProcess()->HasSwitch( On 2015/05/05 21:34:49, nasko wrote: > nit: CHECK ...
5 years, 7 months ago (2015-05-05 22:33:54 UTC) #18
Charlie Reis
Glad to see you've started running some tests! Let's add a bug number and update ...
5 years, 7 months ago (2015-05-19 07:12:30 UTC) #19
lazyboy
Patch set #27 Glad to see you've started running some tests! -> I've separated the ...
5 years, 7 months ago (2015-05-21 23:23:48 UTC) #20
nasko
Thanks for keeping at this! It is getting very close. Most of the comments are ...
5 years, 7 months ago (2015-05-22 16:32:28 UTC) #21
Charlie Reis
Thanks; mostly nits below. Please list some of the high level changes happening in the ...
5 years, 7 months ago (2015-05-22 23:44:29 UTC) #22
lazyboy
Most comments are address, some pending issues: 1) is_loading in SwapOut still needs to be ...
5 years, 7 months ago (2015-05-26 16:32:55 UTC) #23
Fady Samuel
https://chromiumcodereview.appspot.com/972313002/diff/600001/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc File extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc (right): https://chromiumcodereview.appspot.com/972313002/diff/600001/extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc#newcode298 extensions/renderer/guest_view/guest_view_internal_custom_bindings.cc:298: new extensions::ExtensionsGuestViewContainer(embedder_parent_frame); Fix this. https://chromiumcodereview.appspot.com/972313002/diff/600001/extensions/renderer/guest_view/guest_view_request.cc File extensions/renderer/guest_view/guest_view_request.cc (right): https://chromiumcodereview.appspot.com/972313002/diff/600001/extensions/renderer/guest_view/guest_view_request.cc#newcode156 ...
5 years, 7 months ago (2015-05-26 16:42:15 UTC) #24
Fady Samuel
https://codereview.chromium.org/972313002/diff/600001/extensions/renderer/guest_view/extensions_guest_view_container.cc File extensions/renderer/guest_view/extensions_guest_view_container.cc (right): https://codereview.chromium.org/972313002/diff/600001/extensions/renderer/guest_view/extensions_guest_view_container.cc#newcode53 extensions/renderer/guest_view/extensions_guest_view_container.cc:53: bool ExtensionsGuestViewContainer::OnMessage(const IPC::Message& message) { Move this code to ...
5 years, 7 months ago (2015-05-26 19:02:28 UTC) #25
Fady Samuel
My general concerns with this CL are as follows: 1. Too many if (base::CommandLine::ForCurrentProcess()->HasSwitch(switches::kSitePerProcess)) { ...
5 years, 7 months ago (2015-05-26 19:08:45 UTC) #26
lfg
Just a couple of comments. https://codereview.chromium.org/972313002/diff/600001/content/browser/frame_host/render_frame_host_manager.cc File content/browser/frame_host/render_frame_host_manager.cc (right): https://codereview.chromium.org/972313002/diff/600001/content/browser/frame_host/render_frame_host_manager.cc#newcode158 content/browser/frame_host/render_frame_host_manager.cc:158: ->GetSiteInstance()); Can this be ...
5 years, 7 months ago (2015-05-26 19:18:15 UTC) #28
nasko
Some more comments, though most are nits. https://codereview.chromium.org/972313002/diff/600001/content/browser/frame_host/render_frame_host_manager.cc File content/browser/frame_host/render_frame_host_manager.cc (right): https://codereview.chromium.org/972313002/diff/600001/content/browser/frame_host/render_frame_host_manager.cc#newcode158 content/browser/frame_host/render_frame_host_manager.cc:158: ->GetSiteInstance()); On ...
5 years, 6 months ago (2015-05-28 22:13:46 UTC) #29
dcheng
Random drive-by. https://codereview.chromium.org/972313002/diff/600001/extensions/browser/guest_view/extensions_guest_view_message_filter.cc File extensions/browser/guest_view/extensions_guest_view_message_filter.cc (right): https://codereview.chromium.org/972313002/diff/600001/extensions/browser/guest_view/extensions_guest_view_message_filter.cc#newcode92 extensions/browser/guest_view/extensions_guest_view_message_filter.cc:92: auto guest = WebViewGuest::FromWebContents(guest_web_contents); Warning: personal opinion ...
5 years, 6 months ago (2015-05-28 22:23:37 UTC) #30
lazyboy
I'm still juggling ideas to fix higher level extensions/ and components/ issues Fady raised, will ...
5 years, 6 months ago (2015-05-29 00:02:25 UTC) #31
Charlie Reis
Mostly nits for content/. There's still a lot here, but glad to see it has ...
5 years, 6 months ago (2015-06-02 18:19:15 UTC) #32
lazyboy
I've talked to Ken and verified that surfaces are working with this CL without any ...
5 years, 6 months ago (2015-06-02 20:15:21 UTC) #33
Charlie Reis
On 2015/06/02 20:15:21, lazyboy wrote: > I've talked to Ken and verified that surfaces are ...
5 years, 6 months ago (2015-06-02 23:55:44 UTC) #34
lazyboy
How is it doing on try jobs? It's great to hear so many more webview ...
5 years, 6 months ago (2015-06-03 01:37:03 UTC) #35
lazyboy
Fixed the RenderFrameProxyHost::OnDetach() issue in patchset #36. Sent patchset #35 to site isolation bots. (#35 ...
5 years, 6 months ago (2015-06-03 22:32:27 UTC) #36
nasko
Some basic comments, as I see there is some more work to be done on ...
5 years, 6 months ago (2015-06-11 00:21:28 UTC) #37
lazyboy
Hi Nasko, There shouldn't be any pending tasks left to do in this CL, other ...
5 years, 6 months ago (2015-06-11 22:20:29 UTC) #40
Fady Samuel
https://chromiumcodereview.appspot.com/972313002/diff/700001/content/browser/browser_plugin/browser_plugin_guest.h File content/browser/browser_plugin/browser_plugin_guest.h (right): https://chromiumcodereview.appspot.com/972313002/diff/700001/content/browser/browser_plugin/browser_plugin_guest.h#newcode82 content/browser/browser_plugin/browser_plugin_guest.h:82: // dropped on the floor since we don't have ...
5 years, 6 months ago (2015-06-11 22:27:21 UTC) #41
nasko
I've made another pass through content/. While there a lots of comments, most of them ...
5 years, 6 months ago (2015-06-15 23:00:14 UTC) #42
alexmos
https://codereview.chromium.org/972313002/diff/790001/testing/buildbot/chromium.fyi.json File testing/buildbot/chromium.fyi.json (left): https://codereview.chromium.org/972313002/diff/790001/testing/buildbot/chromium.fyi.json#oldcode3496 testing/buildbot/chromium.fyi.json:3496: "--gtest_filter=-AppApiTest.*:BlockedAppApiTest.*:BrowserTest.ClearPendingOnFailUnlessNTP:BrowserTest.OtherRedirectsDontForkProcess:BrowserTest.WindowOpenClose:ChromeAppAPITest.*:ChromeRenderProcessHostTest.*:ChromeRenderProcessHostTestWithCommandLine.*:DevToolsExperimentalExtensionTest.*:DevToolsExtensionTest.*:DnsProbeBrowserTest.*:ErrorPageTest.*:ExecuteScriptApiTest.ExecuteScriptPermissions:ExtensionApiTest.ActiveTab:ExtensionApiTest.ChromeRuntimeOpenOptionsPage:ExtensionApiTest.ContentScriptExtensionIframe:ExtensionApiTest.ContentScriptOtherExtensions:ExtensionApiTest.ContentScriptExtensionProcess:ExtensionApiTest.Tabs2:ExtensionApiTest.TabsOnUpdated:ExtensionApiTest.WindowOpenPopupIframe:ExtensionBrowserTest.LoadChromeExtensionsWithOptionsParamWhenEmbedded:ExtensionCrxInstallerTest.InstallDelayedUntilNextUpdate:ExtensionOptionsApiTest.ExtensionCanEmbedOwnOptions:ExtensionWebUITest.CanEmbedExtensionOptions:ExtensionWebUITest.ReceivesExtensionOptionsOnClose:InlineLoginUISafeIframeBrowserTest.*:IsolatedAppTest.*:LaunchWebAuthFlowFunctionTest.*:MimeHandlerViewTest.*:*.NewAvatarMenuEnabledInGuestMode:OptionsUIBrowserTest.*:*PDFExtensionTest.*:PhishingClassifierTest.*:PhishingDOMFeatureExtractorTest.*:PlatformAppUrlRedirectorBrowserTest.*:PopupBlockerBrowserTest.*:PrerenderBrowserTest.*:ProcessManagementTest.*:RedirectTest.*:ReferrerPolicyTest.*:SSLUITest.*:WebNavigationApiTest.CrossProcessFragment:WebNavigationApiTest.ServerRedirectSingleProcess:WebNavigationApiTest.CrossProcessHistory:WebViewCommonTest.*:WebViewDPITest.*:WebViewPluginTest.*:WebViewTest.*:ZoomControllerBrowserTest.*:*.NavigateFromNTPToOptionsInSameTab:*.ProfilesWithoutPagesNotLaunched:*.TabMove:*.WhitelistedExtension:*.NewTabPageURL:*.HomepageLocation:RestoreOnStartupPolicyTest*:PhishingClassifierDelegateTest.*:WebUIWebViewBrowserTest*:WebRtcBrowserTest.RunsAudioVideoWebRTCCallInTwoTabs" Random drive-by comment: please fix this for the ...
5 years, 6 months ago (2015-06-15 23:06:57 UTC) #44
lazyboy
https://codereview.chromium.org/972313002/diff/790001/content/browser/frame_host/cross_process_frame_connector.cc File content/browser/frame_host/cross_process_frame_connector.cc (right): https://codereview.chromium.org/972313002/diff/790001/content/browser/frame_host/cross_process_frame_connector.cc#newcode151 content/browser/frame_host/cross_process_frame_connector.cc:151: // |frame_proxy_in_parent_renderer_->frame_tree_node()->parent()| will fail. On 2015/06/15 23:00:13, nasko (slow) ...
5 years, 6 months ago (2015-06-16 17:59:52 UTC) #45
lazyboy
I've updated the CL to fix the crash I was mentioning in the mtg yesterday. ...
5 years, 6 months ago (2015-06-17 23:43:30 UTC) #46
Charlie Reis
Here's some comments from patch set 40. Largely nits, but I'm concerned about RemoveOuterDelegateFrame. Also, ...
5 years, 6 months ago (2015-06-18 00:13:17 UTC) #47
Fady Samuel
General guestview comment: Whenever things can be generalized across GuestView types, please place in components. ...
5 years, 6 months ago (2015-06-18 04:47:07 UTC) #48
lazyboy
Charlie: I tried to comment on your RemoveOuterDelegateFrame() concern. Let me know if that's enough. ...
5 years, 6 months ago (2015-06-18 22:45:13 UTC) #49
lazyboy
https://chromiumcodereview.appspot.com/972313002/diff/870001/extensions/browser/guest_view/extensions_guest_view_message_filter.cc File extensions/browser/guest_view/extensions_guest_view_message_filter.cc (right): https://chromiumcodereview.appspot.com/972313002/diff/870001/extensions/browser/guest_view/extensions_guest_view_message_filter.cc#newcode101 extensions/browser/guest_view/extensions_guest_view_message_filter.cc:101: auto* guest = WebViewGuest::FromWebContents(guest_web_contents); On 2015/06/18 04:47:07, Fady Samuel ...
5 years, 6 months ago (2015-06-22 19:43:21 UTC) #51
nasko
LGTM with nits https://codereview.chromium.org/972313002/diff/930001/content/browser/frame_host/cross_process_frame_connector.cc File content/browser/frame_host/cross_process_frame_connector.cc (right): https://codereview.chromium.org/972313002/diff/930001/content/browser/frame_host/cross_process_frame_connector.cc#newcode149 content/browser/frame_host/cross_process_frame_connector.cc:149: if (frame_proxy_in_parent_renderer_->frame_tree_node() nit: Can you put ...
5 years, 6 months ago (2015-06-23 13:39:23 UTC) #52
Fady Samuel
https://codereview.chromium.org/972313002/diff/930001/components/guest_view/common/guest_view_messages.h File components/guest_view/common/guest_view_messages.h (right): https://codereview.chromium.org/972313002/diff/930001/components/guest_view/common/guest_view_messages.h#newcode43 components/guest_view/common/guest_view_messages.h:43: // to a <webview>. This message will attach the ...
5 years, 6 months ago (2015-06-23 14:33:01 UTC) #53
lazyboy
https://chromiumcodereview.appspot.com/972313002/diff/930001/components/guest_view/common/guest_view_messages.h File components/guest_view/common/guest_view_messages.h (right): https://chromiumcodereview.appspot.com/972313002/diff/930001/components/guest_view/common/guest_view_messages.h#newcode43 components/guest_view/common/guest_view_messages.h:43: // to a <webview>. This message will attach the ...
5 years, 6 months ago (2015-06-23 23:00:00 UTC) #55
Fady Samuel
LGTM + nits https://codereview.chromium.org/972313002/diff/970001/components/guest_view/browser/guest_view_message_filter.cc File components/guest_view/browser/guest_view_message_filter.cc (right): https://codereview.chromium.org/972313002/diff/970001/components/guest_view/browser/guest_view_message_filter.cc#newcode127 components/guest_view/browser/guest_view_message_filter.cc:127: DCHECK(guest_web_contents); I would consider CHECK'ing here. ...
5 years, 6 months ago (2015-06-24 22:01:10 UTC) #56
lazyboy
https://chromiumcodereview.appspot.com/972313002/diff/970001/components/guest_view/browser/guest_view_message_filter.cc File components/guest_view/browser/guest_view_message_filter.cc (right): https://chromiumcodereview.appspot.com/972313002/diff/970001/components/guest_view/browser/guest_view_message_filter.cc#newcode127 components/guest_view/browser/guest_view_message_filter.cc:127: DCHECK(guest_web_contents); On 2015/06/24 22:01:10, Fady Samuel wrote: > I ...
5 years, 6 months ago (2015-06-24 23:01:51 UTC) #57
lazyboy
FYI, I'm going to CQ this one soon, the bots seems to be happy about ...
5 years, 5 months ago (2015-06-29 14:55:05 UTC) #58
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/972313002/1060001
5 years, 5 months ago (2015-06-29 15:13:30 UTC) #61
commit-bot: I haz the power
Committed patchset #50 (id:1060001)
5 years, 5 months ago (2015-06-29 15:18:22 UTC) #62
commit-bot: I haz the power
5 years, 5 months ago (2015-06-29 15:19:30 UTC) #63
Message was sent while issue was closed.
Patchset 50 (id:??) landed as
https://crrev.com/6ec48b2a6529b753f28340fd6741908086224bee
Cr-Commit-Position: refs/heads/master@{#336565}

Powered by Google App Engine
This is Rietveld 408576698