Commit c6bb3aba authored by Randy Rossi's avatar Randy Rossi Committed by Commit Bot

Add id to cast web contents observer

Added a id to replace tab_id as unique
identifier for the webviews that are created from the
platform views service.  The tab_id was always 0
because is_root_frame is true on creation.
Subsequent lookups for the ax tree id by tab id
were only working by chance. The linear search for the
tab id among the list of windows always matches the
first one found, which was the correct one.  This ensures
tab ids will be unique.

Bug: None
Test: display assistant created webviews followed by navigation
      causing render frames to be swapped

Change-Id: Ib6b9f1fd217367423f15965e5b760df1f170caa8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2342640
Commit-Queue: Randy Rossi <rmrossi@chromium.org>
Reviewed-by: default avatarDaniel Nicoara <dnicoara@chromium.org>
Cr-Commit-Position: refs/heads/master@{#796903}
parent a844a70d
...@@ -269,6 +269,10 @@ class CastWebContents { ...@@ -269,6 +269,10 @@ class CastWebContents {
// same tab ID at any given time. // same tab ID at any given time.
virtual int tab_id() const = 0; virtual int tab_id() const = 0;
// An identifier for the WebContents, mainly used by platform views service.
// IDs may be re-used but are unique among all live CastWebContents.
virtual int id() const = 0;
// TODO(seantopping): Hide this, clients shouldn't use WebContents directly. // TODO(seantopping): Hide this, clients shouldn't use WebContents directly.
virtual content::WebContents* web_contents() const = 0; virtual content::WebContents* web_contents() const = 0;
virtual PageState page_state() const = 0; virtual PageState page_state() const = 0;
......
...@@ -53,6 +53,9 @@ namespace { ...@@ -53,6 +53,9 @@ namespace {
// IDs start at 1, since 0 is reserved for the root content window. // IDs start at 1, since 0 is reserved for the root content window.
size_t next_tab_id = 1; size_t next_tab_id = 1;
// Next id for id()
size_t next_id = 0;
// Remove the given CastWebContents pointer from the global instance vector. // Remove the given CastWebContents pointer from the global instance vector.
void RemoveCastWebContents(CastWebContents* instance) { void RemoveCastWebContents(CastWebContents* instance) {
auto& all_cast_web_contents = CastWebContents::GetAll(); auto& all_cast_web_contents = CastWebContents::GetAll();
...@@ -129,6 +132,7 @@ CastWebContentsImpl::CastWebContentsImpl(content::WebContents* web_contents, ...@@ -129,6 +132,7 @@ CastWebContentsImpl::CastWebContentsImpl(content::WebContents* web_contents,
activity_url_filter_(std::move(init_params.url_filters)), activity_url_filter_(std::move(init_params.url_filters)),
main_process_host_(nullptr), main_process_host_(nullptr),
tab_id_(init_params.is_root_window ? 0 : next_tab_id++), tab_id_(init_params.is_root_window ? 0 : next_tab_id++),
id_(next_id++),
is_websql_enabled_(init_params.enable_websql), is_websql_enabled_(init_params.enable_websql),
is_mixer_audio_enabled_(init_params.enable_mixer_audio), is_mixer_audio_enabled_(init_params.enable_mixer_audio),
main_frame_loaded_(false), main_frame_loaded_(false),
...@@ -185,6 +189,10 @@ int CastWebContentsImpl::tab_id() const { ...@@ -185,6 +189,10 @@ int CastWebContentsImpl::tab_id() const {
return tab_id_; return tab_id_;
} }
int CastWebContentsImpl::id() const {
return id_;
}
content::WebContents* CastWebContentsImpl::web_contents() const { content::WebContents* CastWebContentsImpl::web_contents() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
return web_contents_; return web_contents_;
......
...@@ -56,6 +56,7 @@ class CastWebContentsImpl : public CastWebContents, ...@@ -56,6 +56,7 @@ class CastWebContentsImpl : public CastWebContents,
// CastWebContents implementation: // CastWebContents implementation:
int tab_id() const override; int tab_id() const override;
int id() const override;
void AddRendererFeatures(std::vector<RendererFeature> features) override; void AddRendererFeatures(std::vector<RendererFeature> features) override;
void AllowWebAndMojoWebUiBindings() override; void AllowWebAndMojoWebUiBindings() override;
void ClearRenderWidgetHostView() override; void ClearRenderWidgetHostView() override;
...@@ -166,6 +167,7 @@ class CastWebContentsImpl : public CastWebContents, ...@@ -166,6 +167,7 @@ class CastWebContentsImpl : public CastWebContents,
std::vector<RendererFeature> renderer_features_; std::vector<RendererFeature> renderer_features_;
const int tab_id_; const int tab_id_;
const int id_;
bool is_websql_enabled_; bool is_websql_enabled_;
bool is_mixer_audio_enabled_; bool is_mixer_audio_enabled_;
base::TimeTicks start_loading_ticks_; base::TimeTicks start_loading_ticks_;
......
...@@ -17,6 +17,7 @@ class MockCastWebContents : public CastWebContents { ...@@ -17,6 +17,7 @@ class MockCastWebContents : public CastWebContents {
// CastWebContents implementation // CastWebContents implementation
MOCK_METHOD(int, tab_id, (), (const, override)); MOCK_METHOD(int, tab_id, (), (const, override));
MOCK_METHOD(int, id, (), (const, override));
MOCK_METHOD(content::WebContents*, web_contents, (), (const, override)); MOCK_METHOD(content::WebContents*, web_contents, (), (const, override));
MOCK_METHOD(PageState, page_state, (), (const, override)); MOCK_METHOD(PageState, page_state, (), (const, override));
MOCK_METHOD(QueryableDataHost*, queryable_data_host, (), (const, override)); MOCK_METHOD(QueryableDataHost*, queryable_data_host, (), (const, override));
......
...@@ -76,12 +76,12 @@ WebviewController::WebviewController(content::BrowserContext* browser_context, ...@@ -76,12 +76,12 @@ WebviewController::WebviewController(content::BrowserContext* browser_context,
std::unique_ptr<webview::WebviewResponse> response = std::unique_ptr<webview::WebviewResponse> response =
std::make_unique<webview::WebviewResponse>(); std::make_unique<webview::WebviewResponse>();
// For webviews, set the ax_id to be the cast_web_contents' tab id rather than // For webviews, set the ax_id to be the cast_web_contents' id
// the ax tree id for the main frame. The main frame can be replaced after // rather than the ax tree id for the main frame. The main frame can be
// we've set this from navigation. Prefix the string with "T:" to tell the ax // replaced after we've set this from navigation. Prefix the string with
// bridge to find the cast_web_contents by tab id. Then it can find the // "T:" to tell the ax bridge to find the cast_web_contents by id.
// current ax tree id from that. // Then it can find the current ax tree id from that.
std::string ax_id = "T:" + base::NumberToString(cast_web_contents_->tab_id()); std::string ax_id = "T:" + base::NumberToString(cast_web_contents_->id());
response->mutable_create_response() response->mutable_create_response()
->mutable_accessibility_info() ->mutable_accessibility_info()
->set_ax_tree_id(ax_id); ->set_ax_tree_id(ax_id);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment