New issue
Advanced search Search tips

Issue 841065 link

Starred by 2 users
Status: Available
Owner: ----
Cc:
mfo...@chromium.org
btolsch@chromium.org
powerb@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac
Pri: 3
Type: Bug



Sign in to add a comment

[Presentation API] Replace hard coded routing IDs in PSDImpl unittest

Project Member Reported by mfo...@chromium.org, May 8 2018 
The hard coded IDs will cause the test to fail if they happen to not point to a valid render frame host.  The test should create a test RFH and get the ids from it.

This affects PresentationServiceDelegateImplTest.AddScreenAvailabilityListener and possibly other test cases.

Comment 1 by mfo...@chromium.org, May 8 2018

Summary: [Presentation API] Replace hard coded routing IDs in PSDImpl unittest (was: [Presentation API] Stop using hard coded routing IDs in PresentationServiceDelegateImpl unittest)

Comment 2 by ajwong@chromium.org, May 17 2018 

I've refactored AddScreenAvailabilityListener to remove the portion of the test that's trying to exercise AddScreenAvailabilityListener() with an invalid render id.

In that situation, the invalid render_id DCHECKs.

See CL: https://chromium-review.googlesource.com/c/chromium/src/+/1028846
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 10 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb00463be55d58bab9808bd794df35f7a2367563

commit cb00463be55d58bab9808bd794df35f7a2367563
Author: Albert J. Wong <ajwong@chromium.org>
Date: Tue Jul 10 22:58:25 2018

Assign RenderWidget[Host] and RenderView[Host] different routing IDs

This will facilitate further detangling of RenderWidget and RenderView.
In the current CL, there is an odd setup where a RenderView still
inherits from RenderWidget meaning that a single object
(the RenderView) ends up being assigned 2 different routing ids.
Because prior work already separated out messages into the
appropriates classes, and because RenderView::OnMessageReceived()
delegates up to RenderWidget::OnMessageReceived(), this chimera
object ends up working okay.

Next step is to break the inheritance relationship.


Bug: 545684, 841065
Change-Id: I6548b4f82f8a040bbc1cb6b309ab1e9383004d9c
Reviewed-on: https://chromium-review.googlesource.com/1028846
Commit-Queue: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: Richard Coles <torne@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Reviewed-by: Derek Cheng <imcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#573966}
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/android_webview/browser/aw_contents.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/chrome/browser/media/router/presentation/presentation_service_delegate_impl_unittest.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/frame_host/frame_tree.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/frame_host/frame_tree.h
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/frame_host/frame_tree_unittest.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/frame_host/interstitial_page_impl.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/frame_host/render_frame_host_impl.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/frame_host/render_frame_host_manager.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/renderer_host/render_view_host_factory.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/renderer_host/render_view_host_factory.h
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/renderer_host/render_view_host_impl.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/renderer_host/render_view_host_impl.h
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/renderer_host/render_widget_host_impl.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/renderer_host/render_widget_host_owner_delegate.h
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/browser/web_contents/web_contents_impl.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/public/test/mock_render_thread.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/public/test/render_view_test.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/renderer/render_frame_impl.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/renderer/render_thread_impl.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/renderer/render_view_impl.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/renderer/render_view_impl.h
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/renderer/render_widget.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/renderer/render_widget.h
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/renderer/render_widget_owner_delegate.h
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/test/test_render_view_host.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/test/test_render_view_host.h
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/test/test_render_view_host_factory.cc
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/test/test_render_view_host_factory.h
[modify] https://crrev.com/cb00463be55d58bab9808bd794df35f7a2367563/content/test/test_web_contents.cc

Comment 4 by powerb@chromium.org, Aug 14

Status: Available (was: Untriaged)
►

Sign in to add a comment