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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 9924026: Browser side implementation of browser plugin (Closed)

Created:
8 years, 8 months ago by Fady Samuel
Modified:
8 years, 8 months ago
Reviewers:
jam
CC:
chromium-reviews, joi+watch-content_chromium.org, darin-cc_chromium.org, jam, rjkroege, piman
Visibility:
Public.

Description

Browser side implementation of browser plugin BUG=117897 TEST=manually Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=131054

Patch Set 1 #

Patch Set 2 : Updated RenderProcessHostImpl #

Total comments: 27

Patch Set 3 : A lot of code simplification. #

Patch Set 4 : Added TabContents change. #

Patch Set 5 : Updated message names #

Total comments: 18

Patch Set 6 : Addressed jam@'s comments #

Patch Set 7 : Lots of cleanup. Browser Plugin gets its own messages #

Patch Set 8 : Fixed comments in browser_plugin_messages.h #

Total comments: 2

Patch Set 9 : Updated according to jam@'s comments #

Patch Set 10 : Removed an extra semicolon. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+412 lines, -0 lines) Patch
A content/browser/browser_plugin/browser_plugin_web_contents_observer.h View 1 2 3 4 5 6 1 chunk +94 lines, -0 lines 0 comments Download
A content/browser/browser_plugin/browser_plugin_web_contents_observer.cc View 1 2 3 4 5 6 7 8 1 chunk +229 lines, -0 lines 0 comments Download
M content/browser/tab_contents/tab_contents.h View 1 2 3 4 5 6 3 chunks +8 lines, -0 lines 0 comments Download
M content/browser/tab_contents/tab_contents.cc View 1 2 3 4 5 6 2 chunks +4 lines, -0 lines 0 comments Download
A content/common/browser_plugin_messages.h View 1 2 3 4 5 6 7 8 9 1 chunk +72 lines, -0 lines 0 comments Download
M content/common/content_message_generator.h View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M content/content_browser.gypi View 1 2 3 4 5 6 1 chunk +2 lines, -0 lines 0 comments Download
M content/content_common.gypi View 1 2 3 4 5 6 7 8 1 chunk +1 line, -0 lines 0 comments Download
M ipc/ipc_message_utils.h View 1 2 3 4 5 6 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 14 (0 generated)
Fady Samuel
Hi John, As I wrap up work on graphics with Anotine, I was hoping to ...
8 years, 8 months ago (2012-03-29 21:11:32 UTC) #1
Fady Samuel
Adding Brett for the Pepper bits. Thanks.
8 years, 8 months ago (2012-03-29 21:12:43 UTC) #2
jam
(I started reviewing from the bottom file up, and didn't look closely at the last ...
8 years, 8 months ago (2012-03-30 05:44:11 UTC) #3
Fady Samuel
It's late so I'll think about these comments more tomorrow, but for now: >>Why do ...
8 years, 8 months ago (2012-03-30 06:02:08 UTC) #4
Fady Samuel
Hi John, I've moved everything to BrowserPluginWebContentsObserver, and so things have been simplified quite a ...
8 years, 8 months ago (2012-04-03 17:01:45 UTC) #5
jam
http://codereview.chromium.org/9924026/diff/8003/content/browser/browser_plugin/browser_plugin_web_contents_observer.cc File content/browser/browser_plugin/browser_plugin_web_contents_observer.cc (right): http://codereview.chromium.org/9924026/diff/8003/content/browser/browser_plugin/browser_plugin_web_contents_observer.cc#newcode72 content/browser/browser_plugin/browser_plugin_web_contents_observer.cc:72: void BrowserPluginWebContentsObserver::OnMsgGuestReady() { nit: just name handlers OnFoo, no ...
8 years, 8 months ago (2012-04-04 00:54:45 UTC) #6
Fady Samuel
Hi John, I believe I've addressed all your comments. Could you please take another look? ...
8 years, 8 months ago (2012-04-04 16:45:40 UTC) #7
jam
https://chromiumcodereview.appspot.com/9924026/diff/8003/content/browser/browser_plugin/browser_plugin_web_contents_observer.cc File content/browser/browser_plugin/browser_plugin_web_contents_observer.cc (right): https://chromiumcodereview.appspot.com/9924026/diff/8003/content/browser/browser_plugin/browser_plugin_web_contents_observer.cc#newcode83 content/browser/browser_plugin/browser_plugin_web_contents_observer.cc:83: msg->set_unblock(true); On 2012/04/04 16:45:40, Fady Samuel wrote: > On ...
8 years, 8 months ago (2012-04-04 21:07:19 UTC) #8
Fady Samuel
-brettw: No real pepper changes at this point. Hi John, I've done a lot of ...
8 years, 8 months ago (2012-04-05 19:35:40 UTC) #9
jam
lgtm with the two changes https://chromiumcodereview.appspot.com/9924026/diff/23001/content/common/browser_plugin/browser_plugin_messages.h File content/common/browser_plugin/browser_plugin_messages.h (right): https://chromiumcodereview.appspot.com/9924026/diff/23001/content/common/browser_plugin/browser_plugin_messages.h#newcode1 content/common/browser_plugin/browser_plugin_messages.h:1: // Copyright (c) 2012 ...
8 years, 8 months ago (2012-04-05 21:03:08 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/9924026/22012
8 years, 8 months ago (2012-04-05 21:30:10 UTC) #11
commit-bot: I haz the power
Try job failure for 9924026-22012 (retry) on linux_rel for step "compile" (clobber build). It's a ...
8 years, 8 months ago (2012-04-05 22:18:11 UTC) #12
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/fsamuel@chromium.org/9924026/20015
8 years, 8 months ago (2012-04-05 22:29:33 UTC) #13
commit-bot: I haz the power
8 years, 8 months ago (2012-04-06 00:49:08 UTC) #14 
Change committed as 131054

Powered by Google App Engine
This is Rietveld 408576698