From 1f071893d22e2260b5ebc6c97a02626a9ca945fe Mon Sep 17 00:00:00 2001 From: limasdf Date: Tue, 9 Feb 2016 02:18:00 -0800 Subject: Get change property value when tabs.onUpdated is fired. When chrome.tabs.onupdated is fired, we get tab values for second parameter (which privacy sensitive values are already scrubed). That means we can just get changed value from it. So just pass changed property name(std::set<>) only instead of property key-value(base::DictionaryValue). BUG=582484 TEST=browser_tests --gtest_filter=ExtensionApiTest.* Review URL: https://codereview.chromium.org/1639253009 Cr-Commit-Position: refs/heads/master@{#374345} --- .../extensions/api/tabs/tabs_event_router.cc | 130 +++++++++------------ .../extensions/api/tabs/tabs_event_router.h | 32 ++--- 2 files changed, 65 insertions(+), 97 deletions(-) (limited to 'chrome/browser/extensions/api') diff --git a/chrome/browser/extensions/api/tabs/tabs_event_router.cc b/chrome/browser/extensions/api/tabs/tabs_event_router.cc index 3eadeab..c2558a1 100644 --- a/chrome/browser/extensions/api/tabs/tabs_event_router.cc +++ b/chrome/browser/extensions/api/tabs/tabs_event_router.cc @@ -36,22 +36,24 @@ namespace tabs = api::tabs; bool WillDispatchTabUpdatedEvent( WebContents* contents, - const base::DictionaryValue* changed_properties, + const std::set changed_property_names, content::BrowserContext* context, const Extension* extension, Event* event, const base::DictionaryValue* listener_filter) { - // Overwrite the second argument with the appropriate properties dictionary, - // depending on extension permissions. - base::DictionaryValue* properties_value = changed_properties->DeepCopy(); - ExtensionTabUtil::ScrubTabValueForExtension(contents, - extension, - properties_value); - event->event_args->Set(1, properties_value); - - // Overwrite the third arg with our tab value as seen by this extension. - event->event_args->Set(2, - ExtensionTabUtil::CreateTabValue(contents, extension)); + base::DictionaryValue* tab_value = + ExtensionTabUtil::CreateTabValue(contents, extension); + + scoped_ptr changed_properties( + new base::DictionaryValue); + const base::Value* value = nullptr; + for (const auto& property : changed_property_names) { + if (tab_value->Get(property, &value)) + changed_properties->Set(property, make_scoped_ptr(value->DeepCopy())); + } + + event->event_args->Set(1, changed_properties.release()); + event->event_args->Set(2, tab_value); return true; } @@ -65,45 +67,19 @@ TabsEventRouter::TabEntry::TabEntry(TabsEventRouter* router, was_muted_(contents->IsAudioMuted()), router_(router) {} -scoped_ptr TabsEventRouter::TabEntry::UpdateLoadState() { +std::set TabsEventRouter::TabEntry::UpdateLoadState() { // The tab may go in & out of loading (for instance if iframes navigate). // We only want to respond to the first change from loading to !loading after // the NavigationEntryCommitted() was fired. - scoped_ptr changed_properties( - new base::DictionaryValue()); if (!complete_waiting_on_load_ || web_contents()->IsLoading()) { - return changed_properties; + return std::set(); } - // Send "complete" state change. + // Send 'status' of tab change. Expecting 'complete' is fired. complete_waiting_on_load_ = false; - changed_properties->SetString(tabs_constants::kStatusKey, - tabs_constants::kStatusValueComplete); - return changed_properties; -} - -scoped_ptr TabsEventRouter::TabEntry::DidNavigate() { - // Send "loading" state change. - complete_waiting_on_load_ = true; - scoped_ptr changed_properties( - new base::DictionaryValue()); - changed_properties->SetString(tabs_constants::kStatusKey, - tabs_constants::kStatusValueLoading); - - if (web_contents()->GetURL() != url_) { - url_ = web_contents()->GetURL(); - changed_properties->SetString(tabs_constants::kUrlKey, url_.spec()); - } - - return changed_properties; -} - -scoped_ptr TabsEventRouter::TabEntry::TitleChanged() { - scoped_ptr changed_properties( - new base::DictionaryValue()); - changed_properties->SetString(tabs_constants::kTitleKey, - web_contents()->GetTitle()); - return changed_properties; + std::set changed_property_names; + changed_property_names.insert(tabs_constants::kStatusKey); + return changed_property_names; } bool TabsEventRouter::TabEntry::SetAudible(bool new_val) { @@ -122,12 +98,24 @@ bool TabsEventRouter::TabEntry::SetMuted(bool new_val) { void TabsEventRouter::TabEntry::NavigationEntryCommitted( const content::LoadCommittedDetails& load_details) { - router_->TabUpdated(this, DidNavigate()); + // Send 'status' of tab change. Expecting 'loading' is fired. + complete_waiting_on_load_ = true; + std::set changed_property_names; + changed_property_names.insert(tabs_constants::kStatusKey); + + if (web_contents()->GetURL() != url_) { + url_ = web_contents()->GetURL(); + changed_property_names.insert(tabs_constants::kUrlKey); + } + + router_->TabUpdated(this, std::move(changed_property_names)); } void TabsEventRouter::TabEntry::TitleWasSet(content::NavigationEntry* entry, bool explicit_set) { - router_->TabUpdated(this, TitleChanged()); + std::set changed_property_names; + changed_property_names.insert(tabs_constants::kTitleKey); + router_->TabUpdated(this, std::move(changed_property_names)); } void TabsEventRouter::TabEntry::WebContentsDestroyed() { @@ -391,40 +379,31 @@ void TabsEventRouter::TabMoved(WebContents* contents, std::move(args), EventRouter::USER_GESTURE_UNKNOWN); } -void TabsEventRouter::TabUpdated( - TabEntry* entry, - scoped_ptr changed_properties) { - CHECK(entry->web_contents()); - +void TabsEventRouter::TabUpdated(TabEntry* entry, + std::set changed_property_names) { bool audible = entry->web_contents()->WasRecentlyAudible(); if (entry->SetAudible(audible)) { - changed_properties->SetBoolean(tabs_constants::kAudibleKey, audible); + changed_property_names.insert(tabs_constants::kAudibleKey); } bool muted = entry->web_contents()->IsAudioMuted(); if (entry->SetMuted(muted)) { - changed_properties->Set( - tabs_constants::kMutedInfoKey, - ExtensionTabUtil::CreateMutedInfo(entry->web_contents())); + changed_property_names.insert(tabs_constants::kMutedInfoKey); } - if (!changed_properties->empty()) { + if (!changed_property_names.empty()) { DispatchTabUpdatedEvent(entry->web_contents(), - std::move(changed_properties)); + std::move(changed_property_names)); } } void TabsEventRouter::FaviconUrlUpdated(WebContents* contents) { - content::NavigationEntry* entry = - contents->GetController().GetVisibleEntry(); - if (!entry || !entry->GetFavicon().valid) - return; - scoped_ptr changed_properties( - new base::DictionaryValue); - changed_properties->SetString( - tabs_constants::kFaviconUrlKey, - entry->GetFavicon().url.possibly_invalid_spec()); - DispatchTabUpdatedEvent(contents, std::move(changed_properties)); + content::NavigationEntry* entry = contents->GetController().GetVisibleEntry(); + if (!entry || !entry->GetFavicon().valid) + return; + std::set changed_property_names; + changed_property_names.insert(tabs_constants::kFaviconUrlKey); + DispatchTabUpdatedEvent(contents, std::move(changed_property_names)); } void TabsEventRouter::DispatchEvent( @@ -446,8 +425,8 @@ void TabsEventRouter::DispatchEvent( void TabsEventRouter::DispatchTabUpdatedEvent( WebContents* contents, - scoped_ptr changed_properties) { - DCHECK(changed_properties); + const std::set changed_property_names) { + DCHECK(!changed_property_names.empty()); DCHECK(contents); // The state of the tab (as seen from the extension point of view) has @@ -471,9 +450,8 @@ void TabsEventRouter::DispatchTabUpdatedEvent( event->restrict_to_browser_context = profile; event->user_gesture = EventRouter::USER_GESTURE_NOT_ENABLED; event->will_dispatch_callback = - base::Bind(&WillDispatchTabUpdatedEvent, - contents, - changed_properties.get()); + base::Bind(&WillDispatchTabUpdatedEvent, contents, + std::move(changed_property_names)); EventRouter::Get(profile)->BroadcastEvent(std::move(event)); } @@ -518,11 +496,9 @@ void TabsEventRouter::TabPinnedStateChanged(WebContents* contents, int index) { int tab_index; if (ExtensionTabUtil::GetTabStripModel(contents, &tab_strip, &tab_index)) { - scoped_ptr changed_properties( - new base::DictionaryValue()); - changed_properties->SetBoolean(tabs_constants::kPinnedKey, - tab_strip->IsTabPinned(tab_index)); - DispatchTabUpdatedEvent(contents, std::move(changed_properties)); + std::set changed_property_names; + changed_property_names.insert(tabs_constants::kPinnedKey); + DispatchTabUpdatedEvent(contents, std::move(changed_property_names)); } } diff --git a/chrome/browser/extensions/api/tabs/tabs_event_router.h b/chrome/browser/extensions/api/tabs/tabs_event_router.h index c78463d..0c512ae 100644 --- a/chrome/browser/extensions/api/tabs/tabs_event_router.h +++ b/chrome/browser/extensions/api/tabs/tabs_event_router.h @@ -6,6 +6,7 @@ #define CHROME_BROWSER_EXTENSIONS_API_TABS_TABS_EVENT_ROUTER_H_ #include +#include #include #include "base/macros.h" @@ -95,7 +96,7 @@ class TabsEventRouter : public TabStripModelObserver, // there's any changed property. class TabEntry; void TabUpdated(TabEntry* entry, - scoped_ptr changed_properties); + std::set changed_property_names); // Triggers a tab updated event if the favicon URL changes. void FaviconUrlUpdated(content::WebContents* contents); @@ -115,11 +116,11 @@ class TabsEventRouter : public TabStripModelObserver, scoped_ptr event_args, scoped_ptr cross_incognito_args); - // Packages |changed_properties| as a tab updated event for the tab |contents| - // and dispatches the event to the extension. + // Packages |changed_property_names| as a tab updated event for the tab + // |contents| and dispatches the event to the extension. void DispatchTabUpdatedEvent( content::WebContents* contents, - scoped_ptr changed_properties); + const std::set changed_property_names); // Register ourselves to receive the various notifications we are interested // in for a tab. Also create tab entry to observe web contents notifications. @@ -142,22 +143,13 @@ class TabsEventRouter : public TabStripModelObserver, // |contents|. TabEntry(TabsEventRouter* router, content::WebContents* contents); - // Indicate via a list of key/value pairs if a tab is loading based on its - // WebContents. Whether the state has changed or not is used to determine - // if events needs to be sent to extensions during processing of - // TabChangedAt(). If this method indicates that a tab should "hold" a - // state-change to "loading", the DidNavigate() method should eventually - // send a similar message to undo it. If false, the returned key/value - // pairs list is empty. - scoped_ptr UpdateLoadState(); - - // Indicate via a list of key/value pairs that a tab load has resulted in a - // navigation and the destination url is available for inspection. The list - // is empty if no updates should be sent. - scoped_ptr DidNavigate(); - - // Indicate via a list of key/value pairs if the title of a tab is changed. - scoped_ptr TitleChanged(); + // Indicate via a list of property names if a tab is loading based on its + // WebContents. Whether the state has changed or not is used to determine if + // events need to be sent to extensions during processing of TabChangedAt() + // If this method indicates that a tab should "hold" a state-change to + // "loading", the NavigationEntryCommitted() method should eventually send a + // similar message to undo it. + std::set UpdateLoadState(); // Update the audible and muted states and return whether they were changed bool SetAudible(bool new_val); -- cgit v1.1