Add support for unregistering web intents default by service_url.

Add support for unregistering defaults by service_url.

BUG=137907
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=150669

[Steve McKay, 2012-08-07 23:45:29]

Calling the new method stack will remove ALL web intents defaults associated
with the supplied service url.

[James Hawkins, 2012-08-08 16:22:37]

https://chromiumcodereview.appspot.com/10824212/diff/3001/chrome/browser/inte...
File chrome/browser/intents/web_intents_registry.h (right):

https://chromiumcodereview.appspot.com/10824212/diff/3001/chrome/browser/inte...
chrome/browser/intents/web_intents_registry.h:99: virtual void
UnregisterDefaultIntentServicesForServiceURL(
nit: This name is a bit unwieldy; my suggestion:

UnregisterDefaultServiceForURL()

Similarly, I'd rename the methods just above to (Un)registerDefaultService()

https://chromiumcodereview.appspot.com/10824212/diff/3001/chrome/browser/inte...
File chrome/browser/intents/web_intents_registry_unittest.cc (right):

https://chromiumcodereview.appspot.com/10824212/diff/3001/chrome/browser/inte...
chrome/browser/intents/web_intents_registry_unittest.cc:738:
TEST_F(WebIntentsRegistryTest, UnregisterDefaultIntentServicesForServiceURL) {
nit: Document what this method is testing.

I'd rename the test to be more indicative of what it's testing as well, i.e.,
not the name of the method.

https://chromiumcodereview.appspot.com/10824212/diff/3001/chrome/browser/inte...
chrome/browser/intents/web_intents_registry_unittest.cc:739: 
nit: Remove blank line.

https://chromiumcodereview.appspot.com/10824212/diff/3001/chrome/browser/inte...
chrome/browser/intents/web_intents_registry_unittest.cc:763: // Test we can
retrieve default entries by action.
nit: Reword to remove pronoun.

https://chromiumcodereview.appspot.com/10824212/diff/3001/chrome/browser/webd...
File chrome/browser/webdata/web_data_service.cc (right):

https://chromiumcodereview.appspot.com/10824212/diff/3001/chrome/browser/webd...
chrome/browser/webdata/web_data_service.cc:347: request));
Optional nit: Condense parameters to save a line.

https://chromiumcodereview.appspot.com/10824212/diff/3001/chrome/browser/webd...
File chrome/browser/webdata/web_intents_table_unittest.cc (right):

https://chromiumcodereview.appspot.com/10824212/diff/3001/chrome/browser/webd...
chrome/browser/webdata/web_intents_table_unittest.cc:25: string16 test_action =
ASCIIToUTF16("http://webintents.org/intents/share");
These should all be const.

[Steve McKay, 2012-08-08 17:54:42]

PTAL.

http://codereview.chromium.org/10824212/diff/3001/chrome/browser/intents/web_...
File chrome/browser/intents/web_intents_registry.h (right):

http://codereview.chromium.org/10824212/diff/3001/chrome/browser/intents/web_...
chrome/browser/intents/web_intents_registry.h:99: virtual void
UnregisterDefaultIntentServicesForServiceURL(
On 2012/08/08 16:22:37, James Hawkins wrote:
> nit: This name is a bit unwieldy; my suggestion:
> 
> UnregisterDefaultServiceForURL()
> 
> Similarly, I'd rename the methods just above to (Un)registerDefaultService()

Since a default service has both a url pattern and a service url I think we need
to qualify which type in the method name.

http://codereview.chromium.org/10824212/diff/3001/chrome/browser/intents/web_...
File chrome/browser/intents/web_intents_registry_unittest.cc (right):

http://codereview.chromium.org/10824212/diff/3001/chrome/browser/intents/web_...
chrome/browser/intents/web_intents_registry_unittest.cc:738:
TEST_F(WebIntentsRegistryTest, UnregisterDefaultIntentServicesForServiceURL) {
On 2012/08/08 16:22:37, James Hawkins wrote:
> nit: Document what this method is testing.
> 
> I'd rename the test to be more indicative of what it's testing as well, i.e.,
> not the name of the method.

Revised method name, leaving this method name as-is.

http://codereview.chromium.org/10824212/diff/3001/chrome/browser/intents/web_...
chrome/browser/intents/web_intents_registry_unittest.cc:739: 
On 2012/08/08 16:22:37, James Hawkins wrote:
> nit: Remove blank line.

Done.

http://codereview.chromium.org/10824212/diff/3001/chrome/browser/intents/web_...
chrome/browser/intents/web_intents_registry_unittest.cc:763: // Test we can
retrieve default entries by action.
On 2012/08/08 16:22:37, James Hawkins wrote:
> nit: Reword to remove pronoun.

Nuked the invalid comment.

http://codereview.chromium.org/10824212/diff/3001/chrome/browser/intents/web_...
chrome/browser/intents/web_intents_registry_unittest.cc:775: // Test we can
retrieve default entries by action.
Nuked this too.

http://codereview.chromium.org/10824212/diff/3001/chrome/browser/webdata/web_...
File chrome/browser/webdata/web_data_service.cc (right):

http://codereview.chromium.org/10824212/diff/3001/chrome/browser/webdata/web_...
chrome/browser/webdata/web_data_service.cc:347: request));
On 2012/08/08 16:22:37, James Hawkins wrote:
> Optional nit: Condense parameters to save a line.

Collapsed entire call after method renaming.

http://codereview.chromium.org/10824212/diff/3001/chrome/browser/webdata/web_...
File chrome/browser/webdata/web_intents_table_unittest.cc (right):

http://codereview.chromium.org/10824212/diff/3001/chrome/browser/webdata/web_...
chrome/browser/webdata/web_intents_table_unittest.cc:25: string16 test_action =
ASCIIToUTF16("http://webintents.org/intents/share");
On 2012/08/08 16:22:37, James Hawkins wrote:
> These should all be const.

Done.

[Steve McKay, 2012-08-08 17:55:24]

FYI, dropped Greg, since James has taken up the review (beyonding lending his
OWNERship approval).

[Steve McKay, 2012-08-08 20:28:22]

PTAL.

[James Hawkins, 2012-08-08 20:30:11]

http://codereview.chromium.org/10824212/diff/8001/chrome/browser/intents/web_...
File chrome/browser/intents/web_intents_registry.h (right):

http://codereview.chromium.org/10824212/diff/8001/chrome/browser/intents/web_...
chrome/browser/intents/web_intents_registry.h:99: virtual void
UnregisterDefaultServices(const GURL& service_url);
'Services' here is a misnomer, since we're technically unregistering multiple
intent filter defaults for a single service.

[James Hawkins, 2012-08-08 20:51:58]

LGTM with UnregisterServiceDefaults().

[commit-bot: I haz the power, 2012-08-08 21:07:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/smckay@chromium.org/10824212/2013

[commit-bot: I haz the power, 2012-08-08 23:22:42]

Try job failure for 10824212-2013 (retry) (retry) on mac_rel for step
"browser_tests".
It's a second try, previously, step "browser_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2012-08-08 23:54:58]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/smckay@chromium.org/10824212/2013

[commit-bot: I haz the power, 2012-08-09 01:22:56]

Change committed as 150669