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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 295083002: BrowserPluginGuest is no longer a WebContentsDelegate (Closed)

Created:
6 years, 7 months ago by Fady Samuel
Modified:
6 years, 6 months ago
Reviewers:
lazyboy, jam
CC:
chromium-reviews, darin-cc_chromium.org, jam
Base URL:
https://chromium.googlesource.com/chromium/src.git@make_context_menu_easier_to_refactor
Visibility:
Public.

Description

BrowserPluginGuest is no longer a WebContentsDelegate BUG=364141, 330264 TBR=jam@chromium.org for code deletion in content/public/browser_plugin_guest_delegate.{cc|h} Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272889

Patch Set 1 #

Patch Set 2 : Fixed focus after crash #

Total comments: 8

Patch Set 3 : Addressed comments #

Patch Set 4 : More cleanup #

Patch Set 5 : Even more cleanup #

Patch Set 6 : Fixed content_browsertests #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+67 lines, -486 lines) Patch
M chrome/browser/guest_view/guest_view_base.h View 1 3 chunks +9 lines, -5 lines 0 comments Download
M chrome/browser/guest_view/guest_view_base.cc View 1 2 chunks +6 lines, -0 lines 0 comments Download
M chrome/browser/guest_view/guest_view_manager.cc View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M chrome/browser/guest_view/web_view/web_view_guest.h View 1 3 chunks +28 lines, -20 lines 0 comments Download
M chrome/browser/guest_view/web_view/web_view_guest.cc View 1 2 3 4 5 10 chunks +14 lines, -8 lines 0 comments Download
M content/browser/browser_plugin/browser_plugin_guest.h View 1 2 3 6 chunks +5 lines, -66 lines 0 comments Download
M content/browser/browser_plugin/browser_plugin_guest.cc View 1 2 3 7 chunks +0 lines, -181 lines 0 comments Download
M content/browser/browser_plugin/test_browser_plugin_guest_delegate.h View 1 2 3 4 5 1 chunk +1 line, -0 lines 0 comments Download
M content/browser/web_contents/web_contents_impl.h View 1 2 3 4 5 1 chunk +0 lines, -1 line 0 comments Download
M content/public/browser/browser_plugin_guest_delegate.h View 1 2 5 chunks +3 lines, -158 lines 1 comment Download
M content/public/browser/browser_plugin_guest_delegate.cc View 1 2 3 4 1 chunk +0 lines, -46 lines 0 comments Download

Messages

Total messages: 18 (0 generated)
Fady Samuel
This is still a work in progress, but I thought you might want to take ...
6 years, 7 months ago (2014-05-20 23:12:37 UTC) #1
Fady Samuel
This is now ready for review. +jam@ for content/public change. Horray for code deletion.
6 years, 7 months ago (2014-05-23 15:50:37 UTC) #2
lazyboy
Overall, remove all the unused includes and fwd declarations. https://chromiumcodereview.appspot.com/295083002/diff/40001/content/browser/browser_plugin/browser_plugin_guest.h File content/browser/browser_plugin/browser_plugin_guest.h (right): https://chromiumcodereview.appspot.com/295083002/diff/40001/content/browser/browser_plugin/browser_plugin_guest.h#newcode79 content/browser/browser_plugin/browser_plugin_guest.h:79: ...
6 years, 7 months ago (2014-05-23 16:06:09 UTC) #3
Fady Samuel
Addressed comments PTAL. https://codereview.chromium.org/295083002/diff/40001/content/browser/browser_plugin/browser_plugin_guest.h File content/browser/browser_plugin/browser_plugin_guest.h (right): https://codereview.chromium.org/295083002/diff/40001/content/browser/browser_plugin/browser_plugin_guest.h#newcode79 content/browser/browser_plugin/browser_plugin_guest.h:79: class CONTENT_EXPORT BrowserPluginGuest On 2014/05/23 16:06:09, ...
6 years, 7 months ago (2014-05-23 16:34:08 UTC) #4
lazyboy
Thanks, LGTM
6 years, 7 months ago (2014-05-23 16:47:00 UTC) #5
Fady Samuel
The CQ bit was checked by fsamuel@chromium.org
6 years, 7 months ago (2014-05-26 13:42:40 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fsamuel@chromium.org/295083002/100001
6 years, 7 months ago (2014-05-26 13:43:09 UTC) #7
Fady Samuel
The CQ bit was checked by fsamuel@chromium.org
6 years, 6 months ago (2014-05-26 14:32:16 UTC) #8
Fady Samuel
The CQ bit was unchecked by fsamuel@chromium.org
6 years, 6 months ago (2014-05-26 14:32:16 UTC) #9
Fady Samuel
The CQ bit was checked by fsamuel@chromium.org
6 years, 6 months ago (2014-05-26 14:32:17 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fsamuel@chromium.org/295083002/120001
6 years, 6 months ago (2014-05-26 14:33:43 UTC) #11
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are ...
6 years, 6 months ago (2014-05-26 16:32:01 UTC) #12
commit-bot: I haz the power
The CQ bit was unchecked by commit-bot@chromium.org
6 years, 6 months ago (2014-05-26 16:40:10 UTC) #13
commit-bot: I haz the power
Try jobs failed on following builders: android_aosp on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/android_aosp/builds/77659)
6 years, 6 months ago (2014-05-26 16:40:10 UTC) #14
Fady Samuel
The CQ bit was checked by fsamuel@chromium.org
6 years, 6 months ago (2014-05-26 17:53:45 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fsamuel@chromium.org/295083002/120001
6 years, 6 months ago (2014-05-26 17:54:41 UTC) #16
commit-bot: I haz the power
Change committed as 272889
6 years, 6 months ago (2014-05-27 00:01:17 UTC) #17
jam
6 years, 6 months ago (2014-05-27 01:37:41 UTC) #18
Message was sent while issue was closed. 
https://codereview.chromium.org/295083002/diff/120001/content/public/browser/...
File content/public/browser/browser_plugin_guest_delegate.h (right):

https://codereview.chromium.org/295083002/diff/120001/content/public/browser/...
content/public/browser/browser_plugin_guest_delegate.h:33: //
WebContentsDelegate.
what about this one?

Powered by Google App Engine
This is Rietveld 408576698