Upstream DocumentTabModelImpl and related classes

Upstream DocumentTabModelImpl and related classes

Moves the DocumentTabModelImpl and its associated tests/utility classes upstream
from the downstream directories.

Updates the findbugs to remove obsolete ones and add
temporary new ones (until more is upstreamed).

Setting NOTRY to true to get around findbugs changes.

BUG=415747
TEST=DocumentTabModelImplTest, OffTheRecordDocumentTabModelTest
NOTRY=true

Committed: https://crrev.com/1f8dc216df5ca11615cc5f5ae49bbd286ac73dba
Cr-Commit-Position: refs/heads/master@{#308680}

[gone, 2014-12-16 00:59:45]

dfalcantara@chromium.org changed reviewers:
+ yusufo@chromium.org

[Yusuf, 2014-12-16 01:02:02]

rubberstamp lgtm

[gone, 2014-12-16 01:05:38]

dfalcantara@chromium.org changed reviewers:
+ tedchoc@chromium.org

[gone, 2014-12-16 18:59:51]

dfalcantara@chromium.org changed reviewers:
+ dtrainor@chromium.org
- tedchoc@chromium.org

[gone, 2014-12-16 18:59:52]

-tedchoc@ who's out
+dtrainor@ for OWNERS

[David Trainor- moved to gerrit, 2014-12-16 19:13:52]

lgtm

[gone, 2014-12-16 19:20:41]

dfalcantara@chromium.org changed reviewers:
+ thakis@chromium.org

[gone, 2014-12-16 19:21:38]

TBRing thakis for chrome/chrome.gyp addition.

[gone, 2014-12-16 19:23:54]

NOTRY=true to get around the findbugs additions (trybots all passed)

[gone, 2014-12-16 19:24:20]

The CQ bit was checked by dfalcantara@chromium.org

[Ted C, 2014-12-16 19:25:32]

tedchoc@chromium.org changed reviewers:
+ tedchoc@chromium.org

[Ted C, 2014-12-16 19:25:33]

lgtm

https://chromiumcodereview.appspot.com/802343003/diff/60001/chrome/android/ja...
File
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/DocumentTabModelImplTest.java
(right):

https://chromiumcodereview.appspot.com/802343003/diff/60001/chrome/android/ja...
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/DocumentTabModelImplTest.java:5:
package org.chromium.chrome.browser.tabmodel;
tabmodel.document?

https://chromiumcodereview.appspot.com/802343003/diff/60001/chrome/android/ja...
File
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/OffTheRecordDocumentTabModelTest.java
(right):

https://chromiumcodereview.appspot.com/802343003/diff/60001/chrome/android/ja...
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/OffTheRecordDocumentTabModelTest.java:5:
package org.chromium.chrome.browser.tabmodel;
same

[commit-bot: I haz the power, 2014-12-16 19:25:48]

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

[gone, 2014-12-16 19:26:01]

The CQ bit was unchecked by dfalcantara@chromium.org

[gone, 2014-12-16 19:33:21]

https://chromiumcodereview.appspot.com/802343003/diff/60001/chrome/android/ja...
File
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/DocumentTabModelImplTest.java
(right):

https://chromiumcodereview.appspot.com/802343003/diff/60001/chrome/android/ja...
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/DocumentTabModelImplTest.java:5:
package org.chromium.chrome.browser.tabmodel;
On 2014/12/16 19:25:32, Ted C wrote:
> tabmodel.document?

Done.

https://chromiumcodereview.appspot.com/802343003/diff/60001/chrome/android/ja...
File
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/OffTheRecordDocumentTabModelTest.java
(right):

https://chromiumcodereview.appspot.com/802343003/diff/60001/chrome/android/ja...
chrome/android/javatests/src/org/chromium/chrome/browser/tabmodel/OffTheRecordDocumentTabModelTest.java:5:
package org.chromium.chrome.browser.tabmodel;
On 2014/12/16 19:25:32, Ted C wrote:
> same

Done.

[gone, 2014-12-16 19:37:21]

The CQ bit was checked by dfalcantara@chromium.org

[commit-bot: I haz the power, 2014-12-16 19:39:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/802343003/80001

[commit-bot: I haz the power, 2014-12-16 19:47:24]

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

[commit-bot: I haz the power, 2014-12-16 19:47:25]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)
  mac_chromium_compile_dbg_ng on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[Nico, 2014-12-16 19:52:29]

chrome.gyp lgtm, but:

https://codereview.chromium.org/802343003/diff/80001/chrome/android/BUILD.gn
File chrome/android/BUILD.gn (right):

https://codereview.chromium.org/802343003/diff/80001/chrome/android/BUILD.gn#...
chrome/android/BUILD.gn:92:
"//third_party/android_protobuf:protobuf_nano_javalib",
This doesn't look right to me; the gyp version doesn't need this and
https://code.google.com/p/chromium/codesearch#chromium/src/build/config/andro...
tries to add this automatically where needed from what i understand (?)

[gone, 2014-12-16 19:59:00]

https://chromiumcodereview.appspot.com/802343003/diff/80001/chrome/android/BU...
File chrome/android/BUILD.gn (right):

https://chromiumcodereview.appspot.com/802343003/diff/80001/chrome/android/BU...
chrome/android/BUILD.gn:92:
"//third_party/android_protobuf:protobuf_nano_javalib",
On 2014/12/16 19:52:29, Nico wrote:
> This doesn't look right to me; the gyp version doesn't need this and
>
https://code.google.com/p/chromium/codesearch#chromium/src/build/config/andro...
> tries to add this automatically where needed from what i understand (?)

Tried to do it without this line, but PS2 failed the GN bots :/

[Nico, 2014-12-16 20:30:02]

On 2014/12/16 19:59:00, dfalcantara wrote:
>
https://chromiumcodereview.appspot.com/802343003/diff/80001/chrome/android/BU...
> File chrome/android/BUILD.gn (right):
> 
>
https://chromiumcodereview.appspot.com/802343003/diff/80001/chrome/android/BU...
> chrome/android/BUILD.gn:92:
> "//third_party/android_protobuf:protobuf_nano_javalib",
> On 2014/12/16 19:52:29, Nico wrote:
> > This doesn't look right to me; the gyp version doesn't need this and
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/build/config/andro...
> > tries to add this automatically where needed from what i understand (?)
> 
> Tried to do it without this line, but PS2 failed the GN bots :/

Why?

I believe you that it failed without this, but I'm not sure it's the right fix.

[gone, 2014-12-16 21:18:05]

On 2014/12/16 20:30:02, Nico wrote:
> On 2014/12/16 19:59:00, dfalcantara wrote:
> >
>
https://chromiumcodereview.appspot.com/802343003/diff/80001/chrome/android/BU...
> > File chrome/android/BUILD.gn (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/802343003/diff/80001/chrome/android/BU...
> > chrome/android/BUILD.gn:92:
> > "//third_party/android_protobuf:protobuf_nano_javalib",
> > On 2014/12/16 19:52:29, Nico wrote:
> > > This doesn't look right to me; the gyp version doesn't need this and
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/build/config/andro...
> > > tries to add this automatically where needed from what i understand (?)
> > 
> > Tried to do it without this line, but PS2 failed the GN bots :/
> 
> Why?
> 
> I believe you that it failed without this, but I'm not sure it's the right
fix.

AFAICT, the GN file only adds that target automatically if there is a srcjar
dependency that is like :${_template_name}__protoc_java, which the
document_tab_model_info_proto_java target doesn't match. 
build/secondary/third_party/cacheinvalidation/BUILD.gn shows that there's an
android_library that gets created for cacheinvalidation, but I think that's only
because cacheinvalidation is a legitimate third_party target.

[gone, 2014-12-16 21:19:44]

On 2014/12/16 21:18:05, dfalcantara wrote:
> On 2014/12/16 20:30:02, Nico wrote:
> > On 2014/12/16 19:59:00, dfalcantara wrote:
> > >
> >
>
https://chromiumcodereview.appspot.com/802343003/diff/80001/chrome/android/BU...
> > > File chrome/android/BUILD.gn (right):
> > > 
> > >
> >
>
https://chromiumcodereview.appspot.com/802343003/diff/80001/chrome/android/BU...
> > > chrome/android/BUILD.gn:92:
> > > "//third_party/android_protobuf:protobuf_nano_javalib",
> > > On 2014/12/16 19:52:29, Nico wrote:
> > > > This doesn't look right to me; the gyp version doesn't need this and
> > > >
> > >
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/build/config/andro...
> > > > tries to add this automatically where needed from what i understand (?)
> > > 
> > > Tried to do it without this line, but PS2 failed the GN bots :/
> > 
> > Why?
> > 
> > I believe you that it failed without this, but I'm not sure it's the right
> fix.
> 
> AFAICT, the GN file only adds that target automatically if there is a srcjar
> dependency that is like :${_template_name}__protoc_java, which the
> document_tab_model_info_proto_java target doesn't match. 
> build/secondary/third_party/cacheinvalidation/BUILD.gn shows that there's an
> android_library that gets created for cacheinvalidation, but I think that's
only
> because cacheinvalidation is a legitimate third_party target.

Actually, now I'm getting confused about how that template is invoked.  Is there
a GN
guru who'd know how things are possibly going awry?

[Nico, 2014-12-16 21:29:38]

cjhopman knows both gn and java; I'd ask him.

On Tue, Dec 16, 2014 at 1:19 PM, <dfalcantara@chromium.org> wrote:
>
> On 2014/12/16 21:18:05, dfalcantara wrote:
>
>> On 2014/12/16 20:30:02, Nico wrote:
>> > On 2014/12/16 19:59:00, dfalcantara wrote:
>> > >
>> >
>>
>
> https://chromiumcodereview.appspot.com/802343003/diff/
> 80001/chrome/android/BUILD.gn
>
>> > > File chrome/android/BUILD.gn (right):
>> > >
>> > >
>> >
>>
>
> https://chromiumcodereview.appspot.com/802343003/diff/
> 80001/chrome/android/BUILD.gn#newcode92
>
>> > > chrome/android/BUILD.gn:92:
>> > > "//third_party/android_protobuf:protobuf_nano_javalib",
>> > > On 2014/12/16 19:52:29, Nico wrote:
>> > > > This doesn't look right to me; the gyp version doesn't need this and
>> > > >
>> > >
>> >
>>
>
> https://code.google.com/p/chromium/codesearch#chromium/
> src/build/config/android/rules.gni&q=protobuf_nano_
> javalib&sq=package:chromium&type=cs&l=1682
>
>> > > > tries to add this automatically where needed from what i understand
>> (?)
>> > >
>> > > Tried to do it without this line, but PS2 failed the GN bots :/
>> >
>> > Why?
>> >
>> > I believe you that it failed without this, but I'm not sure it's the
>> right
>> fix.
>>
>
>  AFAICT, the GN file only adds that target automatically if there is a
>> srcjar
>> dependency that is like :${_template_name}__protoc_java, which the
>> document_tab_model_info_proto_java target doesn't match.
>> build/secondary/third_party/cacheinvalidation/BUILD.gn shows that
>> there's an
>> android_library that gets created for cacheinvalidation, but I think
>> that's
>>
> only
>
>> because cacheinvalidation is a legitimate third_party target.
>>
>
> Actually, now I'm getting confused about how that template is invoked.  Is
> there
> a GN
> guru who'd know how things are possibly going awry?
>
> https://chromiumcodereview.appspot.com/802343003/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[cjhopman, 2014-12-16 21:33:59]

cjhopman@chromium.org changed reviewers:
+ cjhopman@chromium.org

[cjhopman, 2014-12-16 21:34:00]

https://chromiumcodereview.appspot.com/802343003/diff/80001/chrome/android/BU...
File chrome/android/BUILD.gn (right):

https://chromiumcodereview.appspot.com/802343003/diff/80001/chrome/android/BU...
chrome/android/BUILD.gn:92:
"//third_party/android_protobuf:protobuf_nano_javalib",
On 2014/12/16 19:59:00, dfalcantara wrote:
> On 2014/12/16 19:52:29, Nico wrote:
> > This doesn't look right to me; the gyp version doesn't need this and
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/build/config/andro...
> > tries to add this automatically where needed from what i understand (?)
> 
> Tried to do it without this line, but PS2 failed the GN bots :/

In GN, the classpath for a java/android library only contains the things
explicitly listed as dependencies (in gyp it contains all transitive java
dependencies).

The linked template is just for proto_java_library targets.

https://chromiumcodereview.appspot.com/802343003/diff/80001/chrome/android/BU...
chrome/android/BUILD.gn:92:
"//third_party/android_protobuf:protobuf_nano_javalib",
Also, GN will probably change to use "all" transitive dependencies for the
classpath since there are some difficult issues with only using direct
dependencies (though this is not necessarily one of those cases).

[cjhopman, 2014-12-16 21:36:38]

lgtm

[gone, 2014-12-16 21:38:56]

The CQ bit was checked by dfalcantara@chromium.org

[gone, 2014-12-16 21:39:28]

Running it through that final flaky red trybot for good measure.

[commit-bot: I haz the power, 2014-12-16 21:39:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/802343003/80001

[commit-bot: I haz the power, 2014-12-16 21:47:05]

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

[commit-bot: I haz the power, 2014-12-16 21:47:07]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[gone, 2014-12-16 22:05:16]

The CQ bit was checked by dfalcantara@chromium.org

[commit-bot: I haz the power, 2014-12-16 22:06:01]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/802343003/80001

[commit-bot: I haz the power, 2014-12-16 22:12:31]

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

[commit-bot: I haz the power, 2014-12-16 22:12:32]

Try jobs failed on following builders:
  chromium_presubmit on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[gone, 2014-12-16 22:13:44]

The CQ bit was checked by dfalcantara@chromium.org

[commit-bot: I haz the power, 2014-12-16 22:15:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/802343003/80001

[commit-bot: I haz the power, 2014-12-16 22:16:52]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2014-12-16 22:17:34]

Patchset 5 (id:??) landed as
https://crrev.com/1f8dc216df5ca11615cc5f5ae49bbd286ac73dba
Cr-Commit-Position: refs/heads/master@{#308680}