Upstream Enhanced Bookmark Model

Upstream Enhanced Bookmark Model

This is the data layout of enhanced bookmark. There is no reason why
this class should live in private repository.

BUG=428832
Committed: https://crrev.com/691b64f43fc0a69752344e5edbbc4d8f406cf261
Cr-Commit-Position: refs/heads/master@{#315939}

[Ian Wen, 2015-02-06 22:42:26]

ianwen@chromium.org changed reviewers:
+ kkimlabs@chromium.org, tedchoc@chromium.org

[Ian Wen, 2015-02-06 22:42:26]

I created a new folder to match the upstream package name. (Note in clank the
package name is enhancedbookmark, now I correct it to enhanced_bookmarks.) Also,
should I add an OWNER file for it?

[Kibeom Kim (inactive), 2015-02-06 23:11:09]

On 2015/02/06 22:42:26, Ian Wen wrote:
> I created a new folder to match the upstream package name. (Note in clank the
> package name is enhancedbookmark, now I correct it to enhanced_bookmarks.)
Also,
> should I add an OWNER file for it?

If we're going to create a namespace anyways, wouldn't it make sense to move
chrome/android/java/src/org/chromium/chrome/browser/EnhancedBookmarksBridge.java
also inside there? (btw there is OWNER for that file.)

[Ian Wen, 2015-02-09 23:40:25]

ianwen@chromium.org changed reviewers:
+ newt@chromium.org
- tedchoc@chromium.org

[Ian Wen, 2015-02-09 23:43:34]

On 2015/02/06 23:11:09, Kibeom Kim wrote:
> On 2015/02/06 22:42:26, Ian Wen wrote:
> > I created a new folder to match the upstream package name. (Note in clank
the
> > package name is enhancedbookmark, now I correct it to enhanced_bookmarks.)
> Also,
> > should I add an OWNER file for it?
> 
> If we're going to create a namespace anyways, wouldn't it make sense to move
>
chrome/android/java/src/org/chromium/chrome/browser/EnhancedBookmarksBridge.java
> also inside there? (btw there is OWNER for that file.)

I moved all enhanced bookmark related java files to this new package. And added
a new OWNER file.

[Ian Wen, 2015-02-10 18:54:26]

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

[Ian Wen, 2015-02-10 18:54:26]

ptal. :)

[Kibeom Kim (inactive), 2015-02-10 19:18:35]

lgtm thanks :)

https://codereview.chromium.org/899403002/diff/60001/chrome/android/java/src/...
File
chrome/android/java/src/org/chromium/chrome/browser/enhanced_bookmarks/EnhancedBookmarksModel.java
(right):

https://codereview.chromium.org/899403002/diff/60001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/enhanced_bookmarks/EnhancedBookmarksModel.java:1:
// Copyright 2014 The Chromium Authors. All rights reserved.
nit: 2015

[cjhopman, 2015-02-10 19:26:20]

https://codereview.chromium.org/899403002/diff/60001/build/get_landmines.py
File build/get_landmines.py (right):

https://codereview.chromium.org/899403002/diff/60001/build/get_landmines.py#n...
build/get_landmines.py:65: print 'Clobber to fix gyp not rename package name
(crbug.com/457038)'
just add this to the platform() == 'android' block above.

[Ian Wen, 2015-02-11 20:02:53]

https://codereview.chromium.org/899403002/diff/60001/build/get_landmines.py
File build/get_landmines.py (right):

https://codereview.chromium.org/899403002/diff/60001/build/get_landmines.py#n...
build/get_landmines.py:65: print 'Clobber to fix gyp not rename package name
(crbug.com/457038)'
On 2015/02/10 19:26:20, cjhopman wrote:
> just add this to the platform() == 'android' block above.

Done.

[Ian Wen, 2015-02-11 20:03:13]

New patchsets have been uploaded after l-g-t-m from kkimlabs@chromium.org

[Ian Wen, 2015-02-11 21:13:09]

cjhopman@, could you please review get_landmines.py?

newt@, could you take a look at the new files/folders I created?

Thanks! @_@

[newt (away), 2015-02-11 22:57:53]

lgtm after nit

https://codereview.chromium.org/899403002/diff/80001/chrome/android/java/src/...
File
chrome/android/java/src/org/chromium/chrome/browser/enhanced_bookmarks/EnhancedBookmarksModel.java
(right):

https://codereview.chromium.org/899403002/diff/80001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/enhanced_bookmarks/EnhancedBookmarksModel.java:1:
// Copyright 2014 The Chromium Authors. All rights reserved.
2015

[cjhopman, 2015-02-12 01:17:16]

lgtm

https://codereview.chromium.org/899403002/diff/80001/build/get_landmines.py
File build/get_landmines.py (left):

https://codereview.chromium.org/899403002/diff/80001/build/get_landmines.py#o...
build/get_landmines.py:65: 
nit: this blank line should stay ("Separate top-level function and class
definitions with two blank lines").

[Ian Wen, 2015-02-12 04:28:12]

New patchsets have been uploaded after l-g-t-m from
newt@chromium.org,cjhopman@chromium.org

[Ian Wen, 2015-02-12 04:28:48]

https://codereview.chromium.org/899403002/diff/80001/build/get_landmines.py
File build/get_landmines.py (left):

https://codereview.chromium.org/899403002/diff/80001/build/get_landmines.py#o...
build/get_landmines.py:65: 
On 2015/02/12 01:17:16, cjhopman wrote:
> nit: this blank line should stay ("Separate top-level function and class
> definitions with two blank lines").

Done.

https://codereview.chromium.org/899403002/diff/80001/chrome/android/java/src/...
File
chrome/android/java/src/org/chromium/chrome/browser/enhanced_bookmarks/EnhancedBookmarksModel.java
(right):

https://codereview.chromium.org/899403002/diff/80001/chrome/android/java/src/...
chrome/android/java/src/org/chromium/chrome/browser/enhanced_bookmarks/EnhancedBookmarksModel.java:1:
// Copyright 2014 The Chromium Authors. All rights reserved.
On 2015/02/11 22:57:53, newt wrote:
> 2015

Done.

[Ian Wen, 2015-02-12 04:29:02]

The CQ bit was checked by ianwen@chromium.org

[commit-bot: I haz the power, 2015-02-12 04:29:23]

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

[commit-bot: I haz the power, 2015-02-12 06:33:49]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-02-12 06:35:03]

Patchset 6 (id:??) landed as
https://crrev.com/691b64f43fc0a69752344e5edbbc4d8f406cf261
Cr-Commit-Position: refs/heads/master@{#315939}

[alex clarke (OOO till 29th), 2015-02-12 14:51:43]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/920043003/ by alexclarke@chromium.org.

The reason for reverting is: This is blocking downstream and people are
complaining.  I tried to land the downstream fix but there was a compile
problem.

Sorry, hope you can fix the problems and re-land..

[Nico, 2015-03-30 18:50:44]

Please don't use landmines like this. The script now contains this note:

  # DO NOT add landmines as part of a regular CL. Landmines are a last-effort
  # bandaid fix if a CL that got landed has a build dependency bug and all bots
  # need to be cleaned up. If you're writing a new CL that causes build
  # dependency problems, fix the dependency problems instead of adding a
  # landmine.

Instead of adding a landmine, you should've fixed the underlying bug, like
mlamouri just did in https://codereview.chromium.org/1040643003

(Reviewers: Don't lg CLs with landmines either.)