Remove windowed NPAPI code

Remove windowed NPAPI code

BUG=493212
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/bfb2cebc0c8dacff0b93a37a822babe8c2bc0f5e
Cr-Commit-Position: refs/heads/master@{#382094}

[piman, 2016-03-18 00:25:11]

Description was changed from

==========
Remove windowed NPAPI code

BUG=493212
==========

to

==========
Remove windowed NPAPI code

BUG=493212
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

[piman, 2016-03-18 02:47:09]

Patchset #2 (id:20001) has been deleted

[piman, 2016-03-18 02:47:19]

Patchset #1 (id:1) has been deleted

[piman, 2016-03-18 04:33:23]

piman@chromium.org changed reviewers:
+ dcheng@chromium.org, jam@chromium.org

[piman, 2016-03-18 04:33:24]

jam: please review
sky: chrome/browser OWNER
dcheng: plugin_messages.h, plugin_process_messages.h and view_messages.h

[dcheng, 2016-03-18 06:07:31]

*_messages.* changes lgtm

[jam, 2016-03-18 17:00:19]

lgtm

https://codereview.chromium.org/1815593002/diff/40001/chrome/browser/hang_mon...
File chrome/browser/hang_monitor/hung_plugin_action.cc (right):

https://codereview.chromium.org/1815593002/diff/40001/chrome/browser/hang_mon...
chrome/browser/hang_monitor/hung_plugin_action.cc:46:
l10n_util::GetStringUTF16(IDS_UNKNOWN_PLUGIN_NAME);
we should be able to delete this whole file, given that plugins don't have HWNDs
anymore right?

https://codereview.chromium.org/1815593002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/frame/browser_view.cc (right):

https://codereview.chromium.org/1815593002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/frame/browser_view.cc:467: // TODO(piman): is this still
useful now that windowed plugins are gone?
ah, saw this now. yes i dont see a reason to have this code at this point, let's
remove it (fine in a followup)

https://codereview.chromium.org/1815593002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (left):

https://codereview.chromium.org/1815593002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:2779: //
Additonally the legacy dummy window is needed for accessibility and for
nit: this second part of the comment doesn't make as much sense with the
previous removal. update?

[piman, 2016-03-18 18:49:17]

https://codereview.chromium.org/1815593002/diff/40001/chrome/browser/hang_mon...
File chrome/browser/hang_monitor/hung_plugin_action.cc (right):

https://codereview.chromium.org/1815593002/diff/40001/chrome/browser/hang_mon...
chrome/browser/hang_monitor/hung_plugin_action.cc:46:
l10n_util::GetStringUTF16(IDS_UNKNOWN_PLUGIN_NAME);
On 2016/03/18 17:00:19, jam wrote:
> we should be able to delete this whole file, given that plugins don't have
HWNDs
> anymore right?

I think so... The only reason to keep it is if some crazy third-party thing also
hangs the window, but maybe this is too much of an edge case.

I will remove as a follow-up, unless I hear otherwise.

https://codereview.chromium.org/1815593002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (left):

https://codereview.chromium.org/1815593002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:2779: //
Additonally the legacy dummy window is needed for accessibility and for
On 2016/03/18 17:00:19, jam wrote:
> nit: this second part of the comment doesn't make as much sense with the
> previous removal. update?

I thought we still needed that for legacy mouse/trackpad drivers that expect a
given HWND, completely independent of plugins?
(Note, I'm removing the need for this window for windowless plugins in a follow
up, but it won't fully remove it because of that reason)

[piman, 2016-03-18 18:50:38]

piman@chromium.org changed reviewers:
+ sky@chromium.org

[piman, 2016-03-18 18:50:39]

+sky for chrome/browser. Forgot to add you in the first place

[piman, 2016-03-18 19:32:31]

The CQ bit was checked by piman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-03-18 19:33:03]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1815593002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1815593002/60001

[sky, 2016-03-18 19:44:05]

LGTM - make sure you remove any includes that are stale now (specifically in
chrome/browser/hang_monitor/hung_plugin_action.cc )

[piman, 2016-03-18 19:53:29]

On Fri, Mar 18, 2016 at 12:44 PM, <sky@chromium.org> wrote:

> LGTM - make sure you remove any includes that are stale now (specifically
> in
> chrome/browser/hang_monitor/hung_plugin_action.cc )
>

I have a follow-up to remove hung_plugin_action.cc (and other files), in
https://codereview.chromium.org/1815623004/
Do you want me to fixup this one?


>
> https://codereview.chromium.org/1815593002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[sky, 2016-03-18 20:42:07]

The follow on patch is fine.

On Fri, Mar 18, 2016 at 12:53 PM, Antoine Labour <piman@chromium.org> wrote:
>
>
> On Fri, Mar 18, 2016 at 12:44 PM, <sky@chromium.org> wrote:
>>
>> LGTM - make sure you remove any includes that are stale now (specifically
>> in
>> chrome/browser/hang_monitor/hung_plugin_action.cc )
>
>
> I have a follow-up to remove hung_plugin_action.cc (and other files), in
> https://codereview.chromium.org/1815623004/
> Do you want me to fixup this one?
>
>>
>>
>> https://codereview.chromium.org/1815593002/
>
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2016-03-18 21:22:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-03-18 21:22:06]

Dry run: This issue passed the CQ dry run.

[piman, 2016-03-18 21:23:41]

The CQ bit was checked by piman@chromium.org

[piman, 2016-03-18 21:23:41]

The patchset sent to the CQ was uploaded after l-g-t-m from jam@chromium.org,
dcheng@chromium.org
Link to the patchset: https://codereview.chromium.org/1815593002/#ps60001
(title: "rebase")

[commit-bot: I haz the power, 2016-03-18 21:24:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1815593002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1815593002/60001

[commit-bot: I haz the power, 2016-03-18 21:33:09]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-03-18 21:35:14]

Description was changed from

==========
Remove windowed NPAPI code

BUG=493212
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
==========

to

==========
Remove windowed NPAPI code

BUG=493212
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation

Committed: https://crrev.com/bfb2cebc0c8dacff0b93a37a822babe8c2bc0f5e
Cr-Commit-Position: refs/heads/master@{#382094}
==========

[commit-bot: I haz the power, 2016-03-18 21:35:16]

Patchset 2 (id:??) landed as
https://crrev.com/bfb2cebc0c8dacff0b93a37a822babe8c2bc0f5e
Cr-Commit-Position: refs/heads/master@{#382094}

[jam, 2016-03-18 21:56:16]

https://codereview.chromium.org/1815593002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (left):

https://codereview.chromium.org/1815593002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:2779: //
Additonally the legacy dummy window is needed for accessibility and for
On 2016/03/18 18:49:17, piman wrote:
> On 2016/03/18 17:00:19, jam wrote:
> > nit: this second part of the comment doesn't make as much sense with the
> > previous removal. update?
> 
> I thought we still needed that for legacy mouse/trackpad drivers that expect a
> given HWND, completely independent of plugins?
> (Note, I'm removing the need for this window for windowless plugins in a
follow
> up, but it won't fully remove it because of that reason)

to be clear, I meant specifically that the comment that starts with
"Additionally" was depending on the lines that were deleted right?

[piman, 2016-03-18 22:22:00]

https://codereview.chromium.org/1815593002/diff/40001/content/browser/rendere...
File content/browser/renderer_host/render_widget_host_view_aura.cc (left):

https://codereview.chromium.org/1815593002/diff/40001/content/browser/rendere...
content/browser/renderer_host/render_widget_host_view_aura.cc:2779: //
Additonally the legacy dummy window is needed for accessibility and for
On 2016/03/18 21:56:16, jam wrote:
> On 2016/03/18 18:49:17, piman wrote:
> > On 2016/03/18 17:00:19, jam wrote:
> > > nit: this second part of the comment doesn't make as much sense with the
> > > previous removal. update?
> > 
> > I thought we still needed that for legacy mouse/trackpad drivers that expect
a
> > given HWND, completely independent of plugins?
> > (Note, I'm removing the need for this window for windowless plugins in a
> follow
> > up, but it won't fully remove it because of that reason)
> 
> to be clear, I meant specifically that the comment that starts with
> "Additionally" was depending on the lines that were deleted right?

Oh... I kept it because "... This will be passed as the container window for
windowless plugins.", then, "Additionally, the legacy window is needed...."

It's reworded in https://codereview.chromium.org/1813143002/ anyway